Skip to content
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.

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

Use a project dependency for test->main dependency in Java plugin #6483

Open
oehme opened this issue Aug 23, 2018 · 10 comments
Open

Use a project dependency for test->main dependency in Java plugin #6483

oehme opened this issue Aug 23, 2018 · 10 comments
Labels
a:feature A new functionality in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures in:jvm-ecosystem
Milestone

Comments

@oehme
Copy link
Contributor

oehme commented Aug 23, 2018

Instead of doing sourceSets.test.compileClasspath += sourceSets.main.output, the Java plugin should use dependencies { testImplementation thisProject } and Gradle will automatically wire in the main variant of the project.

@oehme oehme added from:member a:chore Minor issue without significant impact in:jvm-ecosystem labels Aug 23, 2018
@oehme oehme added this to the 5.0 RC1 milestone Aug 23, 2018
@bigdaz
Copy link
Member

bigdaz commented Sep 29, 2018

While this change should work, it has some unwanted consequences due to the fact that our variant model for JVM components isn't fully fleshed out:

  • With the java plugin applied, this results in testCompile depending on jar (instead of just classes)
  • With the java-library plugin applied, testCompile correctly depends on classes (and doesn't require the main jar to be built). However the testRuntimeClasspath ends up requiring the jar for the project.

I'm sure we can solve these problems by more correctly modelling the variants when either the java or java-library plugin is installed. Currently, we don't model any classes variants for the java plugin, and only the api-classes variant with the java-library plugin.

I'm not sure if this is worth rushing into 5.0.

@bigdaz bigdaz self-assigned this Sep 29, 2018
@bigdaz bigdaz modified the milestones: 5.0 M1, 5.0 RC1 Sep 29, 2018
@bigdaz
Copy link
Member

bigdaz commented Sep 29, 2018

After a little bit more experimentation, here's what I think is required:

  1. A new outgoing variant Usage.RUNTIME_CLASSES_AND_RESOURCES on runtimeElements that has both the classes and resources directories as artifacts.
  2. Appropriate compatibility and disambiguation rules to choose this variant for a self-referencing project dependency when resolving the testRuntimeClasspath configuration.
  3. As stated in the main comment, change the testImplementation configuration to have a project dependency on self.

However, this has another implication: cross-project dependencies would be resolved the same way, so the test runtime classpath would now contain classes and resources directories for all dependent project, rather than jar files. This might be better, but would be a breaking change.

I think we'll want to do this in a less disruptive way, perhaps by adding rules that choose the new variant only for self-project dependencies, and not for cross-project dependencies.

@bigdaz
Copy link
Member

bigdaz commented Oct 1, 2018

@oehme Given my previous comments, do you still want to target this for 5.0?

@oehme
Copy link
Contributor Author

oehme commented Oct 1, 2018

I think using the JAR is actually better, as it is much faster to load on Windows than classes folders. For the same reason I'm also in favor of creating API JARs instead of classes folders for compilation, but that's another issue.

I don't think we need different behavior for local and downstream consumption. The price is small on Linux and on Windows it's actually faster to compile against a JAR.

I'm fine with doing this in another release, I just added it to 5.0 as it may break some hidden assumptions that some plugins might have.

@bigdaz
Copy link
Member

bigdaz commented Oct 1, 2018

But changing tests to have a local jar input instead of classes and resources would be a breaking change, and should probably be changed over a deprecation cycle: WDYT?

@oehme
Copy link
Contributor Author

oehme commented Oct 2, 2018

You mean by adding an opt-in and then nagging users to use it? That seems reasonable.

@oehme oehme modified the milestones: 5.0 RC1, 5.1 RC1 Oct 10, 2018
@bigdaz bigdaz removed their assignment Nov 5, 2018
@oehme oehme modified the milestones: 5.1 RC1, 5.2 RC1 Dec 5, 2018
@ljacomet ljacomet modified the milestones: 5.2 RC1, 5.3 RC1 Jan 21, 2019
@big-guy big-guy added the @core Issue owned by GBT Core label Jan 31, 2019
@big-guy big-guy modified the milestones: 5.3 RC1, 5.4 RC1 Jan 31, 2019
@big-guy big-guy removed this from the 5.4 RC1 milestone Mar 4, 2019
@ljacomet
Copy link
Member

Note that this is the root cause for #10872

@ljacomet ljacomet added @jvm and removed @jvm labels Feb 18, 2020
@jjohannes
Copy link
Contributor

We should make this configurable. And then we can change the default in a major (e.g. 8.0).

This should be considered when we pick #12912 back up.

@jjohannes jjohannes removed this from the 7.0 RC1 milestone Jan 18, 2021
@jjohannes jjohannes added a:feature A new functionality and removed @core Issue owned by GBT Core a:chore Minor issue without significant impact labels Jan 18, 2021
@jjohannes jjohannes removed the @jvm label Mar 22, 2021
@octylFractal octylFractal added in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures and removed in:dependency-management DO NOT USE labels Nov 22, 2021
@hakanai
Copy link

hakanai commented Mar 2, 2022

We already modify our test compile classpath to swap the main classes out for the jar, purely for the performance benefits. Not having to do that in our own scripts would be nice. :)

Also if you wanted a real performance boost, you could even skip generating the classes to the filesystem in the first place, and write them directly into the jar along with the resources. ;)

@big-guy big-guy added this to the 7.6 RC1 milestone Jul 14, 2022
@big-guy big-guy modified the milestones: 7.6 RC1, 8.0 RC1 Aug 9, 2022
@big-guy
Copy link
Member

big-guy commented Oct 21, 2022

@jvandort let's split this so we can put compileElements into 8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality in:java-plugins java-library, java, java-base, java-platform, java-test-fixtures in:jvm-ecosystem
Projects
None yet
Development

No branches or pull requests

8 participants