[PATCH] guix: go-build-system: use trimpath go flag

  • Done
  • quality assurance status badge
Details
2 participants
  • Ekaitz Zarraga
  • Sharlatan Hellseher
Owner
unassigned
Submitted by
Ekaitz Zarraga
Severity
normal
E
E
Ekaitz Zarraga wrote on 13 Jan 12:46 +0100
(address . guix-patches@gnu.org)
bad254afb05576bb8f95038da01df6f34bb46009.1705146375.git.ekaitz@elenq.tech
Go 1.13 introduced[1] a new "trimpath" flag in charge of removing the
references to the go toolchain in the produced ELFs. We used to remove
these references using the "remove-go-reference" function. This function
was executed after go install. By using this new trimpath flag, we don't
have to remove any store path from the ELFs produced by go install.

We're not using any go older than 1.13 anymore, it's safe to remove
these functions.


-trimpath
remove all file system paths from the resulting executable.
Instead of absolute file system paths, the recorded file names
will begin either a module path@version (when using modules),
or a plain import path (when using the standard library, or
GOPATH).

* guix/build/go-build-system.scm (build): Add -trimpath
(%standard-phases): Remove remove-go-references.
(remove-go-references): Remove.

Change-Id: Idcae366d226da5ce095693f81fd33133fd1d70d6
Co-authored-by: Picnoir <picnoir@alternativebit.fr>
---
guix/build/go-build-system.scm | 59 +++-------------------------------
1 file changed, 4 insertions(+), 55 deletions(-)

Toggle diff (102 lines)
diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm
index 7f25e05d0d..70ddcd07cd 100644
--- a/guix/build/go-build-system.scm
+++ b/guix/build/go-build-system.scm
@@ -6,6 +6,8 @@
;;; Copyright © 2020 Jakub K?dzio?ka <kuba@kadziolka.net>
;;; Copyright © 2020, 2021 Efraim Flashner <efraim@flashner.co.il>
;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
+;;; Copyright © 2024 Ekaitz Zarraga <ekaitz@elenq.tech>
+;;; Copyright © 2024 Picnoir <picnoir@alternativebit.fr>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -90,7 +92,6 @@ (define-module (guix build go-build-system)
;; * Use Go modules [4]
;; * Re-use compiled packages [5]
;; * Avoid the go-inputs hack
-;; * Stop needing remove-go-references (-trimpath ? )
;; * Remove module packages, only offering the full Git repos? This is
;; more idiomatic, I think, because Go downloads Git repos, not modules.
;; What are the trade-offs?
@@ -265,6 +266,7 @@ (define* (build #:key import-path build-flags #:allow-other-keys)
;; Respectively, strip the symbol table and debug
;; information, and the DWARF symbol table.
"-ldflags=-s -w"
+ "-trimpath"
`(,@build-flags ,import-path)))
(lambda (key . args)
(display (string-append "Building '" import-path "' failed.\n"
@@ -304,58 +306,6 @@ (define* (install-license-files #:key unpack-path
unpack-path))
(apply (assoc-ref gnu:%standard-phases 'install-license-files) args)))
-(define* (remove-store-reference file file-name
- #:optional (store (%store-directory)))
- "Remove from FILE occurrences of FILE-NAME in STORE; return #t when FILE-NAME
-is encountered in FILE, #f otherwise. This implementation reads FILE one byte at
-a time, which is slow. Instead, we should use the Boyer-Moore string search
-algorithm; there is an example in (guix build grafts)."
- (define pattern
- (string-take file-name
- (+ 34 (string-length (%store-directory)))))
-
- (with-fluids ((%default-port-encoding #f))
- (with-atomic-file-replacement file
- (lambda (in out)
- ;; We cannot use `regexp-exec' here because it cannot deal with
- ;; strings containing NUL characters.
- (format #t "removing references to `~a' from `~a'...~%" file-name file)
- (setvbuf in 'block 65536)
- (setvbuf out 'block 65536)
- (fold-port-matches (lambda (match result)
- (put-bytevector out (string->utf8 store))
- (put-u8 out (char->integer #\/))
- (put-bytevector out
- (string->utf8
- "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-"))
- #t)
- #f
- pattern
- in
- (lambda (char result)
- (put-u8 out (char->integer char))
- result))))))
-
-(define* (remove-go-references #:key allow-go-reference?
- inputs outputs #:allow-other-keys)
- "Remove any references to the Go compiler from the compiled Go executable
-files in OUTPUTS."
-;; We remove this spurious reference to save bandwidth when installing Go
-;; executables. It would be better to not embed the reference in the first
-;; place, but I'm not sure how to do that. The subject was discussed at:
-;; <https://lists.gnu.org/archive/html/guix-devel/2017-10/msg00207.html>
- (if allow-go-reference?
- #t
- (let ((go (assoc-ref inputs "go"))
- (bin "/bin"))
- (for-each (lambda (output)
- (when (file-exists? (string-append (cdr output)
- bin))
- (for-each (lambda (file)
- (remove-store-reference file go))
- (find-files (string-append (cdr output) bin)))))
- outputs)
- #t)))
(define %standard-phases
(modify-phases gnu:%standard-phases
@@ -367,8 +317,7 @@ (define %standard-phases
(replace 'build build)
(replace 'check check)
(replace 'install install)
- (replace 'install-license-files install-license-files)
- (add-after 'install 'remove-go-references remove-go-references)))
+ (replace 'install-license-files install-license-files)))
(define* (go-build #:key inputs (phases %standard-phases)
#:allow-other-keys #:rest args)

base-commit: c0b303aaa3d6154acbe054120d11467eb98e6d33
--
2.41.0
S
S
Sharlatan Hellseher wrote on 13 Jan 13:58 +0100
(name . Ekaitz Zarraga)(address . ekaitz@elenq.tech)
CAO+9K5om43zrhjjwK0cVs5wsNY9g0rXe3HsJGhE-aGL0xto0ug@mail.gmail.com
Hi Ekaitz,

Thanks for the patch.

I think this is more suitable for core updates brunch as triggers rebuild
the world for all golang packages as far as I see.

Regards,
Oleg

On Sat, 13 Jan 2024, 11:48 Ekaitz Zarraga, <ekaitz@elenq.tech> wrote:

Toggle quote (143 lines)
> Go 1.13 introduced[1] a new "trimpath" flag in charge of removing the
> references to the go toolchain in the produced ELFs. We used to remove
> these references using the "remove-go-reference" function. This function
> was executed after go install. By using this new trimpath flag, we don't
> have to remove any store path from the ELFs produced by go install.
>
> We're not using any go older than 1.13 anymore, it's safe to remove
> these functions.
>
> [1] https://go.dev/doc/go1.13
>
> -trimpath
> remove all file system paths from the resulting executable.
> Instead of absolute file system paths, the recorded file names
> will begin either a module path@version (when using modules),
> or a plain import path (when using the standard library, or
> GOPATH).
>
> * guix/build/go-build-system.scm (build): Add -trimpath
> (%standard-phases): Remove remove-go-references.
> (remove-go-references): Remove.
>
> Change-Id: Idcae366d226da5ce095693f81fd33133fd1d70d6
> Co-authored-by: Picnoir <picnoir@alternativebit.fr>
> ---
> guix/build/go-build-system.scm | 59 +++-------------------------------
> 1 file changed, 4 insertions(+), 55 deletions(-)
>
> diff --git a/guix/build/go-build-system.scm
> b/guix/build/go-build-system.scm
> index 7f25e05d0d..70ddcd07cd 100644
> --- a/guix/build/go-build-system.scm
> +++ b/guix/build/go-build-system.scm
> @@ -6,6 +6,8 @@
> ;;; Copyright © 2020 Jakub K?dzio?ka <kuba@kadziolka.net>
> ;;; Copyright © 2020, 2021 Efraim Flashner <efraim@flashner.co.il>
> ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
> +;;; Copyright © 2024 Ekaitz Zarraga <ekaitz@elenq.tech>
> +;;; Copyright © 2024 Picnoir <picnoir@alternativebit.fr>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -90,7 +92,6 @@ (define-module (guix build go-build-system)
> ;; * Use Go modules [4]
> ;; * Re-use compiled packages [5]
> ;; * Avoid the go-inputs hack
> -;; * Stop needing remove-go-references (-trimpath ? )
> ;; * Remove module packages, only offering the full Git repos? This is
> ;; more idiomatic, I think, because Go downloads Git repos, not modules.
> ;; What are the trade-offs?
> @@ -265,6 +266,7 @@ (define* (build #:key import-path build-flags
> #:allow-other-keys)
> ;; Respectively, strip the symbol table and debug
> ;; information, and the DWARF symbol table.
> "-ldflags=-s -w"
> + "-trimpath"
> `(,@build-flags ,import-path)))
> (lambda (key . args)
> (display (string-append "Building '" import-path "' failed.\n"
> @@ -304,58 +306,6 @@ (define* (install-license-files #:key unpack-path
> unpack-path))
> (apply (assoc-ref gnu:%standard-phases 'install-license-files) args)))
>
> -(define* (remove-store-reference file file-name
> - #:optional (store (%store-directory)))
> - "Remove from FILE occurrences of FILE-NAME in STORE; return #t when
> FILE-NAME
> -is encountered in FILE, #f otherwise. This implementation reads FILE one
> byte at
> -a time, which is slow. Instead, we should use the Boyer-Moore string
> search
> -algorithm; there is an example in (guix build grafts)."
> - (define pattern
> - (string-take file-name
> - (+ 34 (string-length (%store-directory)))))
> -
> - (with-fluids ((%default-port-encoding #f))
> - (with-atomic-file-replacement file
> - (lambda (in out)
> - ;; We cannot use `regexp-exec' here because it cannot deal with
> - ;; strings containing NUL characters.
> - (format #t "removing references to `~a' from `~a'...~%" file-name
> file)
> - (setvbuf in 'block 65536)
> - (setvbuf out 'block 65536)
> - (fold-port-matches (lambda (match result)
> - (put-bytevector out (string->utf8 store))
> - (put-u8 out (char->integer #\/))
> - (put-bytevector out
> - (string->utf8
> -
> "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-"))
> - #t)
> - #f
> - pattern
> - in
> - (lambda (char result)
> - (put-u8 out (char->integer char))
> - result))))))
> -
> -(define* (remove-go-references #:key allow-go-reference?
> - inputs outputs #:allow-other-keys)
> - "Remove any references to the Go compiler from the compiled Go
> executable
> -files in OUTPUTS."
> -;; We remove this spurious reference to save bandwidth when installing Go
> -;; executables. It would be better to not embed the reference in the first
> -;; place, but I'm not sure how to do that. The subject was discussed at:
> -;; <https://lists.gnu.org/archive/html/guix-devel/2017-10/msg00207.html>
> - (if allow-go-reference?
> - #t
> - (let ((go (assoc-ref inputs "go"))
> - (bin "/bin"))
> - (for-each (lambda (output)
> - (when (file-exists? (string-append (cdr output)
> - bin))
> - (for-each (lambda (file)
> - (remove-store-reference file go))
> - (find-files (string-append (cdr output)
> bin)))))
> - outputs)
> - #t)))
>
> (define %standard-phases
> (modify-phases gnu:%standard-phases
> @@ -367,8 +317,7 @@ (define %standard-phases
> (replace 'build build)
> (replace 'check check)
> (replace 'install install)
> - (replace 'install-license-files install-license-files)
> - (add-after 'install 'remove-go-references remove-go-references)))
> + (replace 'install-license-files install-license-files)))
>
> (define* (go-build #:key inputs (phases %standard-phases)
> #:allow-other-keys #:rest args)
>
> base-commit: c0b303aaa3d6154acbe054120d11467eb98e6d33
> --
> 2.41.0
>
>
>
>
Attachment: file
E
E
Ekaitz Zarraga wrote on 13 Jan 14:31 +0100
(name . Sharlatan Hellseher)(address . sharlatanus@gmail.com)
5171475c-288e-24c2-a91c-94ee9b7627af@elenq.tech
Hi,

On 2024-01-13 13:58, Sharlatan Hellseher wrote:
Toggle quote (7 lines)
> Hi Ekaitz,
>
> Thanks for the patch.
>
> I think this is more suitable for core updates brunch as triggers
> rebuild the world for all golang packages as far as I see.

Yeah, probably.

We also thought after the patch that would be interesting to provide a
way to disable this behavior. Or maybe it's ok?

With the previous alternative the packager would just remove the phase,
but hardcoding the option as we did doesn't provide any way to do
that... Some kind of flag should be enough.

Are you ok with the patch, thought?

If this is ok we'll make those small changes and send a v2.

Thanks
S
S
Sharlatan Hellseher wrote on 23 Jan 21:34 +0100
(name . Ekaitz Zarraga)(address . ekaitz@elenq.tech)
CAO+9K5roW33yFyxbHn6qYigNmzHbTeXrtwWzT4P0VBn9BVb8Mg@mail.gmail.com
Hi Ekaitz,

Toggle quote (4 lines)
> With the previous alternative the packager would just remove
> the phase, but hardcoding the option as we did doesn't provide
> any way to do that... Some kind of flag should be enough.

It would be great to have more control over golang build envars
and build flags, like it happen in gnu-build-system or cmake-build-system
via #:make-flags and #:configure-flags.

Having some common defaults hardcoded and kind of #:build-flags as a way to
path anything else to compiler.

There is a similar thread referring go test flags, I've listed all in here:

Thanks,
Oleg
Attachment: file
S
S
Sharlatan Hellseher wrote on 7 Mar 13:21 +0100
[PATCH] guix: go-build-system: use trimpath go flag
(address . 68423-done@debbugs.gnu.org)
87h6hi44i5.fsf@gmail.com
Pushed as 998588599aa46e384b664ec576ab124b83fc16bd to go-team.

--
Oleg
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEmEeB3micIcJkGAhndtcnv/Ys0rUFAmXpsTIACgkQdtcnv/Ys
0rUhpw/+O7E0MoY0m/HN5CLovWECCwm3DIBb0ps51B+aoVt4avWeh9ZGM9UtUHO2
m2/W31DaQTXWu8NmpWtyQRgbGnfn5gLF/uX1XRIZ8JTxEsmt1rA7iuky6XC4mvDq
1jaJXytYQtQS+5QWFJeAV7pefEPwy8X5MkvCwGhXY1qkSyHW8r0nhxJ1XhhOAHoP
qd8pbhZ5osXf/hiZZJFEDrFFYM4vAYHCYnd8PWFg54TWRaSCHaONZ7XOXJfdR9nW
PaCUXo/a0ANcoSdiFDADlOnHGIY5AONjmJ4eWsE3ZfyhOMiTJEfoMl7CCmloTt06
OSWqo+y5AuzTdpzERPjqFT5nPLgEsTHb0RR5h1+YAloDPoY7PbbCWuCQ6J6j0lD7
Ctt2wXGBjFEsEH/vDBUlMowwcrEKuyla/QLHiClsblF0bxF2+ImizZgK4XhvRP+0
H0z12fetRoPqOi7TG+fsQ1ZtJvpmiqJXgKGM0AxF/Ulws7YHCfvspsQOkASfaRxl
d8oIa2fbOoewO+ee7lsojrCv6vkL50zhQ8tpqAQAn9qd/MhePKjU3UD6qfgiIShY
My5GIehhkPrX8IrI6xM7LwkVl52qlbyljLDL2yVQqcwLk4ZhLCoeck0T9v27GHO3
Za96Gx3kA5bkvyREFeACNGDw0AWm3u2xa3jFHm4WfyC9I6S24Zc=
=9UXM
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 68423@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 68423
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch