| Message ID | 20200222201755.50425-1-levenson@mmer.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#39742,1/7] gnu: java-openjfx-build: Add helpful patch. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
Hi I packaged openjfx and would like to get some review. Can someone take a look? Alexey Abramov <levenson@mmer.org> writes: > * gnu/packages/java.scm: Add patch > * gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch: New > file. Allows you to run gradlew to run properly. Useful for debugging. > --- > gnu/packages/java.scm | 3 ++- > .../java-openjfx-build-get_guix_jdk_version.patch | 14 ++++++++++++++ > 2 files changed, 16 insertions(+), 1 deletion(-) > create mode 100644 gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch > > diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm > index 9371901e1f..01541ee419 100644 > --- a/gnu/packages/java.scm > +++ b/gnu/packages/java.scm > @@ -2355,7 +2355,8 @@ new Date();")) > (file-name (string-append name "-" version "-checkout")) > (sha256 > (base32 > - "0yg38mwpivswccv9n96k06x3iv82i4px1a9xg9l8dswzwmfj259f")))) > + "0yg38mwpivswccv9n96k06x3iv82i4px1a9xg9l8dswzwmfj259f")) > + (patches (search-patches "java-openjfx-build-get_guix_jdk_version.patch")))) > (build-system ant-build-system) > (arguments > `(#:jar-name "java-openjfx.jar" > diff --git a/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch > new file mode 100644 > index 0000000000..214ef9949c > --- /dev/null > +++ b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch > @@ -0,0 +1,14 @@ > +--- a/build.gradle > ++++ b/build.gradle > +@@ -742,9 +742,9 @@ > + if (inStream.readLine() != null) { > + String v = inStream.readLine(); > + if (v != null) { > +- int ib = v.indexOf(" (build "); > ++ int ib = v.indexOf(" (guix build "); > + if (ib != -1) { > +- String ver = v.substring(ib + 8, v.size() - 1); > ++ String ver = v.substring(ib + 13, v.size() - 1); > + > + defineProperty("jdkRuntimeVersion", ver) > + defineProperty("jdkVersion", jdkRuntimeVersion.split("-")[0])
Hi, Thanks for these patches and sorry for the delay in review! There are a lot of patches and it takes a while to get through them. On Sat, Feb 22, 2020 at 09:17:49PM +0100, Alexey Abramov wrote: > * gnu/packages/java.scm: Add patch > * gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch: New > file. Allows you to run gradlew to run properly. Useful for debugging. The commit message needs a couple changes, to something like this: ------ * gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch: New file. * gnu/local.mk (dist_patch_DATA): Add it. * gnu/packages/java.scm (java-openjfx-build)[source]: Use it. ------ We don't need to describe the patch in the commit message. Instead, we explain it in a comment at the beginning of the patch file itself. This is easier for the people that will read the patch later. The comment should explain what the patch does and where it came from, including links to bug reports and external patch sources, if they exist. We need to register the new patch file in 'gnu/local.mk', so that it gets distributed in the Guix release process. And finally, we need to name the package variable (java-openjfx-build) that the patch is being used by. Here is an example from an older commit: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=ca5e404f9a1ff81a38a32578c9c3a6c866482a9a
Hi Alexey, On Sat, 22 Feb 2020 21:17:49 +0100 Alexey Abramov <levenson@mmer.org> wrote: > * gnu/packages/java.scm: Add patch > * gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch: > New file. Allows you to run gradlew to run properly. Useful for > debugging. --- This sentence sounds suspicious :-) In Guix, we build everything from source. If we need a compiler, library, or build-tool, we build that also from source. Here, the problem is that: gradle/wrapper/gradle-wrapper.jar is a binary JAR-file included in the sources, which is used by the 'gradlew' script. This jar-file should not be part of the original sources used to to compile this package. Whenever sources contain unwanted binaries, we use 'snippets' to remove them before doing any further steps. You will find them in many places in the java.scm file, just search for 'snippet', for example in the package java-cisd-jhdf5. For using gradle, we need to compile gradle from sources, but this is very hard, there are some bootstrapping problems involed in this. So, first we should first snipp away the jar-files from the original java-openjfx-build package by Julien. And second, I hope you don't rely on the gradle build in any way here :-) Björn > gnu/packages/java.scm | 3 ++- > .../java-openjfx-build-get_guix_jdk_version.patch | 14 > ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) > create mode 100644 > gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch > > diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm > index 9371901e1f..01541ee419 100644 > --- a/gnu/packages/java.scm > +++ b/gnu/packages/java.scm > @@ -2355,7 +2355,8 @@ new Date();")) > (file-name (string-append name "-" version > "-checkout")) (sha256 > (base32 > - > "0yg38mwpivswccv9n96k06x3iv82i4px1a9xg9l8dswzwmfj259f")))) > + > "0yg38mwpivswccv9n96k06x3iv82i4px1a9xg9l8dswzwmfj259f")) > + (patches (search-patches > "java-openjfx-build-get_guix_jdk_version.patch")))) (build-system > ant-build-system) (arguments > `(#:jar-name "java-openjfx.jar" > diff --git > a/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch > b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch > new file mode 100644 index 0000000000..214ef9949c --- /dev/null > +++ > b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch > @@ -0,0 +1,14 @@ +--- a/build.gradle > ++++ b/build.gradle > +@@ -742,9 +742,9 @@ > + if (inStream.readLine() != null) { > + String v = inStream.readLine(); > + if (v != null) { > +- int ib = v.indexOf(" (build "); > ++ int ib = v.indexOf(" (guix build "); > + if (ib != -1) { > +- String ver = v.substring(ib + 8, v.size() - 1); > ++ String ver = v.substring(ib + 13, v.size() - 1); > + > + defineProperty("jdkRuntimeVersion", ver) > + defineProperty("jdkVersion", > jdkRuntimeVersion.split("-")[0])
Hi, I have been trying to compile it so many times, so I patched the file for easy debugging ) I traced some confusing parts of a build.gradle using gradlew ... --verbose to translate it in Scheme. Alexey On March 5, 2020 19:38:40 Björn Höfling <bjoern.hoefling@bjoernhoefling.de> wrote: > Hi Alexey, > > On Sat, 22 Feb 2020 21:17:49 +0100 > Alexey Abramov <levenson@mmer.org> wrote: > >> * gnu/packages/java.scm: Add patch >> * gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch: >> New file. Allows you to run gradlew to run properly. Useful for >> debugging. --- > > This sentence sounds suspicious :-) > > In Guix, we build everything from source. If we need a compiler, > library, or build-tool, we build that also from source. > > Here, the problem is that: > > gradle/wrapper/gradle-wrapper.jar > > is a binary JAR-file included in the sources, which is used by the > 'gradlew' script. This jar-file should not be part of the original > sources used to to compile this package. > > Whenever sources contain unwanted binaries, we use 'snippets' to remove > them before doing any further steps. You will find them in many places > in the java.scm file, just search for 'snippet', for example in the > package java-cisd-jhdf5. > > For using gradle, we need to compile gradle from sources, but this is > very hard, there are some bootstrapping problems involed in this. > > So, first we should first snipp away the jar-files from the original > java-openjfx-build package by Julien. > > And second, I hope you don't rely on the gradle build in any way > here :-) > > Björn > >> gnu/packages/java.scm | 3 ++- >> .../java-openjfx-build-get_guix_jdk_version.patch | 14 >> ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) >> create mode 100644 >> gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch >> >> diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm >> index 9371901e1f..01541ee419 100644 >> --- a/gnu/packages/java.scm >> +++ b/gnu/packages/java.scm >> @@ -2355,7 +2355,8 @@ new Date();")) >> (file-name (string-append name "-" version >> "-checkout")) (sha256 >> (base32 >> - >> "0yg38mwpivswccv9n96k06x3iv82i4px1a9xg9l8dswzwmfj259f")))) >> + >> "0yg38mwpivswccv9n96k06x3iv82i4px1a9xg9l8dswzwmfj259f")) >> + (patches (search-patches >> "java-openjfx-build-get_guix_jdk_version.patch")))) (build-system >> ant-build-system) (arguments >> `(#:jar-name "java-openjfx.jar" >> diff --git >> a/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch >> b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch >> new file mode 100644 index 0000000000..214ef9949c --- /dev/null >> +++ >> b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch >> @@ -0,0 +1,14 @@ +--- a/build.gradle >> ++++ b/build.gradle >> +@@ -742,9 +742,9 @@ >> + if (inStream.readLine() != null) { >> + String v = inStream.readLine(); >> + if (v != null) { >> +- int ib = v.indexOf(" (build "); >> ++ int ib = v.indexOf(" (guix build "); >> + if (ib != -1) { >> +- String ver = v.substring(ib + 8, v.size() - 1); >> ++ String ver = v.substring(ib + 13, v.size() - 1); >> + >> + defineProperty("jdkRuntimeVersion", ver) >> + defineProperty("jdkVersion", >> jdkRuntimeVersion.split("-")[0])
Many thanks! I will fix these in the next patches. Alexey On March 4, 2020 03:25:36 Leo Famulari <leo@famulari.name> wrote: > Hi, > > Thanks for these patches and sorry for the delay in review! There are a > lot of patches and it takes a while to get through them. > > On Sat, Feb 22, 2020 at 09:17:49PM +0100, Alexey Abramov wrote: >> * gnu/packages/java.scm: Add patch >> * gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch: New >> file. Allows you to run gradlew to run properly. Useful for debugging. > > The commit message needs a couple changes, to something like this: > > ------ > * gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch: New file. > * gnu/local.mk (dist_patch_DATA): Add it. > * gnu/packages/java.scm (java-openjfx-build)[source]: Use it. > ------ > > We don't need to describe the patch in the commit message. Instead, we > explain it in a comment at the beginning of the patch file itself. This > is easier for the people that will read the patch later. The comment > should explain what the patch does and where it came from, including > links to bug reports and external patch sources, if they exist. > > We need to register the new patch file in 'gnu/local.mk', so that it > gets distributed in the Guix release process. > > And finally, we need to name the package variable (java-openjfx-build) > that the patch is being used by. > > Here is an example from an older commit: > > https://git.savannah.gnu.org/cgit/guix.git/commit/?id=ca5e404f9a1ff81a38a32578c9c3a6c866482a9a
On Thu, 5 Mar 2020 20:09:25 +0100 Björn Höfling <bjoern.hoefling@bjoernhoefling.de> wrote: > Here, the problem is that: > > gradle/wrapper/gradle-wrapper.jar [..] > So, first we should first snipp away the jar-files from the original > java-openjfx-build package by Julien. I fixed that in the java-openjfx-build package under commit: 2c2b1ef85448681d858f447ac6fed6679a95209f Björn
diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm index 9371901e1f..01541ee419 100644 --- a/gnu/packages/java.scm +++ b/gnu/packages/java.scm @@ -2355,7 +2355,8 @@ new Date();")) (file-name (string-append name "-" version "-checkout")) (sha256 (base32 - "0yg38mwpivswccv9n96k06x3iv82i4px1a9xg9l8dswzwmfj259f")))) + "0yg38mwpivswccv9n96k06x3iv82i4px1a9xg9l8dswzwmfj259f")) + (patches (search-patches "java-openjfx-build-get_guix_jdk_version.patch")))) (build-system ant-build-system) (arguments `(#:jar-name "java-openjfx.jar" diff --git a/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch new file mode 100644 index 0000000000..214ef9949c --- /dev/null +++ b/gnu/packages/patches/java-openjfx-build-get_guix_jdk_version.patch @@ -0,0 +1,14 @@ +--- a/build.gradle ++++ b/build.gradle +@@ -742,9 +742,9 @@ + if (inStream.readLine() != null) { + String v = inStream.readLine(); + if (v != null) { +- int ib = v.indexOf(" (build "); ++ int ib = v.indexOf(" (guix build "); + if (ib != -1) { +- String ver = v.substring(ib + 8, v.size() - 1); ++ String ver = v.substring(ib + 13, v.size() - 1); + + defineProperty("jdkRuntimeVersion", ver) + defineProperty("jdkVersion", jdkRuntimeVersion.split("-")[0])