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

Fix Maven inability to overwrite repository urls by ID #5878

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

jakecoffman
Copy link
Member

In Maven, it is possible to overwrite repository URLs defined elsewhere by defining a repository with the same ID, as long as the file has precedence according to the order defined in the docs.

This is often used to define a different central repository to replace the default one inherited from the Super POM:

<repositories>
    <repository>
      <id>central</id>
      <url>https://example.com</url>
      <snapshots>
        <enabled>false</enabled>
      </snapshots>
    </repository>
  </repositories>

In this PR, I've tried to honor that behavior by gathering the IDs along with the URL and deduplicate by ID according to that precedence.

One caveat is that Maven differentiates between repository and pluginRepository. Our code does not, so if central is overwritten in a repository it will also overwrite it in pluginRepository. It seems somewhat unlikely that only one or the other is overwritten, so I think this is probably ok for now.

@jakecoffman jakecoffman requested a review from a team as a code owner October 12, 2022 20:27
@jakecoffman jakecoffman force-pushed the jakecoffman/maven-overwrite-repository-by-id branch from cd042df to 14f53fa Compare October 12, 2022 20:28
@jakecoffman jakecoffman force-pushed the jakecoffman/maven-overwrite-repository-by-id branch from 14f53fa to afcf67d Compare October 12, 2022 20:34
@jeffwidman jeffwidman changed the title fix maven inability to overwrite repository urls by id Fix Maven inability to overwrite repository urls by ID Oct 12, 2022
@jeffwidman jeffwidman added the L: java:maven Maven packages via Maven label Oct 12, 2022
Copy link
Contributor

@mctofu mctofu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -36,27 +37,39 @@ def initialize(dependency_files:, evaluate_properties: true)

# Collect all repository URLs from this POM and its parents
def repository_urls(pom:, exclude_inherited: false)
repo_urls_in_pom =
entries = gather_repository_urls(pom: pom, exclude_inherited: exclude_inherited)
urls = Set.new
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ids might be a better name since we store the ids in it?

@jakecoffman jakecoffman force-pushed the jakecoffman/maven-overwrite-repository-by-id branch from afcf67d to 2e4650d Compare October 13, 2022 00:41
@jakecoffman jakecoffman merged commit 5ae1a8c into main Oct 13, 2022
@jakecoffman jakecoffman deleted the jakecoffman/maven-overwrite-repository-by-id branch October 13, 2022 15:08
@pavera pavera mentioned this pull request Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: java:maven Maven packages via Maven
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants