8389584: Allow a shared Gradle properties

[nlisker]

Allows using a Gradle properties file at the project level by requiring the user-specific one to be present in the user's Gradle dir. A Gradle properties file is necessary for a proper Gradle build system's configuration. The user file takes precedence over the project one.

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8389584: Allow a shared Gradle properties (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2240/head:pull/2240
$ git checkout pull/2240

Update a local copy of the PR:
$ git checkout pull/2240
$ git pull https://git.openjdk.org/jfx.git pull/2240/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2240

View PR using the GUI difftool:
$ git pr show -t 2240

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2240.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back nlisker! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[nlisker]

@kevinrushforth Please review. Someone from Gluon might also want to take a look.

A notification on the mailing list prior to integration could help give a heads-up with regards to the git conflict that may arise.

[openjdk]

The total number of required reviews for this PR has been set to 2 based on the presence of this label: rfr. This can be overridden with the /reviewers command.

[mlbridge]

Webrevs

• 02: Full - Incremental (4f4b1af8)
• 01: Full - Incremental (01e12841)
• 00: Full (cb45860a)

[AnirvanSarkar]

I have a concern about requiring OpenJFX specific configuration in the user-level ~/.gradle/gradle.properties.

This makes OpenJFX build properties visible to every Gradle build on the machine.
In particular, generically named project defined properties such as JDK_HOME, CONF, INCREMENTAL, and LINT could conflict with unrelated projects or plugins.

It also prevents developers from keeping OpenJFX only Gradle settings scoped to this repository, for example:

org.gradle.java.home=C:/JDK/jdk-25.0.1
org.gradle.dependency.verification=lenient
org.gradle.daemon=false

Is there a reason we cannot continue to support a project local, gitignored gradle.properties instead?
This would still allow each developer to configure their own environment while keeping the configuration isolated to the OpenJFX build.

[andy-goryachev-oracle]

I have the same concern as @AnirvanSarkar

[nlisker]

This makes OpenJFX build properties visible to every Gradle build on the machine.
In particular, generically named project defined properties such as JDK_HOME, CONF, INCREMENTAL, and LINT could conflict with unrelated projects or plugins.

It also prevents developers from keeping OpenJFX only Gradle settings scoped to this repository, for example:

These are valid concerns.

Is there a reason we cannot continue to support a project local, gitignored gradle.properties instead?

As explained above, it is required for project configuration such as the toolchain and build properties, e.g.,:

org.gradle.caching=true
org.gradle.parallel=true
org.gradle.configuration-cache=true

Having this file gitignored means we can't improve the build (we're stuck with Gradle 2 or 3). It is a committed file in every project I've seen (I can make a list if it helps). Figuratively, the current setup "hijacks" the file.
What we can do is add an uncommitted local.properties instead with user properties. It'll be read during configuration and replace the current user-specified gradle.properties (JDK_HOME, CONF, INCREMENTAL). This is also what's being done in some repos.
The org.gradle* properties that are required before configuration time (like the deamon switch) can be passed as command flags. For example, the daemon is very useful when developing, but can be disabled for CI/CD, so this is a good option to have on be default and turned off in the CI/CD script (note that OpenJFX builds without config flags). A GRADLE_OPTS env var can aggregate them if it helps.
Also, git allows locally untracked files if the user really doesn't want the out-of-the-box one, but I don't think it's a good option.

It boils down to what the purpose of gradle.properties here is. If it's part of the build, like the build.gradle and settings.gradle files are (and it usually is), then it needs to contain the build configuration that matches the capabilities of these files. If it's a user-configuration, then the user needs to know what can be turned on. For example, making the build file support parallel execution will do nothing until the user "figures out" they need to create a gradle.properties locally that enables it. This is a rather terrible user experience, especially for features that have been mainstay for many years now.

I'd like to know which gradle settings users want to change and why.

[AnirvanSarkar]

Thanks for the clarification.

Using a gitignored gradle-local.properties for CONF and other OpenJFX specific settings works for me.

I suggest gradle-local.properties rather than local.properties, since the latter is conventionally associated with Android builds [1].

The existing gradle.properties.template could therefore be renamed to gradle-local.properties.template.

The settings I plan to override locally are:

org.gradle.dependency.verification=lenient
org.gradle.daemon=false

Other developers may also want to override settings such as:

org.gradle.jvmargs=-Xmx...

The dependency verification override is needed in my workflow because I usually do not compile the WebKit and Media sources. Instead, I set the STUB_RUNTIME_OPENJFX Gradle property so that the WebKit and Media libraries are downloaded automatically from Maven Central [2].

Because the SHA-256 hashes of these stub runtimes are not currently included in gradle/verification-metadata.xml, I need to set: org.gradle.dependency.verification=lenient.

If SNAPSHOT builds of OpenJFX were available, this override would not be necessary [3].

Gradle daemon is disabled due to known issues on Windows [4][5].

Building OpenJFX wiki page [6] would also require an update if this pull request is merged.

---

[1] - https://developer.android.com/build#properties-files
[2] - https://github.com/openjdk/jfx/blob/28%2B2/WEBKIT-MEDIA-STUBS.md#officially-released-libraries
[3] - https://mail.openjdk.org/archives/list/openjfx-dev@openjdk.org/thread/OPTPHQLEGL7DB3MGKWHM2L4DHTB7JBHZ/
[4] - https://bugs.openjdk.org/browse/JDK-8290976?focusedId=14512413&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14512413
[5] - https://bugs.openjdk.org/browse/JDK-8359899
[6] - https://wiki.openjdk.org/display/OpenJFX/Building+OpenJFX

[nlisker]

The settings I plan to override locally are:

org.gradle.dependency.verification=lenient
org.gradle.daemon=false

To make sure we're on the same page, as explained above, these options can't be overridden in a local properties file that is read during the build configuration phase because they are used before the configuration phase. Those would have to be passed in as command flags (however they can be aggregated into one flag).

[AnirvanSarkar]

Yes, that's my understanding as well.

[nlisker]

@AnirvanSarkar Try this branch if you're able to and see if it answers your needs. Thanks.

[AnirvanSarkar]

@nlisker

Thanks for the changes!

I noticed one behavior change around overriding OpenJFX project properties from the command line using -P.

For example, with the following in gradle-local.properties:

CONF = Release

Running:

./gradlew -PCONF=DebugNative properties | grep CONF

Expected:

CONF: DebugNative
CONF: DebugNative

Current output:

CONF: Release
CONF: Release

I believe it would be preferable for the command-line value (-PCONF=DebugNative) to take precedence over the value specified in gradle-local.properties, to be consistent with the existing behavior.

One possible way to preserve this behavior could be to make localProps global and have defineProperty(String name, String defaultValue) delegate to defineProperty(String name, Properties props, String defaultValue), passing localProps.

[nlisker]

I believe it would be preferable for the command-line value (-PCONF=DebugNative) to take precedence over the value specified in gradle-local.properties, to be consistent with the existing behavior.

True. It should work now.

[kevinrushforth]

I'll want to do both a few local tests and a CI build to ensure that it doesn't cause problems.

@arapte Can you also review?