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

versions:set doesn't set version if non-direct properties are used in version element #916

Closed
llengo-pl opened this issue Feb 2, 2023 · 10 comments · Fixed by #924
Closed
Labels
Milestone

Comments

@llengo-pl
Copy link

llengo-pl commented Feb 2, 2023

Maven supports 5 property classes in pom.xml files:

  1. env.X - environment variables
  2. project.x - any project properties from the pom.xml
  3. settings.x - properties from settings.xml
  4. Java Systems Properties, e.g. java.home
  5. x - properties from <properties> section of the pom.xml

If any of the 1-4 property classes is used in version element (even transitively) the resolution of the current version will fail, and no new version will be set.

E.g. parent pom.xml

<project>
    <modelVersion>4.0.0</modelVersion>

    <groupId>com.example.app</groupId>
    <artifactId>my-parent</artifactId>
    <version>1.0.0</version>
    <packaging>pom</packaging>

</project>

child pom.xml

<project>
    <modelVersion>4.0.0</modelVersion>

    <parent>
        <groupId>com.example.app</groupId>
        <artifactId>my-parent</artifactId>
        <version>1.0.0</version>
    </parent>

    <groupId>com.example.app</groupId>
    <artifactId>my-app</artifactId>
    <version>${project.parent.version}</version>
</project>

Attempt to set version:

$ mvn versions:set -DnewVersion=1.1.1
(...)
[INFO] Building my-app 1.0.0
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- versions-maven-plugin:2.14.2:set (default-cli) @ my-app ---
[INFO] Searching for local aggregator root...
[INFO] Local aggregation root: C:\Work\projects\3rd_party\tst
[INFO] Processing change of com.example.app:my-app:1.0.0 -> 1.1.1
expression: project.parent.version no value

This caught us off guard - we use versions:set to override the version only for our 'dev' builds which are rather seldom run.

Workaround

$ mvn versions:set -DnewVersion=1.1.1 -DoldVersion='*'
(...)
[INFO] Building my-app 1.0.0
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- versions-maven-plugin:2.14.2:set (default-cli) @ my-app ---
[INFO] Searching for local aggregator root...
[INFO] Local aggregation root: C:\Work\projects\3rd_party\tst
[INFO] Processing change of com.example.app:my-app:* -> 1.1.1
expression: project.parent.version no value
[INFO] Processing com.example.app:my-app
[INFO]     Updating project com.example.app:my-app
[INFO]         from version  to 1.1.1

I know this might be a corner case, and not really an issue, since it is encouraged that the version element is constant and not an expression. Furthermore I couldn't come up with other probable use cases.

Having said that, I think that because some property interpolation is already handled by the plugin, one might expect that all property classes may be used, and be surprised by this behavior.

Please consider whether it's worth handling.

@deckrider
Copy link

deckrider commented Feb 24, 2023

This is also an issue with the simplest pom.xml containing:

<version>${revision}-SNAPSHOT</version>

Running:

mvn org.codehaus.mojo:versions-maven-plugin:2.15.0:set -DnewVersion=1.0

should produce:

<version>1.0</version>

However it does nothing.

The last version of this plugin that worked was 2.13.0 and it remains broken in 2.15.0

@jarmoniuk
Copy link
Contributor

jarmoniuk commented Feb 24, 2023

I see. Thanks for reporting this. I'm looking into this.

@jarmoniuk
Copy link
Contributor

jarmoniuk commented Feb 24, 2023

So. Looks like #799 opened a can of worms. Considering reverting it.

@jarmoniuk
Copy link
Contributor

jarmoniuk commented Feb 25, 2023

Let me explain what the change that causes the regression did. Then you will probably appreciate the workaround that I will give until the fix is merged in.

In #794, a user has noticed that if the oldVersion parameter was present, the plugin ignored the versions of the processed files and replaced the versions anyway. I tracked down the reason for this to what I understand is a 'hack', where the replacement is made regardless if a match between the oldVersion and the dependency version is found if the oldVersion parameter does not contain a wildcard.

I understand that the reason for this 'hack' was so that the plugin interpolated the properties, so that version match happened properly on the interpolated value. Since the plugin used raw (=non-interpolated) files, it would mean that it would have to do the interpolation on its own, lose from Maven.

My change made it so that an attempt a (simple) interpolation was done and then the interpolated value was matched. If there was no match, no change was done.

It proves that the interpolation was not complete, so it did not work as it should've. I'm going to revert the change now and look for another way to fix the original issue.

In the mean time, to go around the restriction with the latest version of the plugin, please use -DoldVersion='*'.

jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Feb 25, 2023
- Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params
- Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion
- Matching occurs always unless *processAllModules* is set
- If *processAllModules* is not set, *oldVersion* will be respected
@llengo-pl
Copy link
Author

Thank you for taking a look at this.

@billdinger
Copy link

billdinger commented Mar 13, 2023

So @ajarmoniuk I think I am having the same or similar issue but the workaround here doesn't appear to work. So for awareness

I have a reactor pom and a multiple child poms setup all using <version>${revision}</version> as per the maven ci docs. So the minimal setup looks like this..

root pom.xml:

<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
    <modelVersion>4.0.0</modelVersion>


    <artifactId>foo.bar</artifactId>
    <packaging>pom</packaging>
    <version>${revision}</version>
    <name>Good Test</name>
    <description>I love maven</description>

The child pom(s) then look like this:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <parent>
        <groupId>com.foo.bar</groupId>
        <artifactId>some-thing</artifactId>
        <version>${revision}</version>
        <relativePath>../pom.xml</relativePath>
    </parent>

    <artifactId>good-test</artifactId>
    <packaging>pom</packaging>
    <version>${revision}</version>
    <name>Good Test</name>
    <description>maven is the bst child pom</description>

when using the latest - 2.15.0 when I run the command below I get the following output

mvn -f pom.xml -DnewVersion=1.0.803026055.9ee81a92 - versions:set

[INFO] Processing change of some.project:1.0.803026055.9ee81a92 -> 1.0.803026055.9ee81a92
expression: revision no value
expression: revision no value
expression: revision no value
expression: revision no value
expression: revision no value
expression: revision no value
expression: revision no value
expression: revision no value
expression: revision no value
expression: revision no value

The expression: revision no value are all applied to the child poms. When I pin the version to 2.13.0 it works fine and the revision is properly set in all child poms. 2.14.0 doesn't give me the same errors but also just doesnt update the child poms. the workaround I use is below in case it helps anyone, just pin to 2.13.0

mvn  -DnewVersion=1.0.803026055.9ee81a92  -e org.codehaus.mojo:versions-maven-plugin:2.13.0:set

@jarmoniuk
Copy link
Contributor

Thanks. Gonna check tomorrow (it's 11 pm here in Europe) if my PR covers your case (I think it should). If you're feeling lucky and/or have too much time on your hands, you could also try the PR branch yourself on your setup.

@billdinger
Copy link

I will do so later on (in my day). Hopefully be able to confirm for you by the time you are up and working again $whenever tomorrow. Thanks much!

@billdinger
Copy link

Grabbed your changes off issue-916-revert-799 and just to be safe force updated the version to 2.16.0, ran a mvn clean install and then forced a update:

mvn -o -U -DnewVersion=1.0.803026055.9ee81a92 -e org.codehaus.mojo:versions-maven-plugin:2.16.0:set
worked just great

image

Your regression set versions-maven-plugin/src/it/it-set-issue-916-properties/pom.xml pretty much covers it, the only additional test would be if you wanted to add a child pom. and verify the revision carried through to both the parent and the child. I would happily do so (well, or muddle my way through it ;)) if you think it's valuable.

Thanks for the fix though @ajarmoniuk

@jarmoniuk
Copy link
Contributor

Great to hear.

And I'll update the PR with this test case.

jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Mar 14, 2023
- Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params
- Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion
- Matching occurs always unless *processAllModules* is set
- If *processAllModules* is not set, *oldVersion* will be respected
- Included an additional test case from mojohaus#916 testing child version
@slawekjaranowski slawekjaranowski added this to the 2.16.0 milestone May 9, 2023
@slawekjaranowski slawekjaranowski linked a pull request May 9, 2023 that will close this issue
jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue May 10, 2023
- Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params
- Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion
- Matching occurs always unless *processAllModules* is set
- If *processAllModules* is not set, *oldVersion* will be respected
- Included an additional test case from mojohaus#916 testing child version
slawekjaranowski pushed a commit that referenced this issue May 14, 2023
- Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params
- Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion
- Matching occurs always unless *processAllModules* is set
- If *processAllModules* is not set, *oldVersion* will be respected
- Included an additional test case from #916 testing child version
dongjoon-hyun pushed a commit to apache/spark that referenced this issue Jun 18, 2023
### What changes were proposed in this pull request?
The pr aims to update some maven plugins to newest version. include:
- versions-maven-plugin from 2.15.0 to 2.16.0
- maven-source-plugin from 3.2.1 to 3.3.0
- maven-surefire-plugin from 3.1.0 to 3.1.2
- maven-dependency-plugin from 3.5.0 to 3.6.0

### Why are the changes needed?
- versions-maven-plugin
1.Release Notes: https://github.com/mojohaus/versions/releases/tag/2.16.0
2.Bug Fix:
Resolves: display-dependency-updates only shows updates from the most major allowed segment (mojohaus/versions#966) ajarmoniuk
Resolves mojohaus/versions#931: Fixing problems with encoding in UseDepVersion and PomHelper (mojohaus/versions#932) ajarmoniuk
Resolves mojohaus/versions#916: Partially reverted mojohaus/versions#799. (mojohaus/versions#924) ajarmoniuk
Resolves mojohaus/versions#954: Excluded plexus-container-default (mojohaus/versions#955) ajarmoniuk
Resolves mojohaus/versions#951: DefaultArtifactVersion::getVersion can be null (mojohaus/versions#952) ajarmoniuk
BoundArtifactVersion.toString() to work with NumericVersionComparator (mojohaus/versions#930) ajarmoniuk
Issue mojohaus/versions#925: Protect against an NPE if a dependency version is defined in dependencyManagement (mojohaus/versions#926) ajarmoniuk

- maven-source-plugin
v3.2.1 VS v3.3.0: apache/maven-source-plugin@maven-source-plugin-3.2.1...maven-source-plugin-3.3.0

- maven-surefire-plugin
Release Notes: https://github.com/apache/maven-surefire/releases/tag/surefire-3.1.2

- maven-dependency-plugin
v3.5.0 VS v3.6.0: apache/maven-dependency-plugin@maven-dependency-plugin-3.5.0...maven-dependency-plugin-3.6.0

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA.

Closes #41641 from panbingkun/SPARK-44085.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
czxm pushed a commit to czxm/spark that referenced this issue Jun 19, 2023
### What changes were proposed in this pull request?
The pr aims to update some maven plugins to newest version. include:
- versions-maven-plugin from 2.15.0 to 2.16.0
- maven-source-plugin from 3.2.1 to 3.3.0
- maven-surefire-plugin from 3.1.0 to 3.1.2
- maven-dependency-plugin from 3.5.0 to 3.6.0

### Why are the changes needed?
- versions-maven-plugin
1.Release Notes: https://github.com/mojohaus/versions/releases/tag/2.16.0
2.Bug Fix:
Resolves: display-dependency-updates only shows updates from the most major allowed segment (mojohaus/versions#966) ajarmoniuk
Resolves mojohaus/versions#931: Fixing problems with encoding in UseDepVersion and PomHelper (mojohaus/versions#932) ajarmoniuk
Resolves mojohaus/versions#916: Partially reverted mojohaus/versions#799. (mojohaus/versions#924) ajarmoniuk
Resolves mojohaus/versions#954: Excluded plexus-container-default (mojohaus/versions#955) ajarmoniuk
Resolves mojohaus/versions#951: DefaultArtifactVersion::getVersion can be null (mojohaus/versions#952) ajarmoniuk
BoundArtifactVersion.toString() to work with NumericVersionComparator (mojohaus/versions#930) ajarmoniuk
Issue mojohaus/versions#925: Protect against an NPE if a dependency version is defined in dependencyManagement (mojohaus/versions#926) ajarmoniuk

- maven-source-plugin
v3.2.1 VS v3.3.0: apache/maven-source-plugin@maven-source-plugin-3.2.1...maven-source-plugin-3.3.0

- maven-surefire-plugin
Release Notes: https://github.com/apache/maven-surefire/releases/tag/surefire-3.1.2

- maven-dependency-plugin
v3.5.0 VS v3.6.0: apache/maven-dependency-plugin@maven-dependency-plugin-3.5.0...maven-dependency-plugin-3.6.0

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA.

Closes apache#41641 from panbingkun/SPARK-44085.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants