diff mbox series

[bug#73529,core] gnu: patchelf: Build from git sources.

Message ID 91e06adbec896b8e685d7bc2501da3e7dc932233.1727525522.git.attila@lendvai.name
State New
Headers show
Series [bug#73529,core] gnu: patchelf: Build from git sources. | expand

Commit Message

Attila Lendvai Sept. 28, 2024, 12:12 p.m. UTC 
From: Attila Lendvai <attila@lendvai.name>

Rationale: it makes it much easier to build a pending PR by using
`(commit "pull/544/head")` in the origin, or in general to experiment
with different patchelf versions.  It also makes it more resilient
against attacks hidden in release tarballs.

Change-Id: I94eaedcd826c32b2c90823fe026718213f8d5962
---

sadly, this leads to endless rebuilds...

 gnu/packages/elf.scm | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)


base-commit: 4f86fa20179ded1e6314eeba7da17309d501a32f

Comments

Z572 Sept. 29, 2024, 4:12 p.m. UTC | #1 
attila.lendvai@gmail.com writes:

> From: Attila Lendvai <attila@lendvai.name>
>
> Rationale: it makes it much easier to build a pending PR by using
> `(commit "pull/544/head")` in the origin, or in general to experiment
> with different patchelf versions.  It also makes it more resilient
> against attacks hidden in release tarballs.
>
> Change-Id: I94eaedcd826c32b2c90823fe026718213f8d5962
> ---
>
> sadly, this leads to endless rebuilds...
>
>  gnu/packages/elf.scm | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/gnu/packages/elf.scm b/gnu/packages/elf.scm
> index dc2db836321..21228a9d878 100644
> --- a/gnu/packages/elf.scm
> +++ b/gnu/packages/elf.scm
> @@ -278,19 +278,19 @@ (define-public patchelf
>      (name "patchelf")
>      (version "0.18.0")
>      (source (origin
> -             (method url-fetch)
> -             (uri (string-append
> -                   "https://github.com/NixOS/patchelf/releases/download/"
> -                   version
> -                   "/patchelf-" version ".tar.bz2"))
> -             (sha256
> -              (base32
> -               "02s7ap86rx6yagfh9xwp96sgsj0p6hp99vhiq9wn4mxshakv4lhr"))))
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/NixOS/patchelf")
> +                    (commit version)))
> +              (file-name (git-file-name "patchelf" version))

i think should "(git-file-name name version)" ?

> +              (sha256
> +               (base32
> +                "1v5mh8wdax4a97hw79nvzy0dixkbs0h487xkskkljkkdamq4rj5z"))))
>      (build-system gnu-build-system)
>      (arguments
>       '(#:phases
>         (modify-phases %standard-phases
> -         (add-after 'unpack 'fix-tests
> +         (add-after 'bootstrap 'fix-tests

Just a question, why do you need to change to bootstrap here?

>             ;; Our GCC code ensures that RUNPATH is never empty, it includes
>             ;; at least glibc/lib and gcc:lib/lib.
>             (lambda* (#:key inputs #:allow-other-keys)
> @@ -298,8 +298,8 @@ (define-public patchelf
>                 ;; Disable checking for an empty runpath:
>                 (("^if test.*") "")
>                 ;; Find libgcc_s.so, which is necessary for the test:
> -               (("/xxxxxxxxxxxxxxx") (string-append (assoc-ref inputs "gcc:lib")
> -                                                    "/lib")))
> +               (("/xxxxxxxxxxxxxxx")
> +                (dirname (search-input-file inputs "/lib/libgcc_s.so"))))
>               (substitute* "tests/replace-needed.sh"
>                 ;; This test assumes that only libc will be linked alongside
>                 ;; libfoo, but we also link libgcc_s.
> @@ -312,7 +312,10 @@ (define-public patchelf
>               (substitute* "tests/Makefile.in"
>                 ((".*shared-rpath\\.sh \\.*") "")))))))
>      (native-inputs
> -     `(("gcc:lib" ,gcc "lib")))
> +     (list
> +      autoconf
> +      automake
> +      (list gcc "lib")))

It would be nice to mention removing labels and adding new dependencies
in the submission, and etc/committer.scm maybe useful.

>      (home-page "https://nixos.org/patchelf.html")
>      (synopsis "Modify the dynamic linker and RPATH of ELF executables")
>      (description
>
> base-commit: 4f86fa20179ded1e6314eeba7da17309d501a32f
Attila Lendvai Sept. 30, 2024, 8:14 a.m. UTC | #2 
> > + (file-name (git-file-name "patchelf" version))
> 
> 
> i think should "(git-file-name name version)" ?


hrm. i tried that at first, but it errored out with `name` being undefined. i retried it just now to double check, and now it works. i must have been confused by something.

the updated patch will have `name` in there.


> > + (sha256
> > + (base32
> > + "1v5mh8wdax4a97hw79nvzy0dixkbs0h487xkskkljkkdamq4rj5z"))))
> > (build-system gnu-build-system)
> > (arguments
> > '(#:phases
> > (modify-phases %standard-phases
> > - (add-after 'unpack 'fix-tests
> > + (add-after 'bootstrap 'fix-tests
> 
> 
> Just a question, why do you need to change to bootstrap here?


because the last substitute* touches `tests/Makefile.in` which is generated by the bootstrap phase (by the auto* tools).

but now that you asked, i've changed it to target `tests/Makefile.am` instead.


> > (native-inputs
> > - `(("gcc:lib" ,gcc "lib")))
> > + (list
> > + autoconf
> > + automake
> > + (list gcc "lib")))
> 
> 
> It would be nice to mention removing labels and adding new dependencies
> in the submission, and etc/committer.scm maybe useful.


ok, i'll send an updated patch soon.

BTW, etc/committer.scm broke on this package. i've sent a patch to fix that:

https://issues.guix.gnu.org/73562
diff mbox series

Patch

diff --git a/gnu/packages/elf.scm b/gnu/packages/elf.scm
index dc2db836321..21228a9d878 100644
--- a/gnu/packages/elf.scm
+++ b/gnu/packages/elf.scm
@@ -278,19 +278,19 @@  (define-public patchelf
     (name "patchelf")
     (version "0.18.0")
     (source (origin
-             (method url-fetch)
-             (uri (string-append
-                   "https://github.com/NixOS/patchelf/releases/download/"
-                   version
-                   "/patchelf-" version ".tar.bz2"))
-             (sha256
-              (base32
-               "02s7ap86rx6yagfh9xwp96sgsj0p6hp99vhiq9wn4mxshakv4lhr"))))
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/NixOS/patchelf")
+                    (commit version)))
+              (file-name (git-file-name "patchelf" version))
+              (sha256
+               (base32
+                "1v5mh8wdax4a97hw79nvzy0dixkbs0h487xkskkljkkdamq4rj5z"))))
     (build-system gnu-build-system)
     (arguments
      '(#:phases
        (modify-phases %standard-phases
-         (add-after 'unpack 'fix-tests
+         (add-after 'bootstrap 'fix-tests
            ;; Our GCC code ensures that RUNPATH is never empty, it includes
            ;; at least glibc/lib and gcc:lib/lib.
            (lambda* (#:key inputs #:allow-other-keys)
@@ -298,8 +298,8 @@  (define-public patchelf
                ;; Disable checking for an empty runpath:
                (("^if test.*") "")
                ;; Find libgcc_s.so, which is necessary for the test:
-               (("/xxxxxxxxxxxxxxx") (string-append (assoc-ref inputs "gcc:lib")
-                                                    "/lib")))
+               (("/xxxxxxxxxxxxxxx")
+                (dirname (search-input-file inputs "/lib/libgcc_s.so"))))
              (substitute* "tests/replace-needed.sh"
                ;; This test assumes that only libc will be linked alongside
                ;; libfoo, but we also link libgcc_s.
@@ -312,7 +312,10 @@  (define-public patchelf
              (substitute* "tests/Makefile.in"
                ((".*shared-rpath\\.sh \\.*") "")))))))
     (native-inputs
-     `(("gcc:lib" ,gcc "lib")))
+     (list
+      autoconf
+      automake
+      (list gcc "lib")))
     (home-page "https://nixos.org/patchelf.html")
     (synopsis "Modify the dynamic linker and RPATH of ELF executables")
     (description