[MSHADE-434] Restore project.getFile() after dependency-reduced POM creation - #827
Open
elharo wants to merge 1 commit into
Open
[MSHADE-434] Restore project.getFile() after dependency-reduced POM creation#827elharo wants to merge 1 commit into
elharo wants to merge 1 commit into
Conversation
…reation Calling project.setFile(dependencyReducedPomLocation) changes the project file reference for subsequent plugins, causing them to resolve relative paths against the DRP directory (e.g., target/) instead of the project basedir. This breaks plugins like apache-rat-plugin and checkstyle that rely on project.getFile() for path resolution. Save and restore the original project file around DRP creation to prevent this side-effect. Includes integration test MSHADE-434_ratSideEffect that verifies project.file still points to the original pom.xml after shading.
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Restores MavenProject#getFile() after creating the dependency-reduced POM to prevent side effects on subsequent plugins, and adds an integration test to pin the behavior.
Changes:
- Save the original
project.getFile()before DRP generation and restore it afterward inShadeMojo. - Add an IT project that configures
dependencyReducedPomLocationundertarget/. - Add Groovy verification to assert DRP + shaded JAR creation and that
${project.file}remainspom.xml.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java | Saves/restores project.file around DRP generation to avoid leaking state to later plugins. |
| src/it/projects/MSHADE-434_ratSideEffect/verify.groovy | Verifies DRP + shaded JAR outputs and checks build log for ${project.file} value. |
| src/it/projects/MSHADE-434_ratSideEffect/pom.xml | Integration-test Maven project reproducing the side-effect scenario and evaluating project.file. |
Comments suppressed due to low confidence (1)
src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java:1
project.setFile(originalProjectFile)won’t run ifcreateDependencyReducedPom(artifactIds)(or subsequent logic) throws, so the brokenproject.filecan still leak to later plugins on failure paths. Wrap DRP creation in atry { ... } finally { project.setFile(originalProjectFile); }(and keep theuseDependencyReducedPomInJarlogic inside thetry).
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+41
| def projectFileLine = lines.find { it =~ /^\// && it.endsWith("/pom.xml") } | ||
| assert projectFileLine != null : "project.file value not found in build log (expected a path ending with /pom.xml)" | ||
| assert projectFileLine.endsWith("/pom.xml") : "project.file should be pom.xml, but was: " + projectFileLine |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Calling
project.setFile(dependencyReducedPomLocation)inShadeMojochanges the project file reference for all subsequent plugins in the build. This causes plugins likeapache-rat-pluginandmaven-checkstyle-pluginto resolve relative paths against the wrong directory, breaking their behavior.Specifically, when
dependencyReducedPomLocationis set to a directory other than${basedir}(e.g.,target/),project.getFile()returns the DRP path, and plugins relying ongetFile().getParentFile()for basedir resolution will resolve paths incorrectly.This was introduced/changed by MSHADE-321 (commit 6ea8543) which moved the
createDependencyReducedPom()call outside of the inner artifact-processing block.Fix
Save
project.getFile()before callingcreateDependencyReducedPom()and restore it afterward inShadeMojo.execute(). This ensuresproject.getFile()continues to point to the originalpom.xmlafter the shade plugin finishes, preventing side effects on subsequent plugins.Integration Test
MSHADE-434_ratSideEffect: Configures shade withdependencyReducedPomLocationintarget/, then verifies viamaven-help-plugin:evaluatethat${project.file}still points to the originalpom.xmlafter shading.dependency-reduced-pom.xml)pom.xml)All 72 unit tests pass.
Fixes #453