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

Plumb Add-Exports through to execution and annotation processing sites #1944

Merged
merged 19 commits into from
Nov 3, 2021

Conversation

carterkozak
Copy link
Contributor

@carterkozak carterkozak commented Oct 27, 2021

Before this PR

Unable for jars to define the --add-exports options they need.
We added this to support safepoint metrics, however it would become unnecessary if we add a way to collect manifest entries in sls-packaging: https://github.com/palantir/sls-packaging/blob/7ac5d3bef6528fe36e8dc5a249c2540b7a7023c4/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/LaunchConfigTask.java#L63-L67

After this PR

==COMMIT_MSG==
Plumb Add-Exports through to execution sites
==COMMIT_MSG==

sls-packaging piece here: palantir/sls-packaging#1215
Ideas how we can deduplicate that code?

Possible downsides?

It's a bit funky, we don't have a way to pass this through to IDEs aside from intellij idea with gradle integration.
We're re-purposing the Add-Exports manifest entry in a way that's slightly different from how it was intended.

@changelog-app
Copy link

changelog-app bot commented Oct 27, 2021

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Plumb Add-Exports through to execution and annotation processing sites

Check the box to generate changelog(s)

  • Generate changelog entry

@carterkozak carterkozak changed the title [proof of concept idea] add-exports plugin to enable goethe Plumb Add-Exports through to execution and annotation processing sites Oct 29, 2021
Comment on lines +62 to +87
if (project.hasProperty("add.exports.to.javac")) {
project.getExtensions().getByType(SourceSetContainer.class).configureEach(sourceSet -> {
JavaCompile javaCompile = project.getTasks()
.named(sourceSet.getCompileJavaTaskName(), JavaCompile.class)
.get();
javaCompile
.getOptions()
.getCompilerArgumentProviders()
// Use an anonymous class because tasks with lambda inputs cannot be cached
.add(new CommandLineArgumentProvider() {
@Override
public Iterable<String> asArguments() {
// Annotation processors are executed at compile time
ImmutableList<String> arguments =
collectAnnotationProcessorExports(project, extension, sourceSet);
project.getLogger()
.debug(
"BaselineModuleJvmArgs compiling {} on {} with exports: {}",
javaCompile.getName(),
project,
arguments);
return arguments;
}
});
});
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block is ignored for now, hopefully something becomes of gradle/gradle#18824

Comment on lines +119 to +120
ImmutableList<String> arguments =
collectClasspathExports(project, extension, javaExec.getClasspath());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll need to duplicate this logic in sls-packaging, or figure out some other way to plumb args through

carterkozak added a commit to palantir/sls-packaging that referenced this pull request Oct 29, 2021
carterkozak added a commit to palantir/jvm-diagnostics that referenced this pull request Nov 2, 2021
See palantir/gradle-baseline#1944
This will allow other plugins to apply the export to jvm args
without special casing this library or `java.management` specifically.
carterkozak added a commit to palantir/jvm-diagnostics that referenced this pull request Nov 2, 2021
See palantir/gradle-baseline#1944
This will allow other plugins to apply the export to jvm args
without special casing this library or `java.management` specifically.
@carterkozak carterkozak marked this pull request as ready for review November 2, 2021 17:14
@policy-bot policy-bot bot requested a review from robert3005 November 2, 2021 17:14
bulldozer-bot bot pushed a commit to palantir/witchcraft-api that referenced this pull request Nov 3, 2021
###### _excavator_ is a bot for automating changes across repositories.

Changes produced by the roomba/latest-baseline-oss check.

# Release Notes
## 4.33.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | Plumb Add-Exports through to execution and annotation processing sites | palantir/gradle-baseline#1944 |



To enable or disable this check, please contact the maintainers of Excavator.
This was referenced Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants