diff mbox series

[bug#39146,v2] gnu: icecat: Remove about:buildconfig store references.

Message ID 20200122201313.27946-1-me@tobias.gr
State Accepted
Headers show
Series [bug#39146,v2] gnu: icecat: Remove about:buildconfig store references. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job

Commit Message

ashish.is--- via Guix-patches" via Jan. 22, 2020, 8:13 p.m. UTC 
* gnu/packages/gnuzilla.scm (icecat)[arguments]:
New ‘neutralise-store-references’ phase.

Reported-by: Jakub Kądziołka <kuba@kadziolka.net>
---

So,

Here's a version that correctly calls %store-directory.  Tested again, output looks the same.

Jakub: I accidentally kept your copyright line in v1, but didn't actually use your code (although part of your comment survives :-).  Is the Reported-by above acceptable?

Kind regards,

T G-R

 gnu/packages/gnuzilla.scm | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Marius Bakke Jan. 22, 2020, 8:28 p.m. UTC | #1 
Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org>
writes:

> * gnu/packages/gnuzilla.scm (icecat)[arguments]:
> New ‘neutralise-store-references’ phase.

[...]

> +         (add-after 'build 'neutralise-store-references
> +           (lambda _
> +             ;; Mangle the store references to compilers & other build tools in
> +             ;; about:buildconfig, reducing IceCat's closure by 1 GiB on x86-64.
> +             (substitute*
> +                 "dist/bin/chrome/toolkit/content/global/buildconfig.html"
> +               (((format #f "(~a/)([0-9a-z]{32})" (%store-directory)) _ store hash)
> +                (string-append store (string-take hash 8) "…")))
> +             #t))

Is it possible to use 'remove-store-references' here?
Maja Kądziołka Jan. 22, 2020, 8:50 p.m. UTC | #2 
On Wed, Jan 22, 2020 at 09:13:13PM +0100, Tobias Geerinckx-Rice wrote:
> Jakub: I accidentally kept your copyright line in v1, but didn't actually use your code (although part of your comment survives :-).  Is the Reported-by above acceptable?
I feel like Co-authored-by might fit better, but either is fine by me.

> +             ;; Mangle the store references to compilers & other build tools in
> +             ;; about:buildconfig, reducing IceCat's closure by 1 GiB on x86-64.
Minor² nit: I feel like this wording suggests this only matters on x64.
How about: reducing IceCat's closure by 1 GiB (measured on x86-64).

> +             (substitute*
> +                 "dist/bin/chrome/toolkit/content/global/buildconfig.html"
> +               (((format #f "(~a/)([0-9a-z]{32})" (%store-directory)) _ store hash)
> +                (string-append store (string-take hash 8) "…")))
> +             #t))
I don't know whether this is a supported use case, but... what if
(%store-directory) contains regex metacharacters?

Marius Bakke wrote:
> Is it possible to use 'remove-store-references' here?
For later reference, resolved on IRC:
http://logs.guix.gnu.org/guix/2020-01-22.log#213448

Regards,
Jakub Kądziołka
diff mbox series

Patch

diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
index ae0c58eedb..96c3c78e98 100644
--- a/gnu/packages/gnuzilla.scm
+++ b/gnu/packages/gnuzilla.scm
@@ -7,7 +7,7 @@ 
 ;;; Copyright © 2016 Alex Griffin <a@ajgrf.com>
 ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2017 ng0 <ng0@n0.is>
-;;; Copyright © 2017, 2018 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2017, 2018, 2020 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2019 Ivan Petkov <ivanppetkov@gmail.com>
 ;;; Copyright © 2020 Oleg Pykhalov <go.wigust@gmail.com>
@@ -1067,6 +1067,15 @@  from forcing GEXP-PROMISE."
                                 (force-output)
                                 (retry (- remaining-attempts 1))))
                        (apply build args)))))))
+         (add-after 'build 'neutralise-store-references
+           (lambda _
+             ;; Mangle the store references to compilers & other build tools in
+             ;; about:buildconfig, reducing IceCat's closure by 1 GiB on x86-64.
+             (substitute*
+                 "dist/bin/chrome/toolkit/content/global/buildconfig.html"
+               (((format #f "(~a/)([0-9a-z]{32})" (%store-directory)) _ store hash)
+                (string-append store (string-take hash 8) "…")))
+             #t))
          (add-before 'configure 'install-desktop-entry
            (lambda* (#:key outputs #:allow-other-keys)
              ;; Install the '.desktop' file.