openrewrite
Maven `UpgradeParentVersion` incorrectly attempts & fails to download a parent pom.xml when using `<version>${revision}${sha1}${changelist}</version>` in multi module project
When using CI friendly versions in a multi module maven project, where at least one of the variable is an XML nil value, the upgrade parent recipe fails.
What version of OpenRewrite are you using?
I am using
- Maven plugin v6.25.0
How are you running OpenRewrite?
mvn org.openrewrite.maven:rewrite-maven-plugin:6.25.0:dryRun
I am using the Maven plugin, and my project is a multi module project.
<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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>example.group</groupId>
<artifactId>project-parent</artifactId>
<version>${revision}${sha1}${changelist}</version>
<packaging>pom</packaging>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.5.0</version>
</parent>
<properties>
<revision>1.2.3</revision>
<changelist>-SNAPSHOT</changelist>
<sha1 /> <!-- nil property value -->
</properties>
<modules>
<module>module1</module>
</modules>
<build>
<plugins>
<plugin>
<groupId>org.openrewrite.maven</groupId>
<artifactId>rewrite-maven-plugin</artifactId>
<version>6.25.0</version>
<configuration>
<activeRecipes>
<recipe>example.recipe</recipe>
</activeRecipes>
</configuration>
</plugin>
</plugins>
</build>
</project>
rewrite.yml in the project basedir
---
type: specs.openrewrite.org/v1beta/recipe
name: example.recipe
recipeList:
- org.openrewrite.maven.UpgradeParentVersion:
groupId: org.springframework.boot
artifactId: spring-boot-starter-parent
newVersion: 3.5.x
submodule:
<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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>example.group</groupId>
<artifactId>project-parent</artifactId>
<version>${revision}${sha1}${changelist}</version>
</parent>
<artifactId>project-module1</artifactId>
</project>
What is the smallest, simplest way to reproduce the problem?
See above project setup
What did you expect to see?
I expected the spring-boot-starter-parent of the main project pom to be updated.
What did you see instead?
Recipe fails while because it tries to download from an invalid url:
Illegal character in path at index 77: https://repo.internal/repository/maven-public/example/group/project-parent/${revision}${sha1}${changelist}/project-parent-${revision}${sha1}${changelist}.pom
When I change the property value from <sha1 /> to <sha1></sha1> it does work.
So the issue is probably somewhere in the version checks of the MavenPomDownloader: https://github.com/openrewrite/rewrite/blob/cc34e7578fe8f99ff985e43bf551fdb8daa10628/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java#L504
Likely comparing version 1.2.3-SNAPSHOT with 1.2.3null-SNAPSHOT.
What is the full stack trace of any errors you encountered?
Caused by: java.lang.IllegalArgumentException: Illegal character in path at index 77: https://repo.internal/repository/maven-public/example/group/project-parent/${revision}${sha1}${changelist}/project-parent-${revision}${sha1}${changelist}.pom
at java.net.URI.create (URI.java:906)
at org.openrewrite.maven.internal.MavenPomDownloader.download (MavenPomDownloader.java:561)
at org.openrewrite.maven.tree.ResolvedPom$Resolver.resolveParentPom (ResolvedPom.java:582)
at org.openrewrite.maven.tree.ResolvedPom$Resolver.resolveParentPropertiesAndRepositoriesRecursively (ResolvedPom.java:471)
at org.openrewrite.maven.tree.ResolvedPom$Resolver.resolveParentsRecursively (ResolvedPom.java:430)
at org.openrewrite.maven.tree.ResolvedPom$Resolver.resolve (ResolvedPom.java:419)
at org.openrewrite.maven.tree.ResolvedPom.resolve (ResolvedPom.java:213)
at org.openrewrite.maven.UpdateMavenModel.updateResult (UpdateMavenModel.java:173)
at org.openrewrite.maven.UpdateMavenModel.lambda$updateResult$8 (UpdateMavenModel.java:178)
at org.openrewrite.internal.ListUtils.map (ListUtils.java:245)
at org.openrewrite.internal.ListUtils.map (ListUtils.java:269)
at org.openrewrite.maven.UpdateMavenModel.updateResult (UpdateMavenModel.java:176)
at org.openrewrite.maven.UpdateMavenModel.visitDocument (UpdateMavenModel.java:142)
at org.openrewrite.xml.tree.Xml$Document.acceptXml (Xml.java:153)
at org.openrewrite.xml.tree.Xml.accept (Xml.java:57)
at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:242)
at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:265)
at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:154)
at org.openrewrite.Preconditions$Check.visit (Preconditions.java:159)
at org.openrewrite.Preconditions$Check.visit (Preconditions.java:130)
at org.openrewrite.scheduling.RecipeRunCycle.lambda$editSources$7 (RecipeRunCycle.java:210)
at org.openrewrite.table.RecipeRunStats$PhaseTimer.recordTimed (RecipeRunStats.java:142)
at org.openrewrite.table.RecipeRunStats$RecipeTimers.recordEdit (RecipeRunStats.java:130)
at org.openrewrite.table.RecipeRunStats.recordEdit (RecipeRunStats.java:58)
at org.openrewrite.scheduling.RecipeRunCycle.lambda$editSources$8 (RecipeRunCycle.java:206)
at org.openrewrite.scheduling.RecipeStack.reduce (RecipeStack.java:60)
at org.openrewrite.scheduling.RecipeRunCycle.lambda$editSources$9 (RecipeRunCycle.java:179)
at org.openrewrite.internal.InMemoryLargeSourceSet.lambda$edit$0 (InMemoryLargeSourceSet.java:86)
at org.openrewrite.internal.ListUtils.map (ListUtils.java:245)
at org.openrewrite.internal.ListUtils.map (ListUtils.java:269)
at org.openrewrite.internal.InMemoryLargeSourceSet.edit (InMemoryLargeSourceSet.java:85)
at org.openrewrite.scheduling.RecipeRunCycle.editSources (RecipeRunCycle.java:177)
at org.openrewrite.RecipeScheduler.runRecipeCycles (RecipeScheduler.java:84)
at org.openrewrite.RecipeScheduler.scheduleRun (RecipeScheduler.java:41)
at org.openrewrite.Recipe.run (Recipe.java:449)
at org.openrewrite.Recipe.run (Recipe.java:445)
at org.openrewrite.Recipe.run (Recipe.java:441)
Hi @elmuerte ; Thanks for the report! Such dynamic version numbers have historically been overlooked a bit indeed. Sounds like you're running into another edge case. From your stacktrace it looks like the variables aren't replaced at all, although earlier in that method within a different scope we do replace the placeholders: https://github.com/openrewrite/rewrite/blob/623ee5ed9f5a078f37c38bbf11d612c5c6d4b1d1/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java#L509
Would welcome a PR to handle those placeholders before we download as well, if you're up for it.
I just noticed that explicitly defining a value for the nil property on the commandline does not fix it.
mvn org.openrewrite.maven:rewrite-maven-plugin:6.25.0:dryRun -Dsha1=foo
Results in a similar error (but no automatic stack trace this time).
[ERROR] Failed to execute goal org.openrewrite.maven:rewrite-maven-plugin:6.25.0:dryRun (default-cli) on project project-module1: Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:6.25.0:dryRun failed: Illegal character in path at index 77: https://repo.internal/repository/maven-public/example/group/project-parent/${revision}foo${changelist}/project-parent-${revision}foo${changelist}.pom -> [Help 1]
With -X is shows shows that it fails at the same place, but with a different trace.
Caused by: java.lang.IllegalArgumentException: Illegal character in path at index 77: https://repo.internal/repository/maven-public/example/group/project-parent/${revision}foo${changelist}/project-parent-${revision}foo${changelist}.pom
at java.net.URI.create (URI.java:906)
at org.openrewrite.maven.internal.MavenPomDownloader.download (MavenPomDownloader.java:561)
at org.openrewrite.maven.tree.ResolvedPom$Resolver.resolveParentPom (ResolvedPom.java:582)
at org.openrewrite.maven.tree.ResolvedPom$Resolver.resolveParentPropertiesAndRepositoriesRecursively (ResolvedPom.java:471)
at org.openrewrite.maven.tree.ResolvedPom$Resolver.resolveParentsRecursively (ResolvedPom.java:430)
at org.openrewrite.maven.tree.ResolvedPom$Resolver.resolve (ResolvedPom.java:419)
at org.openrewrite.maven.tree.ResolvedPom.resolve (ResolvedPom.java:213)
at org.openrewrite.maven.tree.Pom.resolve (Pom.java:218)
at org.openrewrite.maven.tree.Pom.resolve (Pom.java:198)
at org.openrewrite.maven.MavenParser.parseInputs (MavenParser.java:110)
at org.openrewrite.Parser.parse (Parser.java:59)
at org.openrewrite.maven.MavenMojoProjectParser.parseMaven (MavenMojoProjectParser.java:627)
at org.openrewrite.maven.MavenMojoProjectParser.listSourceFiles (MavenMojoProjectParser.java:177)
at org.openrewrite.maven.AbstractRewriteBaseRunMojo.loadSourceSet (AbstractRewriteBaseRunMojo.java:236)
at org.openrewrite.maven.AbstractRewriteBaseRunMojo.listResults (AbstractRewriteBaseRunMojo.java:152)
at org.openrewrite.maven.AbstractRewriteDryRunMojo.execute (AbstractRewriteDryRunMojo.java:74)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
Perhaps a good question to ask such that we're not running down the wrong path: should these dependencies be downloaded at all, or are they all from the same multi module build that should just be available in that way?
This dependency should never be downloaded as it's the root of the project. The dependency would probably never exist in a remote repository anyway as it would be the current in-development version of the project.
That helps confirm my suspicion indeed; we'd need to look at why we're even attempting to download that POM rather than getting it from the Maven execution context. Any help appreciated as it's hard for me to find time to dive into such edge cases.
Finally managed to debug the issue.
MavenPomDownloader.download is called with the request for the gav example.group:project-parent:${revision}${sha1}${changelist}
It checks projectPomsByGav in the downloader if that exists. The projectPomsByGav does contain the actual project it is looking for, with the key example.group:project-parent:1.2.3-SNAPSHOT. So it will not find it by key.
Interestingly enough the project linked to that key has the gav example.group:project-parent:1.2.3${sha1}-SNAPSHOT. So when it goes through the projectPoms to match the request gav is gets here:
https://github.com/openrewrite/rewrite/blob/c7dd715c5e5983cb67b1db5ac3806784884617e1/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java#L508-L511
The versionWithReplacements is the correct 1.2.3-SNAPSHOT. But projectPom.getVersion() is, as previously mentioned, 1.2.3${sha1}-SNAPSHOT.
The projectPomsByGav using a key which explicitly goes through an additional properly replacement phase: https://github.com/openrewrite/rewrite/blob/c7dd715c5e5983cb67b1db5ac3806784884617e1/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java#L181
I think the real issue is with the incorrect gav in the projectPom.
I traced back the issue to: https://github.com/openrewrite/rewrite/blob/c1e37317faa7949d79c140cd6d192644acae1cb9/rewrite-maven/src/main/java/org/openrewrite/maven/tree/ResolvedPom.java#L318
The properties array has the key sha1 with the value null. There is obviously a difference between "no such entry", and "entry with value null".
Adding this fixes the problem.
if (propVal != null) {
...
} else if (properties.containsKey(property)) {
// An existing property key with a `null` value should be regarded as an empty string.
return "";
}
But I'm not sure if this creates new problems.
Thanks for diving in, tracing this all the way through and sharing your detailed notes here. Seems reasonable to make that adjustment here. We can look at any fall out on the tests we already in the wider openrewrite org. Would you want to push that up?