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

Add the --enable-preview flag based on the Baseline-Enable-Preview jar manifest attribute #1365

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

iamdanfox
Copy link
Contributor

@iamdanfox iamdanfox commented Jul 29, 2022

Before this PR

I've recently (palantir/gradle-baseline#2322) added functionality to gradle-baseline so that folks can opt in to java's --enable-preview functionality by setting something like javaVersions { distributionTarget = '17_PREVIEW' }. Baseline applies the flag to all the compile time tasks, but we also need this to propagate to the production runtime jvm args.

We decided to convey the information using a jar attribute like Baseline-Enable-Preview: 17. This way if different jars tried to select conflicting runtime values we can provide clear errors.

See https://openjdk.org/jeps/12 for details of the behaviour of --enable-preview.

After this PR

==COMMIT_MSG==
createLaunchConfig task automatically adds the --enable-preview flag based on the Baseline-Enable-Preview jar manifest attribute
==COMMIT_MSG==

Possible downsides?

@changelog-app
Copy link

changelog-app bot commented Jul 29, 2022

Generate changelog in changelog/@unreleased

Type
See change types. Select one:

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

Description

createLaunchConfig task automatically adds the --enable-preview flag based on the Baseline-Enable-Preview jar manifest attribute

Check the box to generate changelog(s)

  • Generate changelog entry

entry -> entry.getValue().enablePreview().get(),
entry -> entry.getKey().getName(),
() -> MultimapBuilder.hashKeys().arrayListValues().build()))
.asMap();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is where I'd intuitively reach for streamex and an entrystream, but I think we've found that including too many deps in our gradle plugins causes sad times right?

+ JarManifestModuleInfo.ENABLE_PREVIEW_ATTRIBUTE + " attribute (" + enablePreviewVersion
+ ") from:\n" + Iterables.getOnlyElement(enablePreviewFromJar.values()));
return Stream.of("--enable-preview");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is pretty complicated, I think we want to stream through the jars, fail if we see any with ENABLE_PREVIEW_ATTRIBUTE != javaVersion, and add the flag if we've seen any ENABLE_PREVIEW_ATTRIBUTE attrs?

imo the Map<String, Collection<String>> enablePreviewFromJar makes things difficult to grok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay, I've updated to use less stream-y stuff... think this should be a bit more readable now!

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.

3 participants