Pom properties are not evaluated #197

lread · 2023-01-17T18:53:18Z

While checking pomegranate for old deps with antq, I noticed that when versions in pom.xml files come from pom properties, these properties are not evaluated.

I'm not suggesting that you fix this, that is, of course, entirely your choice, just thought you would be interested.

Reproduction

Create a pom.xml in an empty directory:

<?xml version="1.0" encoding="UTF-8"?>
<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/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>lread-repro</groupId>
  <artifactId>for-antq</artifactId>
  <version>1.2.3</version>

  <properties>
    <clojure.version>1.4.0</clojure.version>
    <mavenVersion>3.8.6</mavenVersion>
    <resolverVersion>1.9.4</resolverVersion>
  </properties>

  <dependencies>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>clojure</artifactId>
      <version>${clojure.version}</version>
    </dependency>
    <dependency>
      <groupId>org.apache.maven</groupId>
      <artifactId>maven-resolver-provider</artifactId>
      <version>${mavenVersion}</version>
    </dependency>
    <dependency>
      <groupId>org.apache.maven.resolver</groupId>
      <artifactId>maven-resolver-api</artifactId>
      <version>${resolverVersion}</version>
    </dependency>
    <dependency>
      <groupId>org.tcrawley</groupId>
      <artifactId>dynapath</artifactId>
      <version>1.0.0</version>
    </dependency>
  </dependencies>
  <repositories>
    <repository>
      <id>clojars</id>
      <url>https://repo.clojars.org/</url>
    </repository>
  </repositories>
</project>

From that directory, run antq:

clojure -Sdeps '{:deps {com.github.liquidz/antq {:mvn/version "RELEASE"}}}' -M -m antq.core

Actual output

|   :file |                                        :name |           :current | :latest |
|---------+----------------------------------------------+--------------------+---------|
| pom.xml | org.apache.maven.resolver/maven-resolver-api | ${resolverVersion} |   1.9.4 |
|         |     org.apache.maven/maven-resolver-provider |    ${mavenVersion} |   3.8.7 |
|         |                          org.clojure/clojure | ${clojure.version} |  1.11.1 |
|         |                        org.tcrawley/dynapath |              1.0.0 |   1.1.0 |

Expected output

I would expect antq to compare against and present the actual property values:

|   :file |                                        :name |           :current | :latest |
|---------+----------------------------------------------+--------------------+---------|
| pom.xml |     org.apache.maven/maven-resolver-provider |              3.8.6 |   3.8.7 |
|         |                          org.clojure/clojure |              1.4.0 |  1.11.1 |
|         |                        org.tcrawley/dynapath |              1.0.0 |   1.1.0 |

Note that org.apache.maven.resolver/maven-resolver-api would not be presented, because, at the time of this writing, the current version is 1.9.4.

Observation

If, from the same dir, I ask clojure to show the deps:

clojure -Sdeps '{:deps {testing123/testing123 {:local/root "." :deps/manifest :pom}}}' -Stree

We can see that it resolves the maven properties:

org.clojure/clojure 1.11.1
  . org.clojure/spec.alpha 0.3.218
  . org.clojure/core.specs.alpha 0.2.62
testing123/testing123 /home/lee/proj/oss/-verify/ant-repo
  . org.apache.maven/maven-resolver-provider 3.8.6
    . org.apache.maven/maven-model 3.8.6
      . org.codehaus.plexus/plexus-utils 3.3.1
    . org.apache.maven/maven-model-builder 3.8.6
      . org.codehaus.plexus/plexus-utils 3.3.1
      . org.codehaus.plexus/plexus-interpolation 1.26
      . javax.inject/javax.inject 1
      . org.apache.maven/maven-model 3.8.6
      . org.apache.maven/maven-artifact 3.8.6
        . org.codehaus.plexus/plexus-utils 3.3.1
        . org.apache.commons/commons-lang3 3.8.1
      . org.apache.maven/maven-builder-support 3.8.6
      . org.eclipse.sisu/org.eclipse.sisu.inject 0.3.5
    . org.apache.maven/maven-repository-metadata 3.8.6
      . org.codehaus.plexus/plexus-utils 3.3.1
    X org.apache.maven.resolver/maven-resolver-api 1.6.3 :older-version
    . org.apache.maven.resolver/maven-resolver-spi 1.6.3
      X org.apache.maven.resolver/maven-resolver-api 1.6.3 :older-version
    . org.apache.maven.resolver/maven-resolver-util 1.6.3
      X org.apache.maven.resolver/maven-resolver-api 1.6.3 :older-version
    . org.apache.maven.resolver/maven-resolver-impl 1.6.3
      X org.apache.maven.resolver/maven-resolver-api 1.6.3 :older-version
      . org.apache.maven.resolver/maven-resolver-spi 1.6.3
      . org.apache.maven.resolver/maven-resolver-util 1.6.3
      . org.apache.commons/commons-lang3 3.8.1
      . org.slf4j/slf4j-api 1.7.30
    . org.codehaus.plexus/plexus-utils 3.3.1
    . javax.inject/javax.inject 1
  . org.apache.maven.resolver/maven-resolver-api 1.9.4
  . org.tcrawley/dynapath 1.0.0

So maybe tools.deps will be of some help here.
Note also, as I understand it, that properties can come from parent poms.

The text was updated successfully, but these errors were encountered:

liquidz · 2023-01-17T21:25:11Z

@lread Thanks for your reporting!

So maybe tools.deps will be of some help here.
Note also, as I understand it, that properties can come from parent poms.

Great. I'll have a look.

liquidz · 2023-01-18T08:30:51Z

This multimethod resolves properties in pom.xml
https://github.com/clojure/tools.deps/blob/6ae2b6f71773de7549d7f22759e8b09fec27f0d9/src/main/clojure/clojure/tools/deps/extensions/pom.clj#L96-L100

lread · 2023-01-20T14:58:39Z

This multimethod resolves properties in pom.xml https://github.com/clojure/tools.deps/blob/6ae2b6f71773de7549d7f22759e8b09fec27f0d9/src/main/clojure/clojure/tools/deps/extensions/pom.clj#L96-L100

Hiya @liquidz! I'm not sure if you are making a note for yourself or asking me to look into something here.

liquidz · 2023-01-20T20:49:19Z

@lread Oh sorry. It's just a note for me :)

cf. #197

liquidz · 2023-01-23T21:28:40Z

@lread Sorry for late fixing.
I've fixed this problem in dev branch, and upgrading versions managed by properties should work also. (Of course versions managed in parent pom will be skipped)

Could you try dev branch?

liquidz · 2023-01-23T21:31:41Z

Hmm? Something may be wrong with upgrading pom.

lread · 2023-01-24T17:15:43Z

@lread Sorry for late fixing.

Hey, I'm in absolutely no hurry, thanks for looking into this.

Could you try dev branch?

Sure, will do sometime today and get back to you.

lread · 2023-01-24T19:44:25Z

Ok, only did minimal testing but against my repro above from antq dev branch:

Retry of reported reproduction

❯ clojure -M -m antq.core --directory ../../-verify/ant-repo 
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
[##################################################] 24/24

|                          :file |                                    :name | :current | :latest |
|--------------------------------+------------------------------------------+----------+---------|
| ../../-verify/ant-repo/pom.xml | org.apache.maven/maven-resolver-provider |    3.8.6 |   3.8.7 |
|                                |                      org.clojure/clojure |    1.4.0 |  1.11.1 |
|                                |                    org.tcrawley/dynapath |    1.0.0 |   1.1.0 |

Available changes:
- https://github.com/clojure/clojure/blob/clojure-1.11.1/changes.md
- https://github.com/tobias/dynapath/blob/1.1.0/Changelog.md

Looks good!

Pomegranate

Now

And if I hit pomegranate:

❯ clojure -M -m antq.core --directory ../../-verify/pomegranate 
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
[##################################################] 57/57

|                              :file |                :name |             :current |                         :latest |
|------------------------------------+----------------------+----------------------+---------------------------------|
| ../../-verify/pomegranate/deps.edn |  clj-kondo/clj-kondo |           2023.01.16 |                      2023.01.20 |
|                                    | seancorfield/depstar | seancorfield/depstar | com.github.seancorfield/depstar |
|  ../../-verify/pomegranate/pom.xml |  org.clojure/clojure |                1.4.0 |                          1.11.1 |

Available changes:
- https://github.com/clj-kondo/clj-kondo/blob/v2023.01.20/CHANGELOG.md
- https://github.com/clojure/clojure/blob/clojure-1.11.1/changes.md

Looks good!

Previously

Compare with old behaviour:

❯ clojure -Sdeps '{:deps {com.github.liquidz/antq {:mvn/version "RELEASE"}}}' -M -m antq.core
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
[##################################################] 41/41

|    :file |                                                    :name |             :current |                         :latest |
|----------+----------------------------------------------------------+----------------------+---------------------------------|
| deps.edn |                                      clj-kondo/clj-kondo |           2023.01.16 |                      2023.01.20 |
|          |                                     seancorfield/depstar | seancorfield/depstar | com.github.seancorfield/depstar |
|  pom.xml |             org.apache.maven.resolver/maven-resolver-api |   ${resolverVersion} |                           1.9.4 |
|          | org.apache.maven.resolver/maven-resolver-connector-basic |   ${resolverVersion} |                           1.9.4 |
|          |            org.apache.maven.resolver/maven-resolver-impl |   ${resolverVersion} |                           1.9.4 |
|          |             org.apache.maven.resolver/maven-resolver-spi |   ${resolverVersion} |                           1.9.4 |
|          |  org.apache.maven.resolver/maven-resolver-transport-file |   ${resolverVersion} |                           1.9.4 |
|          |  org.apache.maven.resolver/maven-resolver-transport-http |   ${resolverVersion} |                           1.9.4 |
|          | org.apache.maven.resolver/maven-resolver-transport-wagon |   ${resolverVersion} |                           1.9.4 |
|          |            org.apache.maven.resolver/maven-resolver-util |   ${resolverVersion} |                           1.9.4 |
|          |                        org.apache.maven.wagon/wagon-http |      ${wagonVersion} |                           3.5.3 |
|          |                org.apache.maven.wagon/wagon-provider-api |      ${wagonVersion} |                           3.5.3 |
|          |                         org.apache.maven.wagon/wagon-ssh |      ${wagonVersion} |                           3.5.3 |
|          |                 org.apache.maven/maven-resolver-provider |      ${mavenVersion} |                           3.8.7 |
|          |                                      org.clojure/clojure |   ${clojure.version} |                          1.11.1 |

Available changes:
- https://github.com/clj-kondo/clj-kondo/blob/v2023.01.20/CHANGELOG.md

Did not Try...

Upgrading... that's not something I personally use antq for.

liquidz · 2023-01-24T21:19:41Z

@lread Thanks for your confirmation!

liquidz · 2023-01-24T21:28:13Z

Just released 2.2.983 #198

liquidz added the bug Something isn't working label Jan 17, 2023

liquidz added a commit that referenced this issue Jan 21, 2023

fix: Fix pom deps detector to use tools.deps.extensions.pom

7afd1db

cf. #197

liquidz added a commit that referenced this issue Jan 21, 2023

fix: Fix pom upgrader to skip versions managed by properties

ea456e7

cf. #197

liquidz added a commit that referenced this issue Jan 22, 2023

fix: Update to be able to upgrade property managed versions in POM

18f8afb

cf. #197

liquidz added a commit that referenced this issue Jan 22, 2023

fix: Update to be able to upgrade property managed versions in POM

300d578

cf. #197

liquidz added a commit that referenced this issue Jan 22, 2023

test: Add test for parent-child pom

bd205bf

cf. #197

liquidz added a commit that referenced this issue Jan 23, 2023

test: Add test for upgrading child POM

f71105c

cf. #197

liquidz closed this as completed Jan 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pom properties are not evaluated #197

Pom properties are not evaluated #197

lread commented Jan 17, 2023

liquidz commented Jan 17, 2023

liquidz commented Jan 18, 2023

lread commented Jan 20, 2023

liquidz commented Jan 20, 2023

liquidz commented Jan 23, 2023

liquidz commented Jan 23, 2023

lread commented Jan 24, 2023

lread commented Jan 24, 2023

liquidz commented Jan 24, 2023

liquidz commented Jan 24, 2023

Pom properties are not evaluated #197

Pom properties are not evaluated #197

Comments

lread commented Jan 17, 2023

Reproduction

Actual output

Expected output

Observation

liquidz commented Jan 17, 2023

liquidz commented Jan 18, 2023

lread commented Jan 20, 2023

liquidz commented Jan 20, 2023

liquidz commented Jan 23, 2023

liquidz commented Jan 23, 2023

lread commented Jan 24, 2023

lread commented Jan 24, 2023

Retry of reported reproduction

Pomegranate

Now

Previously

Did not Try...

liquidz commented Jan 24, 2023

liquidz commented Jan 24, 2023