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

PomResolver does not resolve properties in repository URI #2603

Closed
rsynek opened this issue Jan 5, 2023 · 1 comment
Closed

PomResolver does not resolve properties in repository URI #2603

rsynek opened this issue Jan 5, 2023 · 1 comment
Assignees
Labels
bug Something isn't working
Milestone

Comments

@rsynek
Copy link

rsynek commented Jan 5, 2023

PomResolver fails to resolve a dependency with a repository in its pom.xml, if the repository URI contains a property defined in a parent pom.xml.

Result:

Caused by: java.lang.IllegalArgumentException: Illegal character in path at index 1: ${sonatypeOssDistMgmtStagingUrl}
	at java.base/java.net.URI.create(URI.java:906)
	at org.openrewrite.maven.internal.MavenPomDownloader.normalizeRepository(MavenPomDownloader.java:643)

Root cause:

The dependency org.eclipse.persistence:org.eclipse.persistence.moxy:4.0.0 has a parent pom.xml org.eclipse.persistence:org.eclipse.persistence.parent:4.0.0, which defines an additional repository. The repository URI is based on a property defined in a parent pom.xml one level above.

https://repo1.maven.org/maven2/org/eclipse/persistence/org.eclipse.persistence.parent/4.0.0/org.eclipse.persistence.parent-4.0.0.pom
https://repo1.maven.org/maven2/org/eclipse/ee4j/project/1.0.7/project-1.0.7.pom

This issue is a sort of a chicken-egg problem; when the PomResolver processes the org.eclipse.persistence.parent, it merges the repository among the repositories it uses for resolving the remaining hierarchy. The next step is an attempt to resolve the org.eclipse.ee4j:project, which fails as the repository URI contains an unresolved property sonatypeOssDistMgmtStagingUrl (that would come from the org.eclipse.ee4j:project).

Reproducer:

#2602

Proposal:

Perhaps there should be a MavenRepository and a ResolvedMavenRepository. Only the latter would be added to the repositories PomResolver internally uses for additional resolving, after the URI (and other properties) of the MavenRepository have been properly resolved and sanitized.

@tkvangorder
Copy link
Contributor

Thank you for reporting, this was very much a chicken-egg problem. We recursively resolve a pom's properties and its parent and we can only resolve a parent based on the information we have so far and we use all repositories defined in a pom to resolve the parent. We really should never try to use a repository if it has unresolved property placeholders but we also don't want to normalize away that repository either.

So now, we simply ignore the repo if it has unresolved property placeholders, this allows resolution to continue to download the parent from Maven central....and then we can pick up the additional properties from the parent. The subsequent usage of that repository will then correctly resolve the repository using the property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants