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.

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

Fixed false error thrown if parent pom version uses variable #7158

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

Achal1607
Copy link
Contributor

@Achal1607 Achal1607 commented Mar 14, 2024

  • When parent pom uses variable for defining version:
    image

  • When child pom uses variable again for the parent version then it throws false error:
    image

So, fixed the above mentioned issue in this PR.
Refer for more info: oracle/javavscode#95

@Achal1607 Achal1607 changed the title Fixed false error thrown if parent pom version uses variable is used Fixed false error thrown if parent pom version uses variable Mar 14, 2024
@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Maven [ci] enable "build tools" tests labels Mar 14, 2024
@mbien mbien added this to the NB22 milestone Mar 14, 2024
@mbien mbien self-requested a review March 15, 2024 06:09
Signed-off-by: Achal Talati <achal.talati@oracle.com>
Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

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

good fix!

@Achal1607
Copy link
Contributor Author

I don't know why this test is failing, I haven't changed anything in context of this test.

@mbien
Copy link
Member

mbien commented Mar 15, 2024

happens sometimes, I restarted it.

@mbien mbien merged commit bbd18aa into apache:master Mar 15, 2024
42 checks passed
@ebarboni
Copy link
Contributor

Hi sorry to join late. But ${revision} is reserved word, if ${my-revision} were used it's should warn that version is incorrect

${revision}, ${changelist}, and ${sha1} are the reserved versions as per https://maven.apache.org/docs/3.2.1/release-notes.html

@Achal1607
Copy link
Contributor Author

Oh sorry I didn't knew of this. So, anything apart from ${revision}, ${changelist}, and ${sha1} cannot be used as expression in version field?

@ebarboni
Copy link
Contributor

I'm not aware of others 'keyword' but if you use a random properties you will have warning

Some problems were encountered while building the effective model for com.mycompany:mavenproject2:pom:${sha1aa}
'version' contains an expression but should be a constant. @ line 6, column 14

It is highly recommended to fix these problems because they threaten the stability of your build.

For this reason, future Maven versions might no longer support building such malformed projects.

@neilcsmith-net
Copy link
Member

Yes, valid properties from here I think - https://github.com/apache/maven/blob/master/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/DefaultModelVersionProcessor.java#L43 Not sure best way to access that, or whether to code in the allowed values?

@ebarboni
Copy link
Contributor

maybe hardcoded + link to the github :p

@Achal1607
Copy link
Contributor Author

  • When child pom uses variable again for the parent version then it throws false error:
    image

So, fixed the above mentioned issue in this PR. Refer for more info: oracle/javavscode#95

The above error is anyways misleading for any case. So, imo we can keep the change that is done in this PR as it is.
Since, warning is already shown if something other than these 3 keywords are used in the version tag, isn't it correct?

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Maven [ci] enable "build tools" tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants