-
Notifications
You must be signed in to change notification settings - Fork 360
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
Do not throw "InvalidPathException: Illegal char" when encountering bill of materials POM in <dependencies> section #3167
Comments
Hi @magicwerk thanks for the detailed report! I've opened it up just now and it appears there's something odd about the way you use the wildfly bill of materials, and I see the same issue with Maven without OpenRewrite. You've declared a test scoped dependency on the bill of materials, but there's no jar associated with that pom.xml:
Could it be that you instead had wanted to use <dependency>
<groupId>org.wildfly</groupId>
<artifactId>wildfly-jms-client-bom</artifactId>
<version>24.0.1.Final</version>
<type>pom</type>
<scope>import</scope>
</dependency> Such an entry can then go into your If I've misunderstood please let me know! |
Hi, thanks for the prompt answer. Can you explain what you mean with seeing the same issue with without OpenRewrite? On the other hand it could as well be that the mentioned dependency is incorrect, but is is part as indicated in our project - where mvn install works, but mvn rewrite:run fails as in the example - let me check that. Nevertheless I think it is also strange if a meaningless or incorrect dependency cause openrewrite to fail with an error, if Maven is somehow able to handle it - maybe the error could be caught and and warning issued? |
My IDE initially failed to resolve You're right in that we should not throw an exception if Maven does not throw an exception under the same circumstances. We can keep this issue open to track that. Are you able to continue without that BOM in your dependencies section? Maybe even move it to dependency management & change the scope to import? Would not want this to block you from applying recipes, but don't know if we'll get to handling this edge case soon enough. |
I made some further attempts to made openrewrite work with our projects, but failed again.
So IMHO this seems not be an edge case and I would consider it rather being a bug than an enhancement. |
@magicwerk tried it just now and I'm getting a different error message with
Can you confirm you're getting that different error message as well, as opposed to the |
It's exactly the same on my machine:
[ERROR] Failed to execute goal org.openrewrite.maven:rewrite-maven-plugin:4.38.2:run (default-cli) on project my-app: Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:4.38.2:run failed: Illegal char <:> at index 10: org.apache:apache\pom.xml -> [Help 1] |
Weird how that's the same issue indeed; and that BOM is not coming in through a parent for instance? I'll log a separate issue with the dependency resolution that I saw locally for |
I'm not sure what you mean with BOM, I can reproduce the behavior with the same tiny maven project i sent you yesterday, just by replacing the dependency.
|
Yes I've logged openrewrite/rewrite-maven-plugin#478 separately, as to me those two issues seem unrelated, if quite unfortunate that you're affected twice! More details about that last one in the other issue, but feel free to add any more details that you feel could help debugging there. |
I'm quite astonished... As said, If I unpack my-app-new.zip and execute I get [INFO] --- rewrite-maven-plugin:4.38.2:run (default-cli) @ my-app --- That's different at your site? |
Yes; when I run your sample I see the
Hence why I logged that as a separate issue; not sure why you're seeing different results. |
My example run on Windows 10
|
I have the same issue |
I ran into this as well, and I think I understand what's going wrong (at least roughly). If you run the maven plugin with -X (debug) enabled, you get the following stacktrace (only copied the relevant part):
This in fact is a pretty good indication the last part of openrewrite it comes from is at org.openrewrite.maven.internal.MavenPomDownloader.download (MavenPomDownloader.java:454)
See https://central.sonatype.com/artifact/org.apache.activemq/artemis-pom/2.28.0 It parses this parent pom, and finds the relative path, and you can clearly see the : in the relativepath at character index 10 where it's failing. It then simply tries to find this as a plain file on the filesystem, which obviously doesn't exist. Whether or not it is correct to use the relativePath I can't decide, but this is where the problem is coming from I believe. In these cases, it shouldn't try to look up the file on the filesystem, but instead treat it as a 'normal' resolveable dependency I think? |
Thanks for chiming in here with your analysis @hawkeye-bot ! Especially love the small details such as |
Fixes #3167 artemis-pom-2.28.0.pom contains a weird relative path: https://repo1.maven.org/maven2/org/apache/activemq/artemis-pom/2.28.0/artemis-pom-2.28.0.pom ```xml <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/maven-v4_0_0.xsd"> <modelVersion>4.0.0</modelVersion> <groupId>org.apache.activemq</groupId> <artifactId>artemis-pom</artifactId> <packaging>pom</packaging> <version>2.28.0</version> <parent> <groupId>org.apache</groupId> <artifactId>apache</artifactId> <version>27</version> <relativePath>org.apache:apache</relativePath> </parent> ``` This worked okay-ish on Linux/Mac, as passing the relativePath into `Paths.get(relativePath, "pom.xml")` will just result in a null `maybeLocalPom`. On Windows however, a colon is not valid to pass into Paths.get(..), and it fails with the reported exception. Since we were effectively ignoring the relative path on Mac/Linux before, we might as well skip it entirely for Windows as well.
Fixes #3167 artemis-pom-2.28.0.pom contains a weird relative path: https://repo1.maven.org/maven2/org/apache/activemq/artemis-pom/2.28.0/artemis-pom-2.28.0.pom ```xml <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/maven-v4_0_0.xsd"> <modelVersion>4.0.0</modelVersion> <groupId>org.apache.activemq</groupId> <artifactId>artemis-pom</artifactId> <packaging>pom</packaging> <version>2.28.0</version> <parent> <groupId>org.apache</groupId> <artifactId>apache</artifactId> <version>27</version> <relativePath>org.apache:apache</relativePath> </parent> ``` This worked okay-ish on Linux/Mac, as passing the relativePath into `Paths.get(relativePath, "pom.xml")` will just result in a null `maybeLocalPom`. On Windows however, a colon is not valid to pass into Paths.get(..), and it fails with the reported exception. Since we were effectively ignoring the relative path on Mac/Linux before, we might as well skip it entirely for Windows as well.
Refactoring fails with exception in MavenPomDownloader.download.
This errors can be triggered by adding the following dependency
and the openrewrite plugin to a freshly created Maven project (see attached example project)
Test case:
unzip my-app.zip
execute mvn rewrite:run -Drewrite.activeRecipes=ChangePackageExample -X
my-app.zip
The text was updated successfully, but these errors were encountered: