diff mbox series

[bug#50227] build-system/go: Trim store references using the native compiler option.

Message ID 20210827164423.17109-1-marius@gnu.org
State New
Headers show
Series [bug#50227] build-system/go: Trim store references using the native compiler option. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Marius Bakke Aug. 27, 2021, 4:44 p.m. UTC
* guix/build/go-build-system.scm (build): Add '-trimpath' to the 'go install'
invocation.
(remove-store-references, remove-go-references): Remove procedures.
(%standard-phases): Don't include remove-go-references.
* gnu/packages/docker.scm (docker)[arguments]: Add the '-trimpath' option to
the build flags.  Remove phase remove-go-references.
* gnu/packages/uucp.scm (nncp)[arguments]: Likewise.
---
 gnu/packages/docker.scm        |  7 ++--
 gnu/packages/uucp.scm          |  8 ++--
 guix/build/go-build-system.scm | 70 +++-------------------------------
 3 files changed, 14 insertions(+), 71 deletions(-)

Comments

Marius Bakke Aug. 27, 2021, 5:47 p.m. UTC | #1
Marius Bakke <marius@gnu.org> skriver:

> * guix/build/go-build-system.scm (build): Add '-trimpath' to the 'go install'
> invocation.
> (remove-store-references, remove-go-references): Remove procedures.
> (%standard-phases): Don't include remove-go-references.
> * gnu/packages/docker.scm (docker)[arguments]: Add the '-trimpath' option to
> the build flags.  Remove phase remove-go-references.
> * gnu/packages/uucp.scm (nncp)[arguments]: Likewise.

Some context for this patch, which I forgot to save before sending:

I happened to rediscover <https://issues.guix.gnu.org/33620> and pulled
out my yak shaving device again.

This patch removes the custom 'remove-store-references' procedure in
favor of the native '-trimpath' option to 'go install', as alluded to in
the bug report as well as the build system commentary.  I don't spot any
difference in sizes from 'master' apart from one package: Docker.

Docker explodes from 764.4 MiB to 1215.5 MiB with this patch even though
it does use the '-trimpath' option.  Perhaps -trimpath does not work as
well with dynamically linked executables as it does for static?

I'm willing to overlook this regression for now in favor of the reduced
complexity, but comments welcome.
diff mbox series

Patch

diff --git a/gnu/packages/docker.scm b/gnu/packages/docker.scm
index 8bac1b89ce..108f355aa7 100644
--- a/gnu/packages/docker.scm
+++ b/gnu/packages/docker.scm
@@ -535,6 +535,8 @@  built-in registry server of Docker.")
              ;; Respectively, strip the symbol table and debug
              ;; information, and the DWARF symbol table.
              (setenv "LDFLAGS" "-s -w")
+             ;; Trim store references from the compiled binary.
+             (setenv "BUILDFLAGS" "-trimpath")
              ;; Make build faster
              (setenv "GOCACHE" "/tmp")
              #t))
@@ -568,10 +570,7 @@  built-in registry server of Docker.")
                (install-file "bundles/dynbinary-daemon/dockerd" out-bin)
                (install-file (string-append "bundles/dynbinary-daemon/dockerd-"
                                             (getenv "VERSION"))
-                             out-bin)
-               #t)))
-         (add-after 'install 'remove-go-references
-           (assoc-ref go:%standard-phases 'remove-go-references)))))
+                             out-bin)))))))
     (inputs
      `(("btrfs-progs" ,btrfs-progs)
        ("containerd" ,containerd)       ; for containerd-shim
diff --git a/gnu/packages/uucp.scm b/gnu/packages/uucp.scm
index 120417dea1..9d39c88fe5 100644
--- a/gnu/packages/uucp.scm
+++ b/gnu/packages/uucp.scm
@@ -127,6 +127,10 @@  between computers.")
              (substitute* (list "bin/default.do" "bin/hjson-cli.do" "test.do")
                ((" -mod=vendor") "")
                ((" -m") ""))
+             (substitute* (list "bin/default.do" "bin/hjson-cli.do")
+               ;; Prevent reference to the Go inputs in the compiled binaries.
+               (("\\$GO build")
+                "$GO build -trimpath"))
              ;; Use the correct module path. `go list` does not report the
              ;; correct module path since we have moved the source files.
              (substitute* "bin/default.do"
@@ -138,9 +142,7 @@  between computers.")
          (replace 'check
            (lambda* (#:key tests? #:allow-other-keys)
              (when tests?
-               (invoke "contrib/do" "-c" "test"))))
-         (add-after 'install 'remove-go-references
-           (assoc-ref go:%standard-phases 'remove-go-references)))))
+               (invoke "contrib/do" "-c" "test")))))))
     (inputs
      `(("go-github-com-davecgh-go-xdr" ,go-github-com-davecgh-go-xdr)
        ("go-github-com-dustin-go-humanize" ,go-github-com-dustin-go-humanize)
diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm
index 37936fe5ca..fc5ee39c8d 100644
--- a/guix/build/go-build-system.scm
+++ b/guix/build/go-build-system.scm
@@ -28,8 +28,6 @@ 
   #:use-module (ice-9 match)
   #:use-module (ice-9 ftw)
   #:use-module (srfi srfi-1)
-  #:use-module (rnrs io ports)
-  #:use-module (rnrs bytevectors)
   #:export (%standard-phases
             go-build))
 
@@ -47,12 +45,9 @@ 
 ;; structure called a 'workspace' [1].  This workspace can be found by Go via
 ;; the GOPATH environment variable.  Typically, all Go source code and compiled
 ;; objects are kept in a single workspace, but GOPATH may be a list of
-;; directories [2].  In this go-build-system we create a file system union of
-;; the Go-language dependencies. Previously, we made GOPATH a list of store
-;; directories, but stopped because Go programs started keeping references to
-;; these directories in Go 1.11:
-;; <https://bugs.gnu.org/33620>.
-;;
+;; directories [2], which we rely on here, with the caveat that the current
+;; package must appear first on GOPATH.
+;
 ;; Go software, whether a package or a command, is uniquely named using an
 ;; 'import path'.  The import path is based on the URL of the software's source.
 ;; Because most source code is provided over the internet, the import path is
@@ -88,7 +83,6 @@ 
 ;; a tmpdir when creating the inputs union.
 ;; * Use Go modules [4]
 ;; * Re-use compiled packages [5]
-;; * 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?
@@ -194,6 +188,8 @@  unpacking."
       (apply invoke "go" "install"
               "-v" ; print the name of packages as they are compiled
               "-x" ; print each command as it is invoked
+              ;; Trim store references from the compiled binaries.
+              "-trimpath"
               ;; Respectively, strip the symbol table and debug
               ;; information, and the DWARF symbol table.
               "-ldflags=-s -w"
@@ -236,59 +232,6 @@  the standard install-license-files phase to first enter the correct directory."
                                                     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
     (delete 'bootstrap)
@@ -299,8 +242,7 @@  files in OUTPUTS."
     (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)