Issue: 17964: Run exclude before and after dependencySubstitution to exclude rewritten dependencies

[attix-zhang]

Apply the exclude spec before and after dependency substitution to
ensure that redirected dependency will be excluded correctly.

Signed-off-by: Attix Zhang attix.zhang.cs@gmail.com

Fixes #17964

Context

Contributor Checklist

• Review Contribution Guidelines
• Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
• Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
• Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team
• Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
• Provide unit tests (under <subproject>/src/test) to verify logic
• Update User Guide, DSL Reference, and Javadoc for public-facing changes
• Ensure that tests pass sanity check: ./gradlew sanityCheck
• Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest

Gradle Core Team Checklist

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation
• Recognize contributor in release notes

[attix-zhang]

Why I don't see the Allow edit from maintainers check box?

[attix-zhang]

Based on my research, the ResolveState::dependencySubstitutionApplicator won't be changed during the Resolution, so it should be fine to cache the dependency states after the dependency substitution.

[ljacomet]

@attix-zhang Thanks for the contribution!

I do not know why you did not see that checkbox. But it should not be a problem in this case.

I agree that we can cache the substitution result, so all good there.

I have started builds to verify the change has no unexpected consequence.

[attix-zhang]

Thank you @ljacomet ! Looks to me, all tests passed! (8 tests passed with re-try, not sure why)

[ljacomet]

Indeed, all tests passed, sending it for merge.

Thanks again for the contribution!

[bot-gradle]

OK, I've already triggered a build for you.

[bot-gradle]

Pre-tested commit build failed.

[blindpirate]

@bot-gradle test and merge

[bot-gradle]

OK, I've already triggered a build for you.

[attix-zhang]

Is there any chance that we can backport it to the previous version such as Gradle 6.x? Or the recommended way is to upgrade the Gradle to 7.3+?

[ljacomet]

The change is simple enough that we might consider a backport to 6.9.x.
It is however not critical enough to be the sole motivation for 6.9.2. Let me create an issue to track it though.

[attix-zhang]

Oh yeah. Thank you! @ljacomet

[mmorrisontx]

Are there side-effects now that the below methods (pending edge resolution, constraint activation) are operating on the un-substituted module instead of the substituted one?

[ljacomet]

dependencies(resolutionFilter) now performs the substitution. So the below method still apply to the substituted dependency.

[mmorrisontx]

Doh, for some reason I had it in my mind that cacheFilteredDependencyStates was only called later on. Thanks for clarification 😄