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

Upgrade should follow Maven relocations #227

Open
pmonks opened this issue Jun 20, 2023 · 4 comments
Open

Upgrade should follow Maven relocations #227

pmonks opened this issue Jun 20, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@pmonks
Copy link
Contributor

pmonks commented Jun 20, 2023

Steps to reproduce:

  1. In an empty directory, create a deps.edn file with this content:
{:deps {mysql/mysql-connector-java {:mvn/version "8.0.30"}}
 :aliases {:outdated {
             :deps {com.github.liquidz/antq {:mvn/version "RELEASE"}}
             :main-opts ["-m" "antq.core"]}}}
  1. Run clj- P and observe that it successfully downloads the referenced library
  2. Run clj -A:outdated and observe that the dependency is out of date (at the time of writing, the latest version is 8.0.33)
  3. Run clj -A:outdated --upgrade --force

Expected result:
The dependency's GAV in the deps.edn file is modified from mysql/mysql-connector-java {:mvn/version "8.0.30"} to com.mysql/mysql-connector-j {:mvn/version "8.0.33"}, since the pom.xml for mysql/mysql-connector-java {:mvn/version "8.0.33"} contains this relocation information:

  <distributionManagement>
    <relocation>
      <groupId>com.mysql</groupId>
      <artifactId>mysql-connector-j</artifactId>
      <message>MySQL Connector/J artifacts moved to reverse-DNS compliant Maven 2+ coordinates.</message>
    </relocation>
  </distributionManagement>

Actual result:
The dependency's GAV in the deps.edn file only has its version updated (to 8.0.33), which will then fail when downloaded (e.g. via clj -P) since there is no Maven artifact for mysql/mysql-connector-java {:mvn/version "8.0.33"} (due to the relocation).

@liquidz
Copy link
Owner

liquidz commented Jun 23, 2023

@pmonks Thank you for your reporting!

It is impossible to know which versions have relocation unless we check.
Therefore, we would need to check for the existence of relocation for all outdated dependencies, but it seems to be quite wasteful.

I'm planning to implement it initially as an optional feature, and then continue to search for a more efficient way of checking.

@pmonks
Copy link
Contributor Author

pmonks commented Jun 23, 2023

No worries! It was actually another Clojure user who pointed this out to me, though I don't know their GitHub handle to @mention them. Regardless they deserve the credit!

Given that upgrades break a previously functional build when a relocation is present, I think the performance cost of checking for them is probably the lesser of two evils (especially as I suspect that cost isn't very high per outdated dependency).

@liquidz liquidz added the enhancement New feature or request label Jun 24, 2023
@liquidz
Copy link
Owner

liquidz commented Jun 25, 2023

@pmonks

Given that upgrades break a previously functional build when a relocation is present, I think the performance cost of checking for them is probably the lesser of two evils (especially as I suspect that cost isn't very high per outdated dependency).

You are right.
For now, I've tried to always check.
Could you try feature/relocation branch?

@pmonks
Copy link
Contributor Author

pmonks commented Aug 14, 2023

I'm sorry but I haven't had the opportunity to try that branch, and probably won't for the foreseeable future. Perhaps it could be placed behind a command line flag and released, so that the wider community can help test it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants