[PATCH] build-system/go: Don't let Go executables refer to the Go compiler.

  • Done
  • quality assurance status badge
Details
2 participants
  • Leo Famulari
  • Ludovic Courtès
Owner
unassigned
Submitted by
Leo Famulari
Severity
normal
L
L
Leo Famulari wrote on 14 Nov 2017 18:00
(address . guix-patches@gnu.org)
5669d4a6fba88e2dd1e048fd2ba292cf5a7c8d15.1510678556.git.leo@famulari.name
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(-)

Toggle diff (99 lines)
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
L
L
Leo Famulari wrote on 14 Nov 2017 19:57
(address . 29299@debbugs.gnu.org)
20171114185738.GA11015@jasmine.lan
On Tue, Nov 14, 2017 at 12:00:36PM -0500, Leo Famulari wrote:
Toggle quote (6 lines)
> 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.
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAloLPJ4ACgkQJkb6MLrK
fwisKBAAlAxx9tPJ80WeptONBr830DQw/UcVFt8yE8ejbYuS3gUGrAs3uEgsSsHh
kG40T1BCzDHFootj1h5Vnb61JsigFQCtqPjO/bVB2Bdc4GUUm5+QyHsXHJW4MNO6
d4k8qVJcldtpaOexs/j5oM9t4e3cd7lis6C9Wfy9Drh/7FLQJQ0P8hInzpLlb26O
7p5fat8qyT5CUyzt875XHmXSTtnAtUoPM6/DzOTr0uld2isSMCljJ7ndPwFk9lb+
lztGs0Xgy/IX+BZ3GVs4D5c9GGfI/Vr2mVase2EGzfnu9L1fKiJOf8Asj8m8pIvN
N5oIAb7PkJGkxIVwLycoRlJWC2iW5InMDXidi73vBSYPR6uWvi8U3TugWMmjlaob
atcDxMFQPiDBxal2+IhSaYwtP5yqk4wk6aZP1KMaPRDFvaYF1nQG8yvuKiJSoBSM
9E1hsMVjtsh7giunQLxNvcxXvf8aj2SiG+DPyn0prcR79YfXGishP66kImYommx4
9VNiUddwU2UI3IwvlQXCHyiuADtxjh0/rzrtY5cMcWWZudQxuNLYmkKQrfNh1ZFR
L+d+tKV1b9ZLjz2eYbMP6G7K0RsDnQuy/Y6v3rAsynMqdoiPl3iuAOIRQzS1ukDS
uAIohZFk4MZLyw0TnpRMtcKGdKCmPIMrVeSZlX+IA1knPGsELOY=
=1znh
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 14 Nov 2017 21:48
(address . 29299@debbugs.gnu.org)
20171114204817.GA9307@jasmine.lan
On Tue, Nov 14, 2017 at 01:57:38PM -0500, Leo Famulari wrote:
Toggle quote (10 lines)
> 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
:)
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAloLVo0ACgkQJkb6MLrK
fwjwPxAAqqKRHMkpC2xpx0oqdCAE61eZHsi4WXheiZF+sAGOxcVefkqF1qlAwzF2
3gw9vc0jWE8SZFOCrN7If+jb2DQjTXYKzUxtaWBHiqCejCQmPTxMtfPiG9HsJFTG
H1MiI9Jnl+m+Z+4ADlX30altx/4sbubuPx/0M4OxW36YlkKnJZou9bQlNYPeUZxS
OsbIdfqkYM90ZjQCEz4bSz04RxWHkZbsVYeG08Vl9QQPHLvxI+KJnkD6F1/n/yf/
dQYyhrgWAPbdhxpEDFilx1+b94owXtH84i55Ah3jThHrQ60mQzJDHnP44p60gPEK
EwuvQlOMWtc7v5N/CvzWL3ox13XcUIJ5toYTCxxOP/7jNQd2NcM31GeYoGBZwLhq
+kgk/1J+gStQUbUgHg+85YsLHGHSVqT6Wt1GaFXyzNxrPbaVatXkW9zkt9zSTcxD
uq/F4L8i0w0n884haCXsfu64Ci8d8pfM+2alyXl3kb4/qrxurs/+Pkyn7XaPUI9Z
zzoReYvagmM2/b6iuUuDNXEfyi3F7Z4ZYezDqjZmB9k6EIICNkc0Z6wVcUsDHh0O
5Ns/5SLQ6HyIUPBMf6r+2gWP1u4BICOZ1oP0hAsJElzigZmx6oPAa2UtKSHL5zpV
DdUlVYyFrI9vFQlZhJzC4tcfLuiqk3ckybRyIl0JOeDw1lJRifg=
=kc94
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 16 Nov 2017 11:50
Re: [bug#29299] [PATCH] build-system/go: Don't let Go executables refer to the Go compiler.
(name . Leo Famulari)(address . leo@famulari.name)(address . 29299@debbugs.gnu.org)
87wp2qfqfk.fsf@gnu.org
Heya,

Leo Famulari <leo@famulari.name> skribis:

Toggle quote (18 lines)
> 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’.
L
L
Ludovic Courtès wrote on 16 Nov 2017 11:52
(name . Leo Famulari)(address . leo@famulari.name)(address . 29299@debbugs.gnu.org)
87shdefqbc.fsf@gnu.org
Hello,

Leo Famulari <leo@famulari.name> skribis:

Toggle quote (11 lines)
> 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.

[...]

Toggle quote (5 lines)
> +(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.

Toggle quote (5 lines)
> +(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’.
L
L
Leo Famulari wrote on 18 Nov 2017 00:27
(no subject)
(address . control@debbugs.gnu.org)
20171117232700.GA25044@jasmine.lan
close 29299
?