| Message ID | 20200517132829.1553311-1-levenson@mmer.org |
|---|---|
| Headers | show |
Ludovic Courtès <ludo@gnu.org> writes: > Hi Alexey, > > Thanks for the patch series! > > Alexey Abramov <levenson@mmer.org> skribis: > >> * gnu/packages/java.scm (java-openjfx-build)[source]: Use it. >> * gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch: New file. >> * gnu/local.mk (dist_patch_DATA): Add it. > > [...] > >> +++ b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch >> @@ -0,0 +1,37 @@ > > Could you shorten this file name a bit? (‘guix lint’ probably complains.) Sure. By the way it complains on other derrived packages like java-openjfx-base which inherits java-openjfx-build. /home/levenson/factory/guix/gnu/packages/java.scm:2486:12: java-openjfx-build@8.202: source not archived on Software Heritage /home/levenson/factory/guix/gnu/packages/java.scm:2533:2: java-openjfx-base@8.202: file names of patches should start with the package name /home/levenson/factory/guix/gnu/packages/java.scm:2537:0: java-openjfx-base@8.202: line 2537 is way too long (111 characters) /home/levenson/factory/guix/gnu/packages/java.scm:2552:0: java-openjfx-base@8.202: line 2552 is way too long (99 characters) /home/levenson/factory/guix/gnu/packages/java.scm:2533:2: java-openjfx-base@8.202: source not archived on Software Heritage > >> +From d156313340d3edcd78751ea8ed5e66c7668275c3 Mon Sep 17 00:00:00 2001 >> +From: Alexey Abramov <levenson@mmer.org> >> +Date: Sun, 8 Mar 2020 20:07:22 +0100 >> +Subject: [PATCH] openjfx: Determine the version of Java in JDK_HOME > > In general I think you can omit the first three lines and just keep the > subject. Ah, great. > >> +Icedtea contains guix in its version, so build.gradle failes to run. Openjfx >> +packaging is not trivial, so you will probably need to try build it with >> +gradlew. > > Otherwise LGTM. -- Alexey
Ludovic Courtès <ludo@gnu.org> writes: > Alexey Abramov <levenson@mmer.org> skribis: > >> --- >> gnu/packages/java.scm | 643 +++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 635 insertions(+), 8 deletions(-) > > Please add a commit log. > >> + (java-aux-class? >> + (lambda (class) >> + (string-match "\\$[0-9]+.class" class))) > > Probably “\\.class” instead of “.class”? Ah, that sneaky dot. Thanks! > >> + (add-after 'chdir-to-a-module-directory 'compile-prism-compilers >> + (lambda _ > > Maybe add a comment explaining why this phase is necessary. Well, you know, I managed to eliminate it. Thanks! > >> + (let ((prism-compilers-classes "build/classes/jsl-compilers/prism") >> + (prism-src "src/main/jsl-prism/")) >> + (mkdir-p prism-compilers-classes) >> + (apply invoke `("javac" >> + "-d" ,prism-compilers-classes >> + ,@javac/h-flags >> + ,@(find-files prism-src "\\.java")))) > > This can be shortened as: > > (apply invoke "javac" "-d" prism-compilers-classes > (append javac/h-flags (find-files prism-src "\\.java"))) > > These two comments apply to the other phases too. > > More generally, why are there so many phases? How do people normally > build it? Gradle? > > Would it make sense to factorize helpers in a new (gnu build > java-openjfx) module or similar? You are right, javafx builds with Gradel. I basically replicated gradle stages, by inspecting ./gradlew build ... --info The reason I didn't start building any helpers is because OpenJFX 8u and OpenJFX 9u or later use different instuctions as it states here [1]. I don't know if these helpers will apply to 9u, also having a (gnu build gradle) would eliminate the need in helpers as well. In any case I think it won't be a problem to do that with a second iteration of openjfx build. -- Alexey Footnotes: [1] https://wiki.openjdk.java.net/display/OpenJFX/Building+OpenJFX+8u
Ludovic Courtès <ludo@gnu.org> writes: > Alexey Abramov <levenson@mmer.org> skribis: > >> --- >> gnu/packages/java.scm | 165 ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 165 insertions(+) > > Please add a commit log. > > [...] > >> + (add-after 'build 'generate-headers >> + (lambda* (#:key inputs #:allow-other-keys) > > Could you add a comment explaining what this phase does and why we’re > doing all this? (Same for the other phases.) Done. > >> + (let* ((module (string-append (getcwd) "/modules/web")) >> + (classes (string-append module "/build/classes/java/main")) >> + (web-generated-headers (string-append module "/build/generated-src/headers")) >> + (web-generated-classes '("com.sun.webkit.ContextMenu" >> + "com.sun.webkit.ContextMenuItem" >> + "com.sun.webkit.CursorManager" > > Perhaps add a comment explaining how you obtained this list of classes > so that our future selves know how to update it. > > Since this is a Web thing, could you check whether it contains pre-built > (“minified”) JavaScript? If it does, that’s a problem because we > generally provide only software built from source. We build not only a web component here, but webkit as well. Webkit requires some extra 3rd party libraries icu, libxml, libxslt and sqlite which are already in openjfx source bundle. We also build them now. One thing to note here, I have tested openjfx only with davmail in order to make oauth2 works. There is one exception during the authorization. When oauth accepts your password and waits for you to approve from another device, it changes the form and tries to shadow some captions, and those captions just don't show up during that period of time. Don't know what to do here. I am not a java developer, so probably it requires more testing. I was planning to talk about on openjfx mailing list, but time didn't let me. --8<---------------cut here---------------start------------->8--- Error: CPU/SIMD peer not found for: Blend_SRC_IN due to error: com.sun.scenario.effect.impl.sw.sse.SSEBlend_SRC_INPeer java.lang.RuntimeException: Could not create peer Blend_SRC_IN for renderer com.sun.scenario.effect.impl.prism.sw.PSWRenderer@6218eb01 at com.sun.scenario.effect.impl.Renderer.getPeerInstance(Unknown Source) at com.sun.scenario.effect.CoreEffect.getPeer(Unknown Source) at com.sun.scenario.effect.CoreEffect.getPeer(Unknown Source) at com.sun.scenario.effect.CoreEffect.filterImageDatas(Unknown Source) at com.sun.scenario.effect.Blend.filterImageDatas(Unknown Source) at com.sun.scenario.effect.FilterEffect.filter(Unknown Source) at com.sun.scenario.effect.impl.prism.PrEffectHelper.render(Unknown Source) at com.sun.javafx.webkit.prism.WCGraphicsPrismContext$ClipLayer.render(Unknown Source) at com.sun.javafx.webkit.prism.WCGraphicsPrismContext.renderLayer(Unknown Source) at com.sun.javafx.webkit.prism.WCGraphicsPrismContext.restoreStateInternal(Unknown Source) at com.sun.javafx.webkit.prism.WCGraphicsPrismContext.restoreState(Unknown Source) at com.sun.webkit.graphics.GraphicsDecoder.decode(Unknown Source) at com.sun.webkit.graphics.WCRenderQueue.decode(Unknown Source) at com.sun.webkit.WebPage.paint2GC(Unknown Source) at com.sun.webkit.WebPage.paint(Unknown Source) at com.sun.javafx.sg.prism.web.NGWebView.renderContent(Unknown Source) at com.sun.javafx.sg.prism.NGNode.doRender(Unknown Source) at com.sun.javafx.sg.prism.NGNode.render(Unknown Source) at com.sun.javafx.sg.prism.NGGroup.renderContent(Unknown Source) at com.sun.javafx.sg.prism.NGRegion.renderContent(Unknown Source) at com.sun.javafx.sg.prism.NGNode.doRender(Unknown Source) at com.sun.javafx.sg.prism.NGNode.render(Unknown Source) at com.sun.javafx.tk.quantum.ViewPainter.doPaint(Unknown Source) at com.sun.javafx.tk.quantum.ViewPainter.paintImpl(Unknown Source) at com.sun.javafx.tk.quantum.UploadingPainter.run(Unknown Source) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) at com.sun.javafx.tk.RenderJob.run(Unknown Source) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.run(Unknown Source) at java.lang.Thread.run(Thread.java:748) --8<---------------cut here---------------end--------------->8--- -- Alexey
Hi Alexey, Alexey Abramov <levenson@mmer.org> skribis: > Ludovic Courtès <ludo@gnu.org> writes: [...] >>> + (let* ((module (string-append (getcwd) "/modules/web")) >>> + (classes (string-append module "/build/classes/java/main")) >>> + (web-generated-headers (string-append module "/build/generated-src/headers")) >>> + (web-generated-classes '("com.sun.webkit.ContextMenu" >>> + "com.sun.webkit.ContextMenuItem" >>> + "com.sun.webkit.CursorManager" >> >> Perhaps add a comment explaining how you obtained this list of classes >> so that our future selves know how to update it. >> >> Since this is a Web thing, could you check whether it contains pre-built >> (“minified”) JavaScript? If it does, that’s a problem because we >> generally provide only software built from source. > > We build not only a web component here, but webkit as well. Webkit requires some extra 3rd party libraries icu, libxml, libxslt and sqlite which are already in openjfx source bundle. We also build them now. Oh sorry, I had overlooked that and it’s problematic. The policy in Guix is to not use bundled software: it’s a waste of resources (especially for large pieces of software like WebKit), but more importantly it’s a security issue (nobody will notice if ‘java-openjfx-web’ includes an outdated, vulnerable version of WebKit). So the solution is to: 1. Pass the relevant flags to tell the build system to use the “system” copies of these libraries. If there’s no such flag, patch the build system to not try to build these things. 2. Add a snippet to remove bundled copies of these libraries, to make sure they’re not picked up by mistake. Sometimes it’s just a build flag, and sometimes it’s harder. If it turns out to be difficult, let us know, and perhaps we can find an interim solution. Could you take a look? Let me know what you think! Thanks, Ludo’.
This is an updated series of patches for openjfx. We are now building WebKit java port and use Guix inputs, instead of embedded source code bundles ship with openjfx. Tests are still disabled. I left round brackets in java-openjfx-build package on a separate line, just to make diffs look cleaner. Is it OK? This is not an ideal build of course. Media component is missing for example. It would be also great to build a complete jfxrt.jar and not expose java-openjfx-<component> vars at all. I have also checked the webkitgtk package while doing this. I found that some patches apply to openjfx as well. Don't know if it's possible to use webkit.org sources for openjfx-web. In the meantime davmail and 2fa works :). Please let me know what you think. Alexey Abramov (12): gnu: java-openjfx-graphics: Implement a complete compilation. gnu: java-openjfx-build: Fix openjfx-swing component build for JDKu8 gnu: java-openjfx-swing: Add new variable. gnu: java-openjfx-swt: Add new variable. gnu: java-openjfx-controls: Add new variable. gnu: java-openjfx-build: Patch DumpRenderTree for the web component. gnu: java-openjfx-build: Do not use an embedded 3rd party libraries. gnu: java-openjfx-build: Fix web component compilation with ICU 59+. gnu: java-openjfx-build: Fix web component linkage. gnu: java-openjfx-build: Fix web component compilation with ICU 65+. gnu: java-openjfx-build: Ensure use of system provided libraries. gnu: java-openjfx-web: Add new variable. gnu/local.mk | 6 + gnu/packages/java.scm | 871 +++++++++++++++++- ...va-openjfx-build-swing-JFXPanel-JDK8.patch | 60 ++ ...penjfx-build-web-DumpRenderTree-java.patch | 30 + ...-openjfx-build-web-OptionsJava.cmake.patch | 32 + .../patches/java-openjfx-build-web-WTF.patch | 33 + ...-build-web-WebKitCompilerFlags.cmake.patch | 36 + .../patches/java-openjfx-build-web-icu.patch | 38 + 8 files changed, 1097 insertions(+), 9 deletions(-) create mode 100644 gnu/packages/patches/java-openjfx-build-swing-JFXPanel-JDK8.patch create mode 100644 gnu/packages/patches/java-openjfx-build-web-DumpRenderTree-java.patch create mode 100644 gnu/packages/patches/java-openjfx-build-web-OptionsJava.cmake.patch create mode 100644 gnu/packages/patches/java-openjfx-build-web-WTF.patch create mode 100644 gnu/packages/patches/java-openjfx-build-web-WebKitCompilerFlags.cmake.patch create mode 100644 gnu/packages/patches/java-openjfx-build-web-icu.patch -- 2.26.2
True. I have sent a new series with fixes. This move allowed me to drop off gcc-5, and switch back to the current one. It is also shed some light on what should/could be investigated/done to make tests work. -- Alexey
Hi,
could you please consider reviewing patches to finalize the building
of the java-openjfx packages in scope of this discussion, as part of
an attempt to complete Alexey's awesome work.
The thread had stopped receiving updates for some time now. I assume
priorities were shifted elsewhere.
I recently got interested testing an openjfx application on Java 1.8,
only to realise that the current packages are incomplete. I came
across this thread, and thought to give the patches a try, and fix
where possible.
I believe I have finalised all openjfx packages in scope to build,
pass all tests and seamlessly link to their native libraries at
runtime.
My test application now works, I can use the graphics, controls and
web openjfx packages by merely specifying their share/java jar
packages on the classpath.
This email has three incremental patches. I've named the as [PATCH v5]
to denote this is a continuation from the last patches.
0001: Alexey's work brought forward to a recent guix revision.
0002: Small changes to updated the above to build with the current
guix spec/packages.
0003: My update to finalize the openjfx packages, normalize inputs,
run all tests where they exist and fix the load path of native
libraries at runtime.
I've put the author of 0001 down to Alexey since this is his works. I
compiled this patch by hand from the thread, I hope I haven't missed
anything.
The individual packages produce a jar output, and some of them also
build native shared libraries. The individual jar packages try to load
their native libraries just by name, and this will fail if the native
libs are not on the library path.
I've taken the decision to load these libraries by their full
gnu/store path instead of their names. I have also applied the same to
an external package, java-swt, since it has a shared library that has
to be located at runtime. Although there are a few native libraries
built by the java-openjfx-graphics package I left it as is, because it
has a sophisticated native library loader that locates shared
libraries relative to the jar location.
The changes in 0003 are as follows:
* java-openjfx-graphics
1. Normalize inputs, only build against gtk-2.0.
2. Output native libs to openjfx standard share/amd64 dir.
3. Do not build libjpeg from source, but rather use corresponding
package.
4. Run all tests.
* java-openjfx-controls
1. Run all tests.
2. Fix java-openjfx-graphics name typo.
* java-openjfx-media
1. Build against gstreamer.
2. Compile native library and modify loader to load it directly from
the store path.
* java-openjfx-web
1. Include version.
2. Modify loader to load native web kit lib directly from the store.
3. Fix a failing test, by applying new
java-openjfx-build-web-js-test.patch as picked up from a later
openjfx revision source code revision, to fix a failing test.
4. Run all tests.
* java-swt
1. Modify loader to load native library directly from the gnu/store
path if it can't be found otherwise. This is required for
seamlessly loading java-swt as an input.
This is my first guix package update, so is likely to require more
work based on your feedback, which is most welcome.
Thanks
Hi!
I think your patches are a bit difficult to review. I'll have a proper look this evening, but for now two things I noticed:
You declare configure flags as a list of flags, but at some point you have:
"-DCMAKE_CXX_FLAGS=\""
…
"\""
In the list. I think it's incorrect because they would be passed as separate arguments (though it might work when parsed by the program). I think you should zake it a single argument with string-append.
Also, we should now use the new style inputs for packages when possible:
(list foo bar) instead of `(("foo" ,foo) ("bar" ,bar))
Thanks!
On June 19, 2022 9:26:32 PM GMT+02:00, Ioannis Kappas <ioannis.kappas@gmail.com> wrote:
>Hi,
>
>could you please consider reviewing patches to finalize the building
>of the java-openjfx packages in scope of this discussion, as part of
>an attempt to complete Alexey's awesome work.
>
>The thread had stopped receiving updates for some time now. I assume
>priorities were shifted elsewhere.
>
>I recently got interested testing an openjfx application on Java 1.8,
>only to realise that the current packages are incomplete. I came
>across this thread, and thought to give the patches a try, and fix
>where possible.
>
>I believe I have finalised all openjfx packages in scope to build,
>pass all tests and seamlessly link to their native libraries at
>runtime.
>
>My test application now works, I can use the graphics, controls and
>web openjfx packages by merely specifying their share/java jar
>packages on the classpath.
>
>This email has three incremental patches. I've named the as [PATCH v5]
>to denote this is a continuation from the last patches.
>
>0001: Alexey's work brought forward to a recent guix revision.
>
>0002: Small changes to updated the above to build with the current
>guix spec/packages.
>
>0003: My update to finalize the openjfx packages, normalize inputs,
>run all tests where they exist and fix the load path of native
>libraries at runtime.
>
>I've put the author of 0001 down to Alexey since this is his works. I
>compiled this patch by hand from the thread, I hope I haven't missed
>anything.
>
>The individual packages produce a jar output, and some of them also
>build native shared libraries. The individual jar packages try to load
>their native libraries just by name, and this will fail if the native
>libs are not on the library path.
>
>I've taken the decision to load these libraries by their full
>gnu/store path instead of their names. I have also applied the same to
>an external package, java-swt, since it has a shared library that has
>to be located at runtime. Although there are a few native libraries
>built by the java-openjfx-graphics package I left it as is, because it
>has a sophisticated native library loader that locates shared
>libraries relative to the jar location.
>
>The changes in 0003 are as follows:
>
>* java-openjfx-graphics
> 1. Normalize inputs, only build against gtk-2.0.
> 2. Output native libs to openjfx standard share/amd64 dir.
> 3. Do not build libjpeg from source, but rather use corresponding
> package.
> 4. Run all tests.
>* java-openjfx-controls
> 1. Run all tests.
> 2. Fix java-openjfx-graphics name typo.
>* java-openjfx-media
> 1. Build against gstreamer.
> 2. Compile native library and modify loader to load it directly from
> the store path.
>* java-openjfx-web
> 1. Include version.
> 2. Modify loader to load native web kit lib directly from the store.
> 3. Fix a failing test, by applying new
> java-openjfx-build-web-js-test.patch as picked up from a later
> openjfx revision source code revision, to fix a failing test.
> 4. Run all tests.
>* java-swt
> 1. Modify loader to load native library directly from the gnu/store
> path if it can't be found otherwise. This is required for
> seamlessly loading java-swt as an input.
>
>This is my first guix package update, so is likely to require more
>work based on your feedback, which is most welcome.
>
>Thanks
Hi I am glad that you find these patches helpful. I tried to apply your finalized patches, but seems like java-openjfx-build-web-js-test.patch is still missing. Regarding the patches themselves, I agree with Ludo. They are indeed hard to maintain. The best approach here would be to pack gradle, so we could build not only 8.u202 but later versions as well. And more! It seems like it can be build with ant at e09125febb2abd4d5eb70714ff68cdc76ee7dc45. In the meantime what we can do is make a separate channel for openjfx. I use openjfx for davmail only, which does provide 2fa for Microsoft Exchange. I have been experiencing some font issues since the beginning and would be happy to try the finalized build you made. Thanks!
Hi Ioannis,
So, I read only patches 2 and 3. In the third patch, I have a couple
suggestions, in addition to what I told you in my previous message.
java-openjfx-build-web-js-test.patch doesn't seem to be present in the
patch.
> 2. Output native libs to openjfx standard share/amd64 dir.
That would only work on x86_64-linux I think. Could you either install
to some common directory, and then fix loading, or to a directory that
depends on the architecture, so it can find the libraries on other
architectures too?
We could get rid of all the #t at the end of phases, it's deprecated
now.
Could you fix all this and send a new patch series, with one patch per
package, so we can try applying them and checking them more closely?
Le Tue, 21 Jun 2022 10:30:20 +0200, Alexey Abramov via Guix-patches via <guix-patches@gnu.org> a écrit : > Hi > > I am glad that you find these patches helpful. I tried to apply your > finalized patches, but seems like java-openjfx-build-web-js-test.patch > is still missing. > > Regarding the patches themselves, I agree with Ludo. They are indeed > hard to maintain. The best approach here would be to pack gradle, so > we could build not only 8.u202 but later versions as well. And more! > It seems like it can be build with ant at > e09125febb2abd4d5eb70714ff68cdc76ee7dc45. > > In the meantime what we can do is make a separate channel for openjfx. > I use openjfx for davmail only, which does provide 2fa for Microsoft > Exchange. I have been experiencing some font issues since the > beginning and would be happy to try the finalized build you made. > Thanks! > Although I agree that maintainability of these packages is going to be hard, I don't think it's so different from, say, maven :) Using Gradle would be the best solution here, but unfortunately, it depends at least on Kotlin for which there is currently no known bootstrap path. I've ended up building a 0.10.something version in a channel, but the next version I tried to build failed for unknown reasons (I think the previous version miscompiles the last version I could build). Even so, building from these old versions would require a lot of intermediate versions before we reach a recent Kotlin and it'll get worse everytime. I haven't found an alternative implementation we could use either. I don't see Gradle coming to Guix soon unfortunately. Using a separate channel wouldn't change the maintainability issue, and it would make openjfx much less discoverable, so I don't think it's a good solution. We should simply add more documentation and make sure we explain what's going on. It's difficult to follow, issues will be hard to fix, but I think it's pretty similar to Maven, where each upgrade brings new issues that are difficult to trace back to a cause and fix.
On Tue, Jun 21, 2022 at 6:27 PM Julien Lepiller <julien@lepiller.eu> wrote: > > Hi Ioannis, > > So, I read only patches 2 and 3. In the third patch, I have a couple > suggestions, in addition to what I told you in my previous message. > > java-openjfx-build-web-js-test.patch doesn't seem to be present in the > patch. My apologies, attached as 0004. (Alexey, it would be great if you can check if the patches solve the davmail issues you've mentioned) > > > 2. Output native libs to openjfx standard share/amd64 dir. > > That would only work on x86_64-linux I think. Could you either install > to some common directory, and then fix loading, or to a directory that > depends on the architecture, so it can find the libraries on other > architectures too? Sure will have a look. There are also some examples in java.scm that utilize %current-system and/or %current-target-system that I was thinking of using, but couldn't find any documentation to point me which one is the correct one. > We could get rid of all the #t at the end of phases, it's deprecated > now. > > Could you fix all this and send a new patch series, with one patch per > package, so we can try applying them and checking them more closely? Will do, and also try to address the comments from the previous email. Happy to get this through as many iterations as necessary. Thanks
Hi Julien, On Tue, Jun 21, 2022 at 6:27 PM Julien Lepiller <julien@lepiller.eu> wrote: > > Hi Ioannis, > > Could you fix all this and send a new patch series, with one patch per > package, so we can try applying them and checking them more closely? please find patch v6 series as attached. Changes from last patch are 1. Each package definition is in a separate commit/patch file - as requested. 1. Output all native libs to lib/openjfx - no arch specific directory, as requested. 2. Add native libs path to java.library.path property at static initialization, so that the NativeLibsLoader can locate them at runtime - This is a better working solution than loading by absolute path, since the latter was not working during testing if the native libs were not installed. 3. Install native libs after 'install phase - previously the native libs were installed unconditionally just after compilation, which is wrong. 4. Pass CMAKE_CXX_FLAGS argument as a single string - as requested. 5. Use new style inputs for packages - as requested. 6. Remove #t at the end of phases - as requested. Thanks