Skip to content

[#26337] Implement class level compile avoidance based on ABI #34882

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[#26337] Implement class level compile avoidance based on ABI #34882

wants to merge 1 commit into from

Conversation

beikov
Copy link

@beikov beikov commented Sep 3, 2025 •
edited
Loading

Fixes #26337

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.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@beikov beikov requested a review from a team as a code owner September 3, 2025 16:11
@beikov beikov requested a review from octylFractal September 3, 2025 16:11
@bot-gradle bot-gradle added from:contributor PR by an external contributor to-triage labels Sep 3, 2025
@beikov beikov mentioned this pull request Sep 3, 2025
Open
@beikov
Copy link
Author

beikov commented Sep 3, 2025

For some reason, on Windows, the ForbiddenNullabilityAnnotationsTest and PlatformBoundariesTest tests fail with an error about the : character in a file path:

java.nio.file.InvalidPathException: Illegal char <:> at index 2: /C:/Users/chris/Documents/GitHub/gradle/platforms/core-runtime/launcher/build/libs/gradle-launcher-9.2.0.jar
	at java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
	at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
	at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
	at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
	at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232)
	at java.base/java.nio.file.Path.of(Path.java:147)
	at java.base/java.nio.file.Paths.get(Paths.java:69)
	at org.gradle.architecture.test.ArchUnitFixture.getClassFile(ArchUnitFixture.java:610)
	at org.gradle.architecture.test.PlatformBoundariesTest$2.test(PlatformBoundariesTest.java:112)
	at org.gradle.architecture.test.PlatformBoundariesTest$2.test(PlatformBoundariesTest.java:109)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:178)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1845)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
	at com.tngtech.archunit.lang.AbstractClassesTransformer$1.doTransform(AbstractClassesTransformer.java:52)
	at com.tngtech.archunit.lang.AbstractClassesTransformer.transform(AbstractClassesTransformer.java:41)
	at com.tngtech.archunit.lang.ArchRule$Factory$SimpleArchRule.evaluate(ArchRule.java:180)
	at com.tngtech.archunit.lang.syntax.ObjectsShouldInternal.evaluate(ObjectsShouldInternal.java:76)
	at com.tngtech.archunit.library.freeze.FreezingArchRule.evaluate(FreezingArchRule.java:123)
	at org.gradle.architecture.test.FreezeInstructionsPrintingArchRule.evaluate(FreezeInstructionsPrintingArchRule.java:48)
	at com.tngtech.archunit.lang.ArchRule$Assertions.check(ArchRule.java:84)
	at org.gradle.architecture.test.FreezeInstructionsPrintingArchRule.check(FreezeInstructionsPrintingArchRule.java:32)
	at com.tngtech.archunit.junit.internal.ArchUnitTestDescriptor$ArchUnitRuleDescriptor.execute(ArchUnitTestDescriptor.java:166)
	at com.tngtech.archunit.junit.internal.ArchUnitTestDescriptor$ArchUnitRuleDescriptor.execute(ArchUnitTestDescriptor.java:149)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

I guess org.gradle.architecture.test.ArchUnitFixture might be doing something platform dependent, but I didn't check. I doubt this error is due to my changes though.

I'm also seeing a few failures when executing ./gradlew language-java:quickTest, though many of them seem to be related to Windows again or some build config that I am missing. I'm happy to look into actual errors once the general design discussion about this feature is done.

@ov7a
Copy link
Member

ov7a commented Sep 4, 2025

Thank you for your contribution!

The DCO check is failing, please look into that as we can't accept the PR otherwise.

For some reason, on Windows, the ForbiddenNullabilityAnnotationsTest

Yes, these are most likely Windows-related issues. Ignore them.

Please dump some details by commenting to the related issue

After the DCO issue is addressed, we'll send it to the team to confirm the implementation choices.

@ov7a ov7a added the pending:dco PR DCO check is failing, commits need to be signed off label Sep 4, 2025
@big-guy big-guy added in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures 👋 team-triage Issues that need to be triaged by a specific team labels Sep 5, 2025
@jvandort
Copy link
Member

jvandort commented Sep 8, 2025

I am not sure when we will be able to get to a review for this PR, however we think it is valuable to address the original issue.

Have you ran any performance benchmarks with this PR? Does it show noticeable improvements in those scenarios?

@jvandort jvandort removed the 👋 team-triage Issues that need to be triaged by a specific team label Sep 8, 2025
@github-actions github-actions bot removed the pending:dco PR DCO check is failing, commits need to be signed off label Sep 8, 2025
@ov7a ov7a removed the to-triage label Sep 9, 2025
@octylFractal octylFractal added the pending:feedback Indicates that changes or additional info are required, and the issue will be closed without them label Sep 10, 2025
@github-actions GitHub Actions
Copy link
Contributor

While we asked for changes to this PR, we received no reaction. If you provide the requested changes, we will consider this pull request again. In the meantime, closing as missing PR feedback.

@github-actions github-actions bot closed this Sep 24, 2025
@github-actions github-actions bot added closed:missing-feedback Feedback was requested but not provided in time and removed pending:feedback Indicates that changes or additional info are required, and the issue will be closed without them labels Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@octylFractal octylFractal Awaiting requested review from octylFractal octylFractal is a code owner automatically assigned from gradle/bt-jvm

Assignees

No one assigned

Labels

closed:missing-feedback Feedback was requested but not provided in time from:contributor PR by an external contributor in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Class-level compilation avoidance

6 participants

@beikov @ov7a @jvandort @octylFractal @big-guy @bot-gradle