GMaven
  1. GMaven
  2. GMAVEN-46

Properties overridden on the command line not recognized

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: 1.3
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      0

      Description

      I have the following property in my pom:

      <properties>
        <artifactsDir>${project.build.directory}</artifactsDir>
      </properties>
      

      This property may be overridden on the command line using -DartifactsDir=<...>. This works everywhere in my build, but not for the GMaven plugin. It still returns (i. e. interprets) the value configured in the pom ({{$

      {project.build.directory}

      }}) when I access it using project.properties.artifactsDir.

        Activity

        Hide
        John Sanda added a comment -

        I just ran into this bug. Since I tend to use the gmaven plugin quite a bit, I checked out the trunk at rev 90 to see about submitting a patch. I first reproduced the bug in one of the integration tests. Here is a diff of my changes to reproduce the bug,

        Index: gmaven-testsuite/src/it/groovy-1.7/compile/pom.xml
        ===================================================================
        — gmaven-testsuite/src/it/groovy-1.7/compile/pom.xml (revision 90)
        +++ gmaven-testsuite/src/it/groovy-1.7/compile/pom.xml (working copy)
        @@ -36,6 +36,7 @@

        <properties>
        <groovy_version>1.7</groovy_version>
        + <myOverride>foo</myOverride>
        </properties>

        <dependencies>
        @@ -104,6 +105,9 @@
        assert expect

        assert version.startsWith(expect)
        +
        + myOverride = pom.properties.myOverride
        + assert myOverride == 'bar', "Wrong value for myOverride"
        </source>
        </configuration>
        </execution>
        Index: gmaven-testsuite/src/it/groovy-1.7/compile/test.properties
        ===================================================================
        — gmaven-testsuite/src/it/groovy-1.7/compile/test.properties (revision 0)
        +++ gmaven-testsuite/src/it/groovy-1.7/compile/test.properties (revision 0)
        @@ -0,0 +1 @@
        +myOverride=bar
        \ No newline at end of file

        And here is a diff of my fix,

        Index: gmaven-plugin/src/main/java/org/codehaus/gmaven/plugin/execute/GroovyMavenProjectAdapter.java
        ===================================================================
        — gmaven-plugin/src/main/java/org/codehaus/gmaven/plugin/execute/GroovyMavenProjectAdapter.java (revision 90)
        +++ gmaven-plugin/src/main/java/org/codehaus/gmaven/plugin/execute/GroovyMavenProjectAdapter.java (working copy)
        @@ -107,8 +107,12 @@
        //

        public Object lookup(final Object key) {
        + Object value = System.getProperty(key.toString());
        +
        // First try our self (pom + custom)

        • Object value = super.get(key);
          + if (value == null) { + value = super.get(key); + }

        // Then try execution (system) properties
        if (value == null) {

        Jason or whoever else has commit access can you please review my patch when you have a chance? This is my first time actually looking at the code for gmaven so it certainly would not surprise that there might be a better, more preferred approach to resolving this. This bug significantly impacts my usage of gmaven so I am more than happy to do what I can to help push a fix.

        Thanks

        • John
        Show
        John Sanda added a comment - I just ran into this bug. Since I tend to use the gmaven plugin quite a bit, I checked out the trunk at rev 90 to see about submitting a patch. I first reproduced the bug in one of the integration tests. Here is a diff of my changes to reproduce the bug, Index: gmaven-testsuite/src/it/groovy-1.7/compile/pom.xml =================================================================== — gmaven-testsuite/src/it/groovy-1.7/compile/pom.xml (revision 90) +++ gmaven-testsuite/src/it/groovy-1.7/compile/pom.xml (working copy) @@ -36,6 +36,7 @@ <properties> <groovy_version>1.7</groovy_version> + <myOverride>foo</myOverride> </properties> <dependencies> @@ -104,6 +105,9 @@ assert expect assert version.startsWith(expect) + + myOverride = pom.properties.myOverride + assert myOverride == 'bar', "Wrong value for myOverride" </source> </configuration> </execution> Index: gmaven-testsuite/src/it/groovy-1.7/compile/test.properties =================================================================== — gmaven-testsuite/src/it/groovy-1.7/compile/test.properties (revision 0) +++ gmaven-testsuite/src/it/groovy-1.7/compile/test.properties (revision 0) @@ -0,0 +1 @@ +myOverride=bar \ No newline at end of file And here is a diff of my fix, Index: gmaven-plugin/src/main/java/org/codehaus/gmaven/plugin/execute/GroovyMavenProjectAdapter.java =================================================================== — gmaven-plugin/src/main/java/org/codehaus/gmaven/plugin/execute/GroovyMavenProjectAdapter.java (revision 90) +++ gmaven-plugin/src/main/java/org/codehaus/gmaven/plugin/execute/GroovyMavenProjectAdapter.java (working copy) @@ -107,8 +107,12 @@ // public Object lookup(final Object key) { + Object value = System.getProperty(key.toString()); + // First try our self (pom + custom) Object value = super.get(key); + if (value == null) { + value = super.get(key); + } // Then try execution (system) properties if (value == null) { Jason or whoever else has commit access can you please review my patch when you have a chance? This is my first time actually looking at the code for gmaven so it certainly would not surprise that there might be a better, more preferred approach to resolving this. This bug significantly impacts my usage of gmaven so I am more than happy to do what I can to help push a fix. Thanks John

          People

          • Assignee:
            Jason Dillon
            Reporter:
            Reinhard Nägele
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: