Skip to content

Conversation

@carterkozak
Copy link
Contributor

@carterkozak carterkozak commented Oct 29, 2021

This provides the same functionality as
palantir/gradle-baseline#1944

We're piggy-backing off the Add-Exports manifest property to collect exports required for any given application based on the libraries it uses.

==COMMIT_MSG==
Apply --add-exports to Java 16+ services based on manifest entries
==COMMIT_MSG==

@changelog-app
Copy link

changelog-app bot commented Oct 29, 2021

Generate changelog in changelog/@unreleased

Type

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

Description

Apply --add-exports to Java 16+ services based on manifest entries

Check the box to generate changelog(s)

  • Generate changelog entry

* Utility functionality which mirrors {@code Add-Exports} plumbing from
* <a href="https://github.com/palantir/gradle-baseline/pull/1944">gradle-baseline#1944</a>.
*/
final class ModuleExports {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be nice to to also have a small explanation here instead of just linking to the PR. Something similar to what you added in:

https://github.com/palantir/gradle-baseline/blob/1a052650da4af39680a7dd3f790a5b291086158e/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineModuleJvmArgs.java#L42-L47

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, updated!

Copy link
Contributor

@fawind fawind left a comment

Choose a reason for hiding this comment

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

Looks good, I think the code duplication with the gradle-baseline feature is actually not too bad!

classpath.getFiles().stream().flatMap(file -> {
try {
if (file.getName().endsWith(".jar") && file.isFile()) {
JarFile jar = new JarFile(file);
Copy link
Contributor

Choose a reason for hiding this comment

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

driveby: shouldn't JarFiles be in a try-with-resources block?

    public void close() throws IOException {
        if (closeRequested) {
            return;
        }
        closeRequested = true;

        synchronized (this) {
            // Close streams, release their inflaters, release cached inflaters
            // and release zip source
            try {
                res.clean();
            } catch (UncheckedIOException ioe) {
                throw ioe.getCause();
            }
        }
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good shout, we should have an error-prone rule for this!

@bulldozer-bot bulldozer-bot bot merged commit d1578b5 into develop Nov 3, 2021
@bulldozer-bot bulldozer-bot bot deleted the ckozak/add_exports branch November 3, 2021 14:48
@svc-autorelease
Copy link
Collaborator

Released 7.7.0

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.

6 participants