Maven
  1. Maven
  2. MNG-3057

properties not expanded in generated POMs when building A/B/C nested projects

    Details

      Description

      Using Maven version: 2.0.8-SNAPSHOT, svn r547427.

      I checked out and built 2.0.8 because I was interested in Jason van Zyl's patch for MNG-2619- "building from the middle pom of a (parent,child,grandchild) heirarchy fails". The fix works for the most part, but there still seems to be a problem with the poms that get generated during the install goal. The poms that get installed into .m2/repository do not have properties interpolated. If I run maven like:

      $ mvn install -Dglobal-version=1.0.0

      I get poms with the string "$

      {global-version}

      " embedded in the resulting poms rather than what I specify on the command line (or in settings.xml). The build works properly aside from this, and if I do "mvn help:effective-pom" the properties are correctly interpolated.

      I've attached a tarfile with an example A/B/C build structure that exhibits this behavior, as well as the generated poms.

      Many thanks for your attention.

      1. example.tar.gz
        6 kB
      2. generated-poms.tar.gz
        2 kB
      3. InstallMojo.java.patch
        4 kB
        Henrik Brautaset Aronsen
      4. InstallMojo.java.patch
        3 kB
        Henrik Brautaset Aronsen
      5. InstallMojo.quoteReplacement.patch
        0.6 kB
        Henrik Brautaset Aronsen
      6. maven-dependency-problem.zip
        2 kB
        Henrik Brautaset Aronsen
      7. maven-install-parent.patch
        8 kB
        Henrik Brautaset Aronsen

        Issue Links

          Activity

          Hide
          Henrik Brautaset Aronsen added a comment -

          I added another test case for this problem in MNG-2446.

          Show
          Henrik Brautaset Aronsen added a comment - I added another test case for this problem in MNG-2446 .
          Hide
          Henrik Brautaset Aronsen added a comment -

          InstallMojo.java.patch is a patch against revision 672740 of maven-install-plugin/src/main/java/org/apache/maven/plugin/install/InstallMojo.java. It's not very pretty, but it fixes this problem.

          How to apply and build:

          $ svn checkout http://svn.apache.org/repos/asf/maven/plugins/trunk/maven-install-plugin maven-install-plugin
          $ cd maven-install-plugin/src/main/java/org/apache/maven/plugin/install
          $ patch < InstallMojo.java.patch
          $ cd ../../../../../../../..
          $ mvn clean install
          
          Show
          Henrik Brautaset Aronsen added a comment - InstallMojo.java.patch is a patch against revision 672740 of maven-install-plugin/src/main/java/org/apache/maven/plugin/install/InstallMojo.java. It's not very pretty, but it fixes this problem. How to apply and build: $ svn checkout http: //svn.apache.org/repos/asf/maven/plugins/trunk/maven-install-plugin maven-install-plugin $ cd maven-install-plugin/src/main/java/org/apache/maven/plugin/install $ patch < InstallMojo.java.patch $ cd ../../../../../../../.. $ mvn clean install
          Hide
          Harsha Godugu added a comment - - edited

          Hello:

          The above patch does not work, if the global-version property is defined outside pom.xml or outside settings.xml . In other words, if you define like :

          $ mvn install -Dglobal-version=1.0.0

          in maven 2.0.9 it does not reflect the property "global-version" into the project properties (another existing bug against maven).
          Hence, the generated pom still will have stale $

          {global-version}

          as value.

          As you can guess obviously, one need to add a few lines of code to fix the bug. Check property against System.getProperty(key) to see if there is a global property defined from command line...

          Here is a partial diff:

          + while (m.find()) {
          + String match = m.group();
          + String key = match.substring(2, match.length()-1);
          + String value = properties.getProperty(key);

          + if (value == null)

          { + System.out.println("Trying .. system wide property.."); + value = System.getProperty(key); + if (value !=null) + System.out.println("Found value from sys=" + value); + }

          + if (value != null) {
          + s = s.replaceAll("\\$

          {" + key + "\\}

          ", value);
          + }

          Show
          Harsha Godugu added a comment - - edited Hello: The above patch does not work, if the global-version property is defined outside pom.xml or outside settings.xml . In other words, if you define like : $ mvn install -Dglobal-version=1.0.0 in maven 2.0.9 it does not reflect the property "global-version" into the project properties (another existing bug against maven). Hence, the generated pom still will have stale $ {global-version} as value. As you can guess obviously, one need to add a few lines of code to fix the bug. Check property against System.getProperty(key) to see if there is a global property defined from command line... Here is a partial diff: + while (m.find()) { + String match = m.group(); + String key = match.substring(2, match.length()-1); + String value = properties.getProperty(key); + if (value == null) { + System.out.println("Trying .. system wide property.."); + value = System.getProperty(key); + if (value !=null) + System.out.println("Found value from sys=" + value); + } + if (value != null) { + s = s.replaceAll("\\$ {" + key + "\\} ", value); + }
          Hide
          Henrik Brautaset Aronsen added a comment - - edited

          I've updated the patch. System properties are taken into account as well now.

          Show
          Henrik Brautaset Aronsen added a comment - - edited I've updated the patch . System properties are taken into account as well now.
          Hide
          Maxim Petrashev added a comment - - edited

          Hi, Henrik

          Can you, please,

          • add supporting of nested values? I mean if we have property:
            someProperty1=/somepath/somesubpath/$ {otherPropery2}

            /something
            then it will fail with
            java.lang.IllegalArgumentException: Illegal group reference
            at java.util.regex.Matcher.appendReplacement(Matcher.java:706)
            at java.util.regex.Matcher.replaceAll(Matcher.java:806)
            at java.lang.String.replaceAll(String.java:2000)
            at org.apache.maven.plugin.install.InstallMojo.insertProperties(InstallMojo.java:118)
            where my InstallMojo.java:118 line is:
            s = s.replaceAll("\\$

            {" + key + "\\}

            ", value);

          • extend mask for "." sign in property names? In you case {myComponent-version}

            works fine, but

            {myComponent.version}

            doesn't work

          • I see just 1 place for properties replacement:
            if ( isPomArtifact ) { installer.install( insertProperties(pomFile), artifact, localRepository ); }

            how about else statement?
            else
            {
            metadata = new ProjectArtifactMetadata( artifact, pomFile );
            I believe we have to change it on
            metadata = new ProjectArtifactMetadata( artifact, insertProperties(pomFile) );
            too?

          • Am I right that we need to extend deploy plugin too to make it configurable for pomFile property? Otherwise, original pom.xml will be deployed, not target/maven-install-plugin/pom.xml

          Many thanks for your efforts! Does maven team have any plans to include this patch in the next release?

          Show
          Maxim Petrashev added a comment - - edited Hi, Henrik Can you, please, add supporting of nested values? I mean if we have property: someProperty1=/somepath/somesubpath/$ {otherPropery2} /something then it will fail with java.lang.IllegalArgumentException: Illegal group reference at java.util.regex.Matcher.appendReplacement(Matcher.java:706) at java.util.regex.Matcher.replaceAll(Matcher.java:806) at java.lang.String.replaceAll(String.java:2000) at org.apache.maven.plugin.install.InstallMojo.insertProperties(InstallMojo.java:118) where my InstallMojo.java:118 line is: s = s.replaceAll("\\$ {" + key + "\\} ", value); extend mask for "." sign in property names? In you case {myComponent-version} works fine, but {myComponent.version} doesn't work I see just 1 place for properties replacement: if ( isPomArtifact ) { installer.install( insertProperties(pomFile), artifact, localRepository ); } how about else statement? else { metadata = new ProjectArtifactMetadata( artifact, pomFile ); I believe we have to change it on metadata = new ProjectArtifactMetadata( artifact, insertProperties(pomFile) ); too? Am I right that we need to extend deploy plugin too to make it configurable for pomFile property? Otherwise, original pom.xml will be deployed, not target/maven-install-plugin/pom.xml Many thanks for your efforts! Does maven team have any plans to include this patch in the next release?
          Hide
          Henrik Brautaset Aronsen added a comment -

          Hi Maxim.

          I'm attaching maven-install-parent.patch, which is the one I've been using locally for awhile. It fixes dots in property names and the properties replacements you mentioned, as well as the failing unit tests.

          I am not using nested properties and mvn deploy myself, and haven't given that much thought – sorry about that.

          I don't think the maven team have any plans of including this patch, but there's ongoing work in MNG-624 to do something simliar which might be included in Maven 2.1.0-M6 .

          Show
          Henrik Brautaset Aronsen added a comment - Hi Maxim. I'm attaching maven-install-parent.patch , which is the one I've been using locally for awhile. It fixes dots in property names and the properties replacements you mentioned, as well as the failing unit tests. I am not using nested properties and mvn deploy myself, and haven't given that much thought – sorry about that. I don't think the maven team have any plans of including this patch, but there's ongoing work in MNG-624 to do something simliar which might be included in Maven 2.1.0-M6 .
          Hide
          Henrik Brautaset Aronsen added a comment -

          Maxim: Here's a quick fix for the Illegal group reference bug (nested properties): InstallMojo.quoteReplacement.patch

          Show
          Henrik Brautaset Aronsen added a comment - Maxim: Here's a quick fix for the Illegal group reference bug (nested properties): InstallMojo.quoteReplacement.patch
          Hide
          Brett Porter added a comment -

          review as a popular issue

          Show
          Brett Porter added a comment - review as a popular issue
          Hide
          John Casey added a comment -

          Consolidating to 2.1.0-M1 so we can then rename to 2.1.0. We can weed out any issues we want to push to a later release from this set once we've done the consolidation.

          Show
          John Casey added a comment - Consolidating to 2.1.0-M1 so we can then rename to 2.1.0. We can weed out any issues we want to push to a later release from this set once we've done the consolidation.
          Hide
          John Casey added a comment -

          We need to do more design work/discussion on this point, since interpolating versions before deployment must happen selectively, if at all. Additionally, if the POM has packaging == "pom" it may be an organizational POM, which could conceivably mean that versions made up of multiple expressions could be meant to allow those parts to vary in child POMs and be interpreted correctly. If that were true, then interpolating these versions on deployment of the org POM would violate that expectation.

          This seems related to the practice of using things like $

          {wagonVersion}

          in POMs where multiple related dependencies are specified in a parent POM. In most cases, these versions are specified in an application's parent POM, which means they should NOT vary in child POMs...so making them concrete on deployment could actually be a good thing. But I'm not sure we've got the use cases covered well enough to make this change.

          Show
          John Casey added a comment - We need to do more design work/discussion on this point, since interpolating versions before deployment must happen selectively, if at all. Additionally, if the POM has packaging == "pom" it may be an organizational POM, which could conceivably mean that versions made up of multiple expressions could be meant to allow those parts to vary in child POMs and be interpreted correctly. If that were true, then interpolating these versions on deployment of the org POM would violate that expectation. This seems related to the practice of using things like $ {wagonVersion} in POMs where multiple related dependencies are specified in a parent POM. In most cases, these versions are specified in an application's parent POM, which means they should NOT vary in child POMs...so making them concrete on deployment could actually be a good thing. But I'm not sure we've got the use cases covered well enough to make this change.
          Hide
          John Casey added a comment -

          moving back into 2.1.0 bucket for discussions related to MDEPLOY-94, etc. Maybe we should take a shot at this and start climbing the learning curve now in terms of use cases.

          Show
          John Casey added a comment - moving back into 2.1.0 bucket for discussions related to MDEPLOY-94 , etc. Maybe we should take a shot at this and start climbing the learning curve now in terms of use cases.
          Hide
          John Casey added a comment -

          The fix will interpolate version expressions for parent, project, dependencies, dependencyManagement, plugins, pluginManagement, plugin-level dependencies, and reporting plugins. It will leave all other fields alone and uninterpolated.

          Show
          John Casey added a comment - The fix will interpolate version expressions for parent, project, dependencies, dependencyManagement, plugins, pluginManagement, plugin-level dependencies, and reporting plugins. It will leave all other fields alone and uninterpolated.
          Hide
          John Casey added a comment -

          POM version-expression transformer is removing comments, because it's using the XPP3 reader/writer. I'm not sure how to work around this yet.

          Show
          John Casey added a comment - POM version-expression transformer is removing comments, because it's using the XPP3 reader/writer. I'm not sure how to work around this yet.
          Hide
          Benjamin Bentmann added a comment -

          John, the removal of the license header was a regression of MNG-2820 for which I just added a (disabled) IT.

          Show
          Benjamin Bentmann added a comment - John, the removal of the license header was a regression of MNG-2820 for which I just added a (disabled) IT.
          Hide
          John Casey added a comment -

          Thanks, Benjamin, that saved me some time.

          Show
          John Casey added a comment - Thanks, Benjamin, that saved me some time.
          Hide
          John Casey added a comment -

          Added new interpolation strategy that uses raw contents of the POM XML file, and interpolates it from that string. This is a two-stage process, where the first stage interpolator isolates the version elements and hands the value off to the second-stage interpolator to actually resolve any expressions. Then, the result is re-inserted into the XML.

          Also, adjusted the handling for artifacts with type == pom during install/deploy, such that the accompanying file is forcibly set to the artifact's file, to work around MCOMPILER-94. Attachments should be handled separately, since they have their own Artifact instance. I've modified the integration test for MNG-3057 to inject the configuration from MCOMPILER-94 just to be sure it works.

          Show
          John Casey added a comment - Added new interpolation strategy that uses raw contents of the POM XML file, and interpolates it from that string. This is a two-stage process, where the first stage interpolator isolates the version elements and hands the value off to the second-stage interpolator to actually resolve any expressions. Then, the result is re-inserted into the XML. Also, adjusted the handling for artifacts with type == pom during install/deploy, such that the accompanying file is forcibly set to the artifact's file, to work around MCOMPILER-94 . Attachments should be handled separately, since they have their own Artifact instance. I've modified the integration test for MNG-3057 to inject the configuration from MCOMPILER-94 just to be sure it works.
          Hide
          Henrik Brautaset Aronsen added a comment -

          This issue is still not fixed in 2.1.0.

          Steps to reproduce:

          The project layout is this:

          first--+--second----third
                 |
                 +--fourth
          

          "third" has "second" as parent version, and "second"/"fourth" has "first" as parent version. They all use a common applicationVersion property, which is set in "first". "fourth" has a dependency on "third", which resolves OK, but it fails to resolve the transitive dependency to "second":

          [INFO] Failed to resolve artifact.
          
          GroupId: me
          ArtifactId: second
          Version: ${applicationVersion}
          
          Reason: Unable to download the artifact from any repository
          
            me:second:pom:${applicationVersion}
          
          Show
          Henrik Brautaset Aronsen added a comment - This issue is still not fixed in 2.1.0. Steps to reproduce: download and unzip maven-dependency-problem.zip cd first mvn clean install cd fourth mvn clean install (which fails) The project layout is this: first--+--second----third | +--fourth "third" has "second" as parent version, and "second"/"fourth" has "first" as parent version. They all use a common applicationVersion property, which is set in "first". "fourth" has a dependency on "third", which resolves OK, but it fails to resolve the transitive dependency to "second": [INFO] Failed to resolve artifact. GroupId: me ArtifactId: second Version: ${applicationVersion} Reason: Unable to download the artifact from any repository me:second:pom:${applicationVersion}
          Hide
          Benjamin Bentmann added a comment -

          The code that was introduced for this fix was removed from Maven 2.2+ due to reasons outlined in MNG-4223.

          Show
          Benjamin Bentmann added a comment - The code that was introduced for this fix was removed from Maven 2.2+ due to reasons outlined in MNG-4223 .

            People

            • Assignee:
              John Casey
              Reporter:
              George Armhold
            • Votes:
              9 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development