On Tue, 18 Aug 2026 20:07:08 GMT, Marius Hanl <[email protected]> wrote:

>> Note for reviewers: Some testing was done in a follow-up PR, so worth 
>> checking as well: https://github.com/openjdk/jfx/pull/2144
>> 
>> Follow up documentation draft PR: https://github.com/openjdk/jfx/pull/2208
>> 
>> ---
>> 
>> This PR splits the maven publishing logic into an own file, 
>> `maven-publish.gradle`, that is next to the root `build.gradle`.
>> 
>> The `build.gradle` will apply the `maven-publish.gradle`. The 
>> `maven-publish.gradle` will then configure the Maven related properties and 
>> register all modules for publication.
>> 
>> This way, we decoupled the logic that much, the only things we need to do:
>> - apply from `maven-publish.gradle`
>> - Call `configureMavenPublication` later in the build chain.
>> 
>> To better understand the context, this is the commit where the whole Maven 
>> Publishing logic was introduced. I moved all of that logic out of the main 
>> `build.gradle`: 5a18677f
>> 
>> This will reduce the size by ~170 lines for the `build.gradle`.
>> 
>> Tested with:
>> - `./gradlew -PMAVEN_PUBLISH=true -PMAVEN_VERSION=custom publishToMavenLocal`
>> - `./gradlew -PMAVEN_PUBLISH=true publishToMavenLocal`
>> - `./gradlew -PMAVEN_PUBLISH=true -PMILESTONE_FCS=true publishToMavenLocal`
>> 
>> Everything still works:
>> -> Example: javafx.base from the local .m2 repository
>> <img width="1051" height="388" alt="image" 
>> src="https://github.com/user-attachments/assets/66ccd007-ff2c-4309-982a-1e8123e9f52a";
>>  />
>> 
>> I think this is a good step and an easy way to split out functionality 
>> without blowing things up. We might want to do that for other parts as well.
>> 
>> Note: I also fixed the deprecated `buildDir`, the deprecated `project.task` 
>> method and 2 warnings where it seems like he might not be able to infer the 
>> type (changing `def` to the actual type). 
>> -> The file is completely green, no warnings or deprecations.
>> Note2: I did a small improvement to the `addMavenPublication` method. It is 
>> now more 'typesafe', e.g. projects must be passed in directly, not as 
>> String. If a project does not exist, the build will fail with an exception.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Marius Hanl has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 16 additional 
> commits since the last revision:
> 
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 
> 833146-mvn-publish
>  - fix gradle warning
>  - Also match :all task name as outlined in Gradle best practices
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 
> 833146-mvn-publish
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 
> 833146-mvn-publish
>  - all task build the publication now
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 
> 833146-mvn-publish
>  - Use defineProperty and fail method from default build.gradle
>  - Depend maven publishing on the build modules task
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 
> 833146-mvn-publish
>  - ... and 6 more: https://git.openjdk.org/jfx/compare/caa81655...ff79685e

Sorry for the delay. I ran a build on our CI system and all looks good. I left 
one comment about a copyright year and will reapprove if you change it.

I also ran a pair of builds -- one from `master` and one from this PR branch 
(with master merged in) -- using `SOURCE_DATE_EPOCH` and they are identical, 
aside from a known issue that the generated `.bss` files in the 
`javafx.controls` module are not reproducible.

Please wait for either @johanvos or @tiainen

buildSrc/src/main/groovy/com/sun/javafx/gradle/CompileTarget.java line 2:

> 1: /*
> 2:  * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.

2026

maven-publish.gradle line 34:

> 32: 
> 33: defineProperty("MAVEN_PUBLISH", "false")
> 34: ext.IS_MAVEN_PUBLISH = Boolean.parseBoolean(MAVEN_PUBLISH) || 
> isAllTaskRequested()

This flag was formerly `false` by default even when running `gradle all`. Are 
there other side effect to now setting this flag to `true`? Did you consider a 
separate flag that qualifies the creation of the `publications` dir? It's 
probably OK as-is, since I didn't see any issues with the build.

-------------

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1970#pullrequestreview-4976267060
PR Review Comment: https://git.openjdk.org/jfx/pull/1970#discussion_r3816326340
PR Review Comment: https://git.openjdk.org/jfx/pull/1970#discussion_r3816400884

Reply via email to