⚽ Live Match: apache/maven enforcer/pull/

w3lld1 · 2026-07-12T21:02:22Z

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

This pull request addresses one issue without unrelated changes.
The description explains what the pull request does, how, and why.
The commit has a meaningful subject line and body.
Regression tests cover the behavioral change.
mvn verify passed as part of the command below.
Integration tests passed with mvn -Prun-its verify.

Summary

ArtifactMatcher.match() identifies dependencies that are excluded and not restored by an include override. enforceBytecodeVersion previously kept those matches for analysis, reversing the intended filtering behavior.

This change negates that matcher result so excluded dependencies are skipped while explicit includes still override broad exclusions. It adds regression coverage for both excludes-only configuration and include overrides.

Verification

mvn -Prun-its verify — passed (151 plugin integration tests and 1 extension integration test; all reactor modules successful)
mvn -pl enforcer-rules -am -Dtest=EnforceBytecodeVersionTest -Dsurefire.failIfNoSpecifiedTests=false test — passed (28 tests)
git diff --check — passed

Fixes #985

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.

Invert the ArtifactMatcher result so excluded dependencies are skipped while include overrides remain eligible for bytecode checks. Add regression coverage for excludes with and without include overrides.

Copilot

Pull request overview

This PR fixes enforceBytecodeVersion dependency filtering so that dependencies matching configured excludes are actually skipped from bytecode analysis, while explicit includes continue to override broader excludes (aligning the rule’s filtering with ArtifactMatcher semantics and the documented behavior in the issue).

Changes:

Invert ArtifactMatcher.match(...) usage in EnforceBytecodeVersion#filterDependencies(...) so “matched” artifacts are excluded from analysis.
Add regression tests covering “excludes-only” filtering and “includes override excludes” behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/EnforceBytecodeVersion.java	Fixes filtering predicate so excluded dependencies are not analyzed.
enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/EnforceBytecodeVersionTest.java	Adds targeted regression tests for the excludes/includes filtering behavior.

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

elharo · 2026-08-19T13:24:29Z

+        Method method = EnforceBytecodeVersion.class.getDeclaredMethod("filterDependencies", List.class);
+        method.setAccessible(true);
+        return (List<Dependency>) method.invoke(rule, dependencies);
+    }


I agree with the diagnosis, but the fix should probably be to use an integration test rather than a unit test.

elharo

needs an integration test

Signed-off-by: w3lld1 <42353747+w3lld1@users.noreply.github.com>

w3lld1 · 2026-08-23T11:52:47Z

Thanks — replaced the reflective unit coverage with the existing integration-test project in 3d8b7108. It now exercises broad exclusion plus an explicit include override through the plugin configuration. The full current CI matrix is green on this head.

elharo · 2026-08-23T12:27:55Z

        if (includes != null || excludes != null) {
            ArtifactMatcher artifactMatcher = new ArtifactMatcher(excludes, includes);
-            includeExcludeMatcher = d -> artifactMatcher.match(ArtifactUtils.toArtifact(d));
+            includeExcludeMatcher = d -> !artifactMatcher.match(ArtifactUtils.toArtifact(d));


It's not obvious from the code whether the not is needed or not. The word "filter" is ambiguous. It needs comments explaining what is expected that leaves no room for interpretation.

Avoiding lambdas would also make this clearer. As is I can't really tell what it's doing or what it's intended to do which is how we ended up with a bug in the first place.

elharo · 2026-08-23T12:28:41Z

+        return new Dependency(new DefaultArtifact(groupId + ":" + artifactId + ":jar:1.0"), "compile");
+    }
+
+    private static void setField(EnforceBytecodeVersion rule, String name, List<String> value) throws Exception {


don't use reflection

Signed-off-by: w3lld1 <42353747+w3lld1@users.noreply.github.com>

w3lld1 · 2026-08-24T20:56:28Z

Thanks — addressed both follow-ups in 49171b5b:

replaced the ambiguous negated predicate with an explicit loop that names and skips excluded dependencies, with a comment documenting ArtifactMatcher semantics;
removed the reflection-based unit tests; the regression remains covered by the existing integration-test project.

Fresh validation on the pushed commit:

focused EnforceBytecodeVersionTest: 26 passed;
enforce-bytecode-version-excluded-deps invoker integration test: passed;
Checkstyle, Spotless, RAT, and git diff --check: passed.

elharo

It should be possible to test this with an integration test without using reflection

w3lld1 · 2026-08-31T07:38:27Z

The current head already uses the existing invoker integration-test project without reflection. Commit 49171b5b removed the reflective unit tests; maven-enforcer-plugin/src/it/projects/enforce-bytecode-version-excluded-deps/pom.xml now covers both the broad exclusion and explicit include override through plugin configuration. The invoker test and the full CI matrix pass on this head.

slawekjaranowski · 2026-08-31T14:06:01Z

@elharo please look again

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

slawekjaranowski · 2026-08-31T16:30:40Z

 						<enforceBytecodeVersion>
 							<maxJdkVersion>1.5</maxJdkVersion>
 							<excludes>
-								<exclude>org.mindrot:jbcrypt:*:*</exclude>
+								<exclude>*:*</exclude>
 							</excludes>
+							<includes>
+								<include>com.google.code.findbugs:jsr305</include>
+							</includes>
 						</enforceBytecodeVersion>


We should not change this test .... it should be pass ... but it is wrong

org/mindrot/jbcrypt/BCrypt.class is compiled to JDK 1.5 so test will bass even wrong exclusion ...

we can change

<maxJdkVersion>1.5</maxJdkVersion>

to

<maxJdkVersion>1.4</maxJdkVersion>

slawekjaranowski

still fail

w3lld1 · 2026-09-02T15:32:18Z

Thanks — fixed the integration test in 4b0b810. It now excludes all dependencies at maxJdkVersion 1.4 without an include override. That makes the test fail if exclusion filtering regresses because jbcrypt targets Java 5, while keeping the intended successful-build assertion.

Fresh validation on the pushed commit:

mvn -T 2 -Prun-its -pl maven-enforcer-plugin -am -Dinvoker.test=enforce-bytecode-version-excluded-deps -DskipTests verify — BUILD SUCCESS; 1 integration test passed
Checkstyle, Spotless, RAT, and git diff --check passed

Signed-off-by: w3lld1 <42353747+w3lld1@users.noreply.github.com>

The enforce-bytecode-version-excluded-deps project excluded every dependency with *:* at maxJdkVersion 1.4, so it passed just as well when filterDependencies() returned nothing at all. Narrow the exclude to commons-lang3, the only dependency targeting a JDK above the threshold, and keep jsr305 as a dependency that is really analysed and passes. Add enforce-bytecode-version-included-deps for the case where an include overrides a wide exclude - there the build has to fail on commons-lang3 alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

slawekjaranowski · 2026-09-14T18:35:33Z

@w3lld1 thanks for contribution, I have added next IT

Fix enforceBytecodeVersion excludes filtering

15a5398

Invert the ArtifactMatcher result so excluded dependencies are skipped while include overrides remain eligible for bytecode checks. Add regression coverage for excludes with and without include overrides.

slawekjaranowski requested a review from cstamas July 15, 2026 22:40

slawekjaranowski added the bug Something isn't working label Jul 15, 2026

slawekjaranowski requested a review from Copilot July 21, 2026 18:15

Copilot started reviewing on behalf of slawekjaranowski July 21, 2026 18:16

View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

elharo requested changes Aug 19, 2026

View reviewed changes

test: cover bytecode excludes in integration test

3d8b710

Signed-off-by: w3lld1 <42353747+w3lld1@users.noreply.github.com>

elharo requested changes Aug 23, 2026

View reviewed changes

fix: clarify bytecode dependency filtering

49171b5

Signed-off-by: w3lld1 <42353747+w3lld1@users.noreply.github.com>

elharo reviewed Aug 24, 2026

View reviewed changes

elharo approved these changes Aug 31, 2026

View reviewed changes

slawekjaranowski requested a lite review from Copilot August 31, 2026 15:36

Copilot started reviewing on behalf of slawekjaranowski August 31, 2026 15:39

View session

Copilot AI reviewed Aug 31, 2026

View reviewed changes

slawekjaranowski requested changes Aug 31, 2026

View reviewed changes

test: exercise bytecode exclusion threshold

4b0b810

Signed-off-by: w3lld1 <42353747+w3lld1@users.noreply.github.com>

w3lld1 force-pushed the fix/enforce-bytecode-excludes branch from 241d311 to 4b0b810 Compare September 2, 2026 15:45

slawekjaranowski approved these changes Sep 14, 2026

View reviewed changes

slawekjaranowski merged commit 4991d9c into apache:master Sep 14, 2026
17 checks passed

github-actions Bot assigned slawekjaranowski Sep 14, 2026

github-actions Bot added this to the 3.6.4 milestone Sep 14, 2026

Conversation

w3lld1 commented Jul 12, 2026

Summary

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

elharo Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

elharo left a comment

Choose a reason for hiding this comment

Uh oh!

w3lld1 commented Aug 23, 2026

Uh oh!

elharo Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

elharo Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

w3lld1 commented Aug 24, 2026

Uh oh!

elharo left a comment

Choose a reason for hiding this comment

Uh oh!

w3lld1 commented Aug 31, 2026

Uh oh!

slawekjaranowski commented Aug 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

slawekjaranowski Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

slawekjaranowski left a comment

Choose a reason for hiding this comment

Uh oh!

w3lld1 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slawekjaranowski commented Sep 14, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

w3lld1 commented Sep 2, 2026 •

edited

Loading