GNU bug report logs - #29299
[PATCH] build-system/go: Don't let Go executables refer to the Go compiler.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29299 in the body.
You can then email your comments to 29299 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: guix-patches <at> gnu.org
Subject: [PATCH] build-system/go: Don't let Go executables refer to the Go
 compiler.
Date: Tue, 14 Nov 2017 12:00:36 -0500

This is a naive adaptation of ((guix build utils)
remove-store-references). 

It takes ~55 seconds to remove the references from Syncthing's
executables (96 MiB) on an SSD. Any ideas about how to speed it up?

* guix/build/go-build-system.scm (remove-store-reference, remove-go-references):
New variables.
(%standard-phases): Add 'remove-go-references' phase.
* guix/build/go.scm (go-build): Add allow-go-reference? key.
---
 guix/build-system/go.scm       |  2 ++
 guix/build/go-build-system.scm | 52 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/guix/build-system/go.scm b/guix/build-system/go.scm
index ec447d2a2..cf9116327 100644
--- a/guix/build-system/go.scm
+++ b/guix/build-system/go.scm
@@ -82,6 +82,7 @@
                    (import-path "")
                    (unpack-path "")
                    (tests? #t)
+                   (allow-go-reference? #f)
                    (system (%current-system))
                    (guile #f)
                    (imported-modules %go-build-system-modules)
@@ -107,6 +108,7 @@
                 #:import-path ,import-path
                 #:unpack-path ,unpack-path
                 #:tests? ,tests?
+                #:allow-go-reference? ,allow-go-reference?
                 #:inputs %build-inputs)))
 
   (define guile-for-build
diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm
index d175f3b76..05fc2d8ae 100644
--- a/guix/build/go-build-system.scm
+++ b/guix/build/go-build-system.scm
@@ -22,6 +22,8 @@
   #:use-module (guix build utils)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
+  #:use-module (rnrs io ports)
+  #:use-module (rnrs bytevectors)
   #:export (%standard-phases
             go-build))
 
@@ -204,6 +206,53 @@ on $GOBIN in the build phase."
     (copy-recursively "pkg" (string-append (assoc-ref outputs "out") "/pkg")))
   #t)
 
+(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."
+  (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."
+  (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 'configure)
@@ -213,7 +262,8 @@ on $GOBIN in the build phase."
     (add-before 'build 'setup-environment setup-environment)
     (replace 'build build)
     (replace 'check check)
-    (replace 'install install)))
+    (replace 'install install)
+    (add-after 'install 'remove-go-references remove-go-references)))
 
 (define* (go-build #:key inputs (phases %standard-phases)
                       #:allow-other-keys #:rest args)
-- 
2.15.0

Message #8 received at 29299 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: 29299 <at> debbugs.gnu.org
Subject: Re: [PATCH] build-system/go: Don't let Go executables refer to the
 Go compiler.
Date: Tue, 14 Nov 2017 13:57:38 -0500

[Message part 1 (text/plain, inline)]

On Tue, Nov 14, 2017 at 12:00:36PM -0500, Leo Famulari wrote:
> This is a naive adaptation of ((guix build utils)
> remove-store-references). 
> 
> It takes ~55 seconds to remove the references from Syncthing's
> executables (96 MiB) on an SSD. Any ideas about how to speed it up?

I checked, and remove-store-references (aka nuke-refs) takes the same
amount of time.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 29299 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: 29299 <at> debbugs.gnu.org
Subject: Re: [PATCH] build-system/go: Don't let Go executables refer to the
 Go compiler.
Date: Tue, 14 Nov 2017 15:48:17 -0500

[Message part 1 (text/plain, inline)]

On Tue, Nov 14, 2017 at 01:57:38PM -0500, Leo Famulari wrote:
> On Tue, Nov 14, 2017 at 12:00:36PM -0500, Leo Famulari wrote:
> > This is a naive adaptation of ((guix build utils)
> > remove-store-references). 
> > 
> > It takes ~55 seconds to remove the references from Syncthing's
> > executables (96 MiB) on an SSD. Any ideas about how to speed it up?
> 
> I checked, and remove-store-references (aka nuke-refs) takes the same
> amount of time.

So, fold-port-matches reads the port one character at a time, IIUC (I
don't really understand it).

I think we don't need to do that in this case. We could instead do
Boyer-Moore and skip 34 bytes + the length of %store-directory, which
could speed things up a lot.

But, I don't think we should wait for that in order to push this patch
:)

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 29299 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Leo Famulari <leo <at> famulari.name>
Cc: 29299 <at> debbugs.gnu.org
Subject: Re: [bug#29299] [PATCH] build-system/go: Don't let Go executables
 refer to the Go compiler.
Date: Thu, 16 Nov 2017 11:50:23 +0100

Heya,

Leo Famulari <leo <at> famulari.name> skribis:

> On Tue, Nov 14, 2017 at 01:57:38PM -0500, Leo Famulari wrote:
>> On Tue, Nov 14, 2017 at 12:00:36PM -0500, Leo Famulari wrote:
>> > This is a naive adaptation of ((guix build utils)
>> > remove-store-references). 
>> > 
>> > It takes ~55 seconds to remove the references from Syncthing's
>> > executables (96 MiB) on an SSD. Any ideas about how to speed it up?
>> 
>> I checked, and remove-store-references (aka nuke-refs) takes the same
>> amount of time.
>
> So, fold-port-matches reads the port one character at a time, IIUC (I
> don't really understand it).
>
> I think we don't need to do that in this case. We could instead do
> Boyer-Moore and skip 34 bytes + the length of %store-directory, which
> could speed things up a lot.

Indeed.  Mark’s code in (guix build grafts) does this, perhaps that
would be a better source of inspiration.

But yeah, no need to wait for that optimization.  :-)

Ludo’.

Message #17 received at 29299 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Leo Famulari <leo <at> famulari.name>
Cc: 29299 <at> debbugs.gnu.org
Subject: Re: [bug#29299] [PATCH] build-system/go: Don't let Go executables
 refer to the Go compiler.
Date: Thu, 16 Nov 2017 11:52:55 +0100

Hello,

Leo Famulari <leo <at> famulari.name> skribis:

> This is a naive adaptation of ((guix build utils)
> remove-store-references). 
>
> It takes ~55 seconds to remove the references from Syncthing's
> executables (96 MiB) on an SSD. Any ideas about how to speed it up?
>
> * guix/build/go-build-system.scm (remove-store-reference, remove-go-references):
> New variables.
> (%standard-phases): Add 'remove-go-references' phase.
> * guix/build/go.scm (go-build): Add allow-go-reference? key.

[...]

> +(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."

Maybe leave a note about the optimization opportunity.

> +(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."

… and here a comment as to why we’re doing this, possibly linking to
previous discussions.

Otherwise LGTM, thank you!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.