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

Upgradle to 8.2-rc-2 #3351

Merged
merged 1 commit into from
Jun 15, 2023
Merged

Upgradle to 8.2-rc-2 #3351

merged 1 commit into from
Jun 15, 2023

Conversation

marcphilipp
Copy link
Member

No description provided.

@marcphilipp
Copy link
Member Author

Blocked by bndtools/bnd#5695

tresat added a commit to tresat/junit5 that referenced this pull request Jun 14, 2023
Eagerly configure Jar tasks to create the BundleTaskExtension prior to the Gradle compileClasspath configuration becoming locked and triggering it's beforeLocking callback to run.  This allows the BND extension to modify the LibraryElements attribute on that configuration and request a variant with the "jar" value prior to Gradle automatically modifying the attribute to request the "classes" value.  This ensures jars are present on the classpath for BND to use when checked for exported classes to use when building the manifest.  If it only uses the classes directory for this task, it will not have access to version information and will not write version info to the manifest it is assembling for the current project, causing the verifyOSGi task to fail.

Addresses issue bndtools/bnd issue #5695 and allows merging of junit-team/junit5 pr junit-team#3351.
@tresat
Copy link
Contributor

tresat commented Jun 14, 2023

Hi Marc, after debugging yesterday, Sterling and I determined that this failure is caused by a change in what was thought was merely an internal behavior in Gradle 8.2. However, BND is unfortunately relying on the old behavior.

I'm going to submit a PR to BND next containing a proper fix for them. The PR here will workaround this issue and should work with Gradle 8.1.x as well as >= 8.2-rc-2. It will eagerly configure the Jar tasks in the JUnit 5 build to avoid the timing issue causing this failure, at the expense of only some minor unnecessary configuration when not running these tasks in a build.

What's meant to be happening is this:

  1. In the constructor for BundleTaskExtension BND calls the setSourceSet method here.
  2. This calls the jarLibraryElements method passing the main compileClasspath configuration. This method attempts to set the LIBRARY_ELEMENTS_ATTRIBUTE to the value jar here.
  3. Using the value jar during dependency resolution ensures that the junit-platform-runner project gathers jar files built by other local JUnit subprojects it depends upon in its compileClasspath.
  4. When BND is looking to build the list in the Import-Package property in the MANIFEST.MF file it creates, it reads this version from the Export-Package property in dependencies' MANIFEST.MF files. This manifest file is only available on the compileClasspath to read if the depednencies' jar files containing it are present. If the classes variant of the dependencies is selected rather than the standard variant containing jar files, only the compiled classes are available on the classpath, BND won't be able to determine version information to write.

What's happening now after the change:

A change in Gradle 8.2 to DefaultRootComponentMetadataBuilder#getRootComponentMetadata here caused this process to fail. Previously, we had looked at every configuration and its artifacts when building the root metadata when building the dependencies graph. This caused the Jar task building those artifacts to be realized, which caused the BundleTaskExtension to run, which caused the jar value for LibraryElements to be set, which allowed jars to be selected in the compileClasspath, which was needed for BND to see the versions to write as described above.

Due to the now lazier behavior when building the root metadata, Jar tasks that are required to build the artifacts supplied by a configuration are no longer realized prior to the beforeLocking callback running for those configurations.

This is meaningful because by default, Gradle would substitute the secondary classes variant that supplies the directory containing the compiled classes for local project dependencies. This would allow the compilation of this project to begin slightly earlier, without waiting for its local project dependencies' library files to be assembled first. Gradle accomplished this here. (Note that this method was just removed from master - the fix in the PR I linked will work with or without it present).

When the beforeLocking callback does run, the if check sees a value already is present for LibraryElements and if so, does nothing. So that value is already jar, the callback won't set it to classes.

Summary

Whether BND works or not ultimately comes down to whether Jar tasks are realized before or after the beforeLocking callback runs.

This is clearly not a requirement a build author should be thinking about, especially as the the order that configurations are iterated in beforeLocking is also meaningful here. Fro instance, if the compileClasspath had been named such that it came alphabetically ahead of archives (for instance aCompileClasspath), then this behavior would never have worked in Gradle 8.1.1, since then the beforeLocking callback also would run prior to the Jar task being realized, and we'd see the same behavior as in Gradle 8.2.

Hopefully this makes sense and this fix works for now, until BND can be also be fixed.

marcphilipp pushed a commit that referenced this pull request Jun 15, 2023
Eagerly configure Jar tasks to create the BundleTaskExtension prior to
the Gradle compileClasspath configuration becoming locked and
triggering it's beforeLocking callback to run. This allows the BND
extension to modify the LibraryElements attribute on that configuration
and request a variant with the "jar" value prior to Gradle
automatically modifying the attribute to request the "classes" value.
This ensures jars are present on the classpath for BND to use when
checked for exported classes to use when building the manifest. If it
only uses the classes directory for this task, it will not have access
to version information and will not write version info to the manifest
it is assembling for the current project, causing the verifyOSGi task
to fail.

Addresses issue bndtools/bnd#5695 and allows merging of #3351.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants