RFR: 8244212: Optionally download media and webkit libraries from latest openjfx EA build

Kevin Rushforth kcr at openjdk.java.net
Sun May 10 08:59:53 UTC 2020


On Thu, 30 Apr 2020 18:56:40 GMT, Jesper Skov <github.com+2720909+jskov at openjdk.org> wrote:

> I have tested on Linux (Fedora 31) only.
> It works as intended (with one test failure due to 15-ea+4 being too old now).
> 
> UPDATE_STUB_CACHE suggests there is some magic available to help manage the stub cache.
> But as I could not find anything about it, that section in the documentation may need some love.

Here are a few preliminary comments:

I filed the following JBS bug to track this enhancement:

https://bugs.openjdk.java.net/browse/JDK-8244212

When you are ready for a formal review, you can change the title to:

8244212: Optionally download media and webkit libraries from latest openjfx EA build

WEBKIT-MEDIA-STUBS.md line 22:

> 21:
> 22: * Unix libraries (*.so or *.dynlib files)
> 23: ````

typo: should be "dylib"

WEBKIT-MEDIA-STUBS.md line 24:

> 23: ````
> 24:     $projectDir/../caches/sdk/lib
> 25:     $projectDir/../caches/modular-sdk/modules_libs/$module

I would just list this one and not the other two. In particular, I don't want to encourage developers to copy these
libraries into the JDK since it could cause unexpected results if those libraries were loaded at runtime.

WEBKIT-MEDIA-STUBS.md line 36:

> 35:
> 36: ## Officially released libraries
> 37:

I recommend that you add a comment that gradle will download these for you from Maven Central.

By the same token, you could indicate in the above section that copying them to `../caches/sdk` is a manual step.

WEBKIT-MEDIA-STUBS.md line 53:

> 52:
> 53: Note that this is fine for local work. But a full test *is* required before submitting a PR, see CONTRIBUTING.md.

Can you add a hyperlink to CONTRIBUTING.md?

build.gradle line 1252:

> 1251:
> 1252: // Allows automatic provisioning of webkit+media shared libraries from an official OpenJfx build
> 1253: defineProperty("STUB_RUNTIME_OPENJFX", "")

If you are going to capitalize it, then it should be "OpenJFX".

Can you add "from Maven Central" ?

build.gradle line 1259:

> 1258: def String jdkStubRuntime = cygpath("$JDK_HOME")
> 1259: def String openjfxStubRuntime = cygpath("$projectDir") + "/build/openjfxStub"
> 1260:

I might suggest `buildSrc/build/openjfxStub` so it doesn't get removed by `gradle clean`.

build.gradle line 3391:

> 3390:         doFirst {
> 3391:             if (IS_STUB_RUNTIME_OPENJFX) {
> 3392:                 println "********************************************************"

Perhaps this should be inside the `if (!IS_COMPILE_WEBKIT)` ?

build.gradle line 4338:

> 4337:  *                                                                            *
> 4338:  *                             OpenJfx Stubs                                  *
> 4339:  *                                                                            *

OpenJFX

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

PR: https://git.openjdk.java.net/jfx/pull/202


More information about the openjfx-dev mailing list