[MSHADE-573] Fix missing exclusions - direct and transitive by marwin1991 · Pull Request #770 · apache/maven-shade-plugin

marwin1991 · 2025-11-09T11:32:38Z

Following this checklist to help us incorporate your
contribution quickly and easily:

Make sure there is a JIRA issue filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes. =[MSHADE-202] When promoteTransitiveDependencies=true, some <exclusions> are stripped from the dependency-reduced-pom #573
Each commit in the pull request should have a meaningful subject line and body.
Format the pull request title like [MSHADE-XXX] - Fixes bug in ApproximateQuantiles,
where you replace MSHADE-XXX with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Run mvn clean verify to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
You have run the integration tests successfully (mvn -Prun-its clean verify).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.

Pull Request Description

Preserve original and transitive exclusions in dependency-reduced-pom to improve POM correctness

Summary

This PR fixes a long-standing issue where exclusions declared in the original pom.xml — including wildcard and transitive exclusions — were not propagated to the generated dependency-reduced-pom.xml. As a result, the reduced POM could reintroduce dependencies that the user had explicitly excluded.

The change introduces a dedicated exclusion resolution step and updates the reduced-POM generation so that:

Direct exclusions defined on dependencies in the original POM are preserved.
Transitive exclusions (required to keep the effective dependency graph consistent) are carried over.

Motivation and context

Example of the incorrect behavior (existing upstream case):

Source POM defines a wildcard exclusion on a dependency:

<dependency>
    <groupId>com.itextpdf</groupId>
    <artifactId>itextpdf</artifactId>
    <version>5.5.13.3</version>
    <exclusions>
        <exclusion>
            <groupId>*</groupId>
            <artifactId>*</artifactId>
        </exclusion>
    </exclusions>
</dependency>

But the resulting dependency-reduced-pom.xml previously dropped the exclusions entirely:

<dependency>
    <groupId>com.itextpdf</groupId>
    <artifactId>itextpdf</artifactId>
    <version>5.5.13.3</version>
</dependency>

Reference:

maven-shade-plugin/src/it/projects/MSHADE-467_parallel-dependency-reduced-pom/pom.xml

Line 64 in 073e7ca

<groupId>com.itextpdf</groupId>

With this PR, exclusions like the above (including */*) are preserved in the reduced POM.

What changed

Core logic
- Added DependenciesExclusionResolver to compute effective exclusions for both direct and transitive dependencies.
- Introduced DependencyList utility to help build the reduced POM dependency set with proper scopes and exclusions.
- Refactored ShadeMojo to apply resolved exclusions during reduced POM generation and to streamline transitive dependency handling.
Tests
- Added a new IT project: dep-reduced-pom-with-transitive-dependencies-with-exclusions (verifies propagation of direct and transitive exclusions).
- Updated/added expected reduced POMs for MSHADE-467 parallel builds (shadeMT1..shadeMT4/expected-dependency-reduced-pom.xml).
- Kept changes minimal for now to validate the approach first.

Behavior before vs after

Before: dependency-reduced-pom.xml could omit user-declared exclusions and omit required transitive exclusions, causing unexpected dependencies to appear for consumers.
After: The reduced POM preserves original exclusions and includes necessary transitive exclusions, ensuring the effective dependency graph remains consistent.

Why this matters

Ensures consumers of the shaded artifact do not unintentionally pull dependencies that were explicitly excluded.
Aligns the reduced POM with what users expect from the original POM’s dependency declarations.
Improves reproducibility and clarity of dependency management for downstream builds.

Testing

Added basic integration tests to demonstrate the corrected behavior:
- src/it/projects/dep-reduced-pom-with-transitive-dependencies-with-exclusions/*
- Updated MSHADE-467 parallel ITs with expected-dependency-reduced-pom.xml snapshots for visibility into changes, for example:
  - src/it/projects/MSHADE-467_parallel-dependency-reduced-pom/shadeMT1/expected-dependency-reduced-pom.xml

Note: At this stage only the foundational tests are included. If the overall approach is accepted, I will add more test coverage, including corner cases (wildcards, multiple excludes, ordering, and mixed scopes).

Open question: how strict should IT verification be?

Currently, to make differences obvious, I added snapshot files like:

src/it/projects/MSHADE-467_parallel-dependency-reduced-pom/shadeMT1/expected-dependency-reduced-pom.xml

Is a strict one-to-one comparison (expected vs. generated reduced POM) desirable for ITs? My recommendation:

Prefer structure-aware assertions over literal byte-for-byte equality to avoid false negatives due to formatting, ordering, or benign metadata changes. Options:
- Normalize and compare: strip whitespace, sort dependency and exclusion lists where ordering is non-semantic, and compare normalized XML.
- XPath-based checks: assert presence of specific dependencies and their exclusion blocks, and absence of those that should remain excluded.
We can keep a small number of full-file snapshots for human readability, but rely on normalized or XPath checks for robustness.

I’m happy to implement whichever verification approach the project prefers.

Refactor shade plugin to streamline dependency exclusion handling and remove VerboseJavaScopeSelector implementation Refactor dependency reduced POM handling to improve exclusion resolution logic and streamline transitive dependency processing Update integration test to reflect transitive dependency handling in dependency reduced POM with exclusions Remove test files for dependency reduced POM validation in `shadeMT2` and `shadeMT3` integration tests

marwin1991 · 2025-11-09T11:42:43Z

@@ -0,0 +1,600 @@
+<!--


Preserve only for testing.

I’m wondering if I should add a 1:1 comparison between the generated reduced-pom.xml and the expected one (the expected file was generated in the main branch).

Then, in a pull request, maintainers could see the diff in the expected reduced-pom.xml, which would make it clearer how the pull request affects the plugin.

I’m a maintainer of a changelog plugin, and in that project I verify one-to-one changes for every integration test. This way, after community contributions, I can clearly see how they influence the final artifact—instead of just guessing what changed.

marwin1991 · 2025-11-12T12:29:57Z

FYI @slachiewicz @Bukama @slawekjaranowski @olamy

marwin1991 · 2026-05-07T09:22:23Z

Hello @slachiewicz @Bukama @slawekjaranowski @olamy can I somehow help you during review this changes?

Copilot

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.

Fixes generation of dependency-reduced-pom.xml so that direct and transitive exclusions (including wildcard exclusions) are preserved, improving correctness of the reduced POM.

Changes:

Refactors reduced-POM rewrite logic in ShadeMojo to delegate exclusion computation to new helper classes.
Adds DependencyList and DependenciesExclusionResolver to compute/propagate exclusions into the reduced POM.
Adds/updates integration tests and expected reduced POM snapshots to validate exclusion propagation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/main/java/org/apache/maven/plugins/shade/mojo/ShadeMojo.java	Refactors reduced-POM rewrite loop to delegate exclusion resolution and exposes id helpers for reuse.
src/main/java/org/apache/maven/plugins/shade/mojo/DependencyList.java	Introduces dependency cloning wrapper and encapsulates exclusion-resolution step during rewrite.
src/main/java/org/apache/maven/plugins/shade/mojo/DependenciesExclusionResolver.java	New resolver that carries direct and transitive exclusions into final reduced dependencies.
src/it/projects/dep-reduced-pom-with-transitive-dependencies-with-exclusions/verify.groovy	Adds IT assertions for exclusion propagation in reduced POM.
src/it/projects/dep-reduced-pom-with-transitive-dependencies-with-exclusions/pom.xml	New IT project exercising promoted transitive dependencies and exclusions.
src/it/projects/MSHADE-467_parallel-dependency-reduced-pom/verify.groovy	Updates IT expectation for number of exclusions produced.
src/it/projects/MSHADE-467_parallel-dependency-reduced-pom/shadeMT1/expected-dependency-reduced-pom.xml	Adds expected reduced POM snapshot reflecting new exclusion behavior.
src/it/mrm/repository/dep-reduced-pom-with-transitive-dependencies-with-exclusions/dep-reduced-pom-with-transitive-dependencies-with-exclusions-1.0.pom	Adds test artifact POM used by the new integration test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+            for (Dependency dependencyWithMissingExclusions : finalDependencies) {
+                // MSHADE-311: do not add exclusion for provided transitive dep
+                //       note: MSHADE-31 introduced the exclusion logic for promoteTransitiveDependencies=true,
+                //             but as of 3.2.1 promoteTransitiveDependencies has no effect for provided deps,
+                //             which makes this fix even possible (see also MSHADE-181)
+                if (!dependencyWithMissingExclusions.getScope().equals("provided")) {
+                    // move original exclusions from pom.xml to reduced pom.xml
+                    if (isEqual(dependencyWithMissingExclusions, originalDirectDep)) {
+                        addDirectExclusions(dependencyWithMissingExclusions, directExclusions);
+                    }


+        DependencyList origDeps =
+                new DependencyList(promoteTransitiveDependencies ? transitiveDeps : project.getDependencies());


+    private boolean isEqual(Dependency d1, Dependency d2) {
+        return d1.getGroupId().equals(d2.getGroupId()) && d1.getArtifactId().equals(d2.getArtifactId());
+    }
+
+    private boolean isEqual(Dependency d1, org.eclipse.aether.graph.Dependency d2) {
+        return d1.getGroupId().equals(d2.getArtifact().getGroupId())
+                && d1.getArtifactId().equals(d2.getArtifact().getArtifactId());
+    }


+    public boolean resolveTransitiveDependenciesExclusions(
+            MavenSession session,
+            MavenProject originalProject,
+            ProjectBuilder projectBuilder,
+            File reducedPomFile,
+            RepositorySystem repositorySystem,
+            List<Dependency> finalDependencies,
+            Log log) {
+        try {
+
+            synchronized (session.getProjectBuildingRequest()) { // Lock critical section to fix MSHADE-467
+                ProjectBuildingRequest request = new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
+                request.setLocalRepository(session.getLocalRepository());
+                request.setRemoteRepositories(originalProject.getRemoteArtifactRepositories());
+
+                ProjectBuildingResult shaded = projectBuilder.build(reducedPomFile, request);
+
+                DependenciesExclusionResolver resolver = new DependenciesExclusionResolver(
+                        session, originalProject, shaded.getProject(), repositorySystem, log);
+
+                return resolver.resolve(this, finalDependencies);
+            }
+        } catch (ArtifactDescriptorException | DependencyCollectionException | ProjectBuildingException e) {
+            log.error("Failed to resolve exclusions for " + originalProject.getArtifact(), e);
+            throw new RuntimeException(e);
+        }
+    }


+    private void addDirectAndTransitiveExclusions(List<Dependency> finalDependencies)
+            throws ArtifactDescriptorException {
+        for (Dependency originalDirectDep : originalProject.getDependencies()) {
+            List<Exclusion> directExclusions = getDirectExclusions(originalDirectDep);
+            log.debug(
+                    "Found " + directExclusions.size() + " direct exclusions for " + originalDirectDep.getArtifactId());
+
+            RepositorySystemSession repoSession = session.getRepositorySession();
+
+            ArtifactDescriptorRequest descriptorRequest = new ArtifactDescriptorRequest();
+            descriptorRequest.setArtifact(toArtifact(originalDirectDep));
+            descriptorRequest.setRepositories(originalProject.getRemoteProjectRepositories());
+
+            ArtifactDescriptorResult descriptorResult =
+                    repositorySystem.readArtifactDescriptor(repoSession, descriptorRequest);
+
+            List<org.eclipse.aether.graph.Dependency> dependenciesWithExclusions =
+                    getDependenciesWithExclusions(descriptorResult);
+
+            for (Dependency dependencyWithMissingExclusions : finalDependencies) {


+def ns = new groovy.xml.Namespace("http://maven.apache.org/POM/4.0.0") 
+def pom = new XmlParser().parse( pomFile )
+
+assert pom[ns.modelVersion].size() == 1
+assert pom[ns.dependencies][ns.dependency].size() == 5
+assert pom[ns.dependencies][ns.dependency][1][ns.exclusions][ns.exclusion].size() == 3
+assert pom[ns.dependencies][ns.dependency][3][ns.exclusions][ns.exclusion].size() == 1


+ * 2 CASE - direct exclusion without promotion
+ * If our pom.xml has a dependency called A (and A is NOT defined to include in artifactSet).
+ * And this dependency has defined exclusions (in out pom.xml),
+ * So in reduced pom, we need to add original exclusions to A dependency


marwin1991 added 2 commits November 9, 2025 12:30

Remove unused updateExcludesInDeps method from ShadeMojo class

a1af4ac

marwin1991 commented Nov 9, 2025

View reviewed changes

marwin1991 mentioned this pull request Dec 14, 2025

[MSHADE-467] Dependency-reduced POM with missing exclusions in concurrent build #721

Closed

elharo requested a review from Copilot July 29, 2026 12:29

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Copilot started reviewing on behalf of elharo July 29, 2026 13:04 View session

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MSHADE-573] Fix missing exclusions - direct and transitive - #770

[MSHADE-573] Fix missing exclusions - direct and transitive #770
marwin1991 wants to merge 2 commits into
apache:masterfrom
marwin1991:fix-missing-exclusions

marwin1991 commented Nov 9, 2025

Uh oh!

marwin1991 Nov 9, 2025

Uh oh!

marwin1991 commented Nov 12, 2025

Uh oh!

marwin1991 commented May 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		DependencyList origDeps =
		new DependencyList(promoteTransitiveDependencies ? transitiveDeps : project.getDependencies());

Uh oh!

Conversation

marwin1991 commented Nov 9, 2025

Pull Request Description

Summary

Motivation and context

What changed

Behavior before vs after

Why this matters

Testing

Open question: how strict should IT verification be?

Uh oh!

marwin1991 Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

marwin1991 commented Nov 12, 2025

Uh oh!

marwin1991 commented May 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants