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