Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes Jakarta Servlet API version #25032

Merged
merged 2 commits into from
Jul 7, 2024
Merged

Fixes Jakarta Servlet API version #25032

merged 2 commits into from
Jul 7, 2024

Conversation

avpinchuk
Copy link
Contributor

@avpinchuk avpinchuk commented Jul 7, 2024 •
edited
Loading

Fixes Jakarta Servlet API version in various POMs.

If we build GlassFish with the empty maven cache:

mvn clean install -Pstaging

the local maven repo contains strange artifact:

image

@avpinchuk
Fixes Jakarta Servlet API version
d3729d9 
Signed-off-by: Alexander Pinčuk <alexander.v.pinchuk@gmail.com>
@avpinchuk avpinchuk added bug Something isn't working build and test improvement labels Jul 7, 2024
@avpinchuk avpinchuk added this to the 7.0.16 milestone Jul 7, 2024
@avpinchuk avpinchuk self-assigned this Jul 7, 2024
pzygielo
pzygielo reviewed Jul 7, 2024
View reviewed changes
appserver/pom.xml
@@ -883,12 +883,12 @@
<artifact>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<version>${servlet-api.version}</version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this missing property didn't bother the plugin...

This comment was marked as outdated.

Sign in to view

@avpinchuk
Fixes spec version
c07d814 
Signed-off-by: Alexander Pinčuk <alexander.v.pinchuk@gmail.com>
@pzygielo
Copy link
Contributor

pzygielo commented Jul 7, 2024

If we build GlassFish with the empty maven cache:

mvn clean install -Pstaging

the local maven repo contains strange artifact:

I suppose: maven resolver tries to download specified version (#25032 (comment)) but there is no such property to be interpolated so it stays as is.

@arjantijms arjantijms merged commit b92076d into eclipse-ee4j:master Jul 7, 2024
2 checks passed
@pzygielo pzygielo mentioned this pull request Jul 7, 2024
Merged
@dmatej
Copy link
Contributor

dmatej commented Jul 7, 2024

Aha, I got it, dependencies are specified correctly (except tests, where it is reported as an error by Eclipse IDE; maybe those tests fail to compile but bash script ignores the result? We should check ...), just the spec plugin is wrong and unfortunately doesn't crash the build even with unresolved properties.

Good catch!

@pzygielo
Copy link
Contributor

pzygielo commented Jul 7, 2024

@avpinchuk avpinchuk deleted the fix-servlet-api-version branch July 8, 2024 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arjantijms arjantijms arjantijms approved these changes

@pzygielo pzygielo pzygielo approved these changes

@dmatej dmatej Awaiting requested review from dmatej

@hs536 hs536 Awaiting requested review from hs536

Assignees

@avpinchuk avpinchuk

Labels
bug Something isn't working build and test improvement
Projects
None yet
Milestone
7.0.16
Development

Successfully merging this pull request may close these issues.
4 participants
@avpinchuk @pzygielo @dmatej @arjantijms