liquidz / antq

Point out your outdated dependencies.
Other
395 stars 22 forks source link

Pom properties are not evaluated #197

Closed lread closed 1 year ago

lread commented 1 year ago

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.

liquidz commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

liquidz commented 1 year ago

@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 commented 1 year ago

Hmm? Something may be wrong with upgrading pom.

lread commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@lread Thanks for your confirmation!

liquidz commented 1 year ago

Just released 2.2.983 #198