[bug#39742,1/7] gnu: java-openjfx-build: Add helpful patch.
diff mbox series

Message ID 20200222201755.50425-1-levenson@mmer.org
State Accepted
Headers show
Series
  • [bug#39742,1/7] gnu: java-openjfx-build: Add helpful patch.
Related show

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job

Commit Message

Alexey Abramov Feb. 22, 2020, 8:17 p.m. UTC
* 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

Comments

Alexey Abramov March 3, 2020, 5:33 p.m. UTC | #1
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])
Leo Famulari March 4, 2020, 2:56 a.m. UTC | #2
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
Björn Höfling March 5, 2020, 7:09 p.m. UTC | #3
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])
Alexey Abramov March 5, 2020, 8:10 p.m. UTC | #4
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])
Alexey Abramov March 5, 2020, 8:42 p.m. UTC | #5
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
Björn Höfling March 5, 2020, 9:35 p.m. UTC | #6
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

Patch
diff mbox series

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])