GNU bug report logs - #55966
[PATCH 9/9] gnu: chromium extensions lighter make-crx.

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 55966 in the body.
You can then email your comments to 55966 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: Nicolas Graves <ngraves <at> ngraves.fr>
To: guix-patches <at> gnu.org
Cc: Nicolas Graves <ngraves <at> ngraves.fr>
Subject: [PATCH 9/9] gnu: chromium extensions lighter make-crx.
Date: Tue, 14 Jun 2022 11:49:54 +0200

---
 gnu/build/chromium-extension.scm | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/gnu/build/chromium-extension.scm b/gnu/build/chromium-extension.scm
index 8ca5251957..8d52153751 100644
--- a/gnu/build/chromium-extension.scm
+++ b/gnu/build/chromium-extension.scm
@@ -19,10 +19,9 @@
 (define-module (gnu build chromium-extension)
   #:use-module (guix gexp)
   #:use-module (guix packages)
-  #:use-module (gnu packages chromium)
   #:use-module (gnu packages gnupg)
   #:use-module (gnu packages tls)
-  #:use-module (gnu packages xorg)
+  #:use-module (gnu packages node-xyz)
   #:use-module (guix build-system trivial)
   #:export (make-chromium-extension))
 
@@ -69,24 +68,14 @@ (define version (package-version package))
    (string-append name "-" version ".crx")
    (with-imported-modules '((guix build utils))
      #~(begin
-         ;; This is not great.  We pull Xorg and Chromium just to Zip and
-         ;; sign an extension.  This should be implemented with something
-         ;; lighter.  (TODO: where is the CRXv3 documentation..?)
          (use-modules (guix build utils))
-         (let ((chromium #$(file-append ungoogled-chromium "/bin/chromium"))
-               (xvfb #$(file-append xorg-server "/bin/Xvfb"))
+         (let ((crx3 #$(file-append node-crx3 "/bin/crx3"))
                (packdir (string-append (getcwd) "/extension")))
            (mkdir packdir)
            (copy-recursively (ungexp package package-output) packdir
                              ;; Ensure consistent file modification times.
                              #:keep-mtime? #t)
-           (system (string-append xvfb " :1 &"))
-           (setenv "DISPLAY" ":1")
-           (sleep 2)                    ;give Xorg some time to initialize...
-           (invoke chromium
-                   "--user-data-dir=chromium-profile"
-                   (string-append "--pack-extension=" packdir)
-                   (string-append "--pack-extension-key=" #$signing-key))
+           (invoke crx3 "--keyPath" #$signing-key packdir)
            (copy-file (string-append packdir ".crx") #$output))))
    #:local-build? #t))
 
-- 
2.36.1

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

From: Marius Bakke <marius <at> gnu.org>
To: Nicolas Graves via Guix-patches via <guix-patches <at> gnu.org>,
 55966 <at> debbugs.gnu.org
Cc: Nicolas Graves <ngraves <at> ngraves.fr>
Subject: Re: [bug#55966] [PATCH 9/9] gnu: chromium extensions lighter make-crx.
Date: Thu, 23 Jun 2022 23:20:57 +0200

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

Nicolas Graves via Guix-patches via <guix-patches <at> gnu.org> skriver:

> ---
>  gnu/build/chromium-extension.scm | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)

This commit lacks a message describing the changed variable.  The commit
title could also be more descriptive.  ;-)

> diff --git a/gnu/build/chromium-extension.scm b/gnu/build/chromium-extension.scm
> index 8ca5251957..8d52153751 100644
> --- a/gnu/build/chromium-extension.scm
> +++ b/gnu/build/chromium-extension.scm
> @@ -19,10 +19,9 @@
>  (define-module (gnu build chromium-extension)
>    #:use-module (guix gexp)
>    #:use-module (guix packages)
> -  #:use-module (gnu packages chromium)
>    #:use-module (gnu packages gnupg)
>    #:use-module (gnu packages tls)
> -  #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages node-xyz)
>    #:use-module (guix build-system trivial)
>    #:export (make-chromium-extension))
>  
> @@ -69,24 +68,14 @@ (define version (package-version package))
>     (string-append name "-" version ".crx")
>     (with-imported-modules '((guix build utils))
>       #~(begin
> -         ;; This is not great.  We pull Xorg and Chromium just to Zip and
> -         ;; sign an extension.  This should be implemented with something
> -         ;; lighter.  (TODO: where is the CRXv3 documentation..?)

Wohoo.  :-)

>           (use-modules (guix build utils))
> -         (let ((chromium #$(file-append ungoogled-chromium "/bin/chromium"))
> -               (xvfb #$(file-append xorg-server "/bin/Xvfb"))
> +         (let ((crx3 #$(file-append node-crx3 "/bin/crx3"))
>                 (packdir (string-append (getcwd) "/extension")))
>             (mkdir packdir)
>             (copy-recursively (ungexp package package-output) packdir
>                               ;; Ensure consistent file modification times.
>                               #:keep-mtime? #t)
> -           (system (string-append xvfb " :1 &"))
> -           (setenv "DISPLAY" ":1")
> -           (sleep 2)                    ;give Xorg some time to initialize...
> -           (invoke chromium
> -                   "--user-data-dir=chromium-profile"
> -                   (string-append "--pack-extension=" packdir)
> -                   (string-append "--pack-extension-key=" #$signing-key))
> +           (invoke crx3 "--keyPath" #$signing-key packdir)

LGTM!  Feel free to add your copyright here too.

Can you send an updated series?

Thanks!

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

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

From: Maxime Devos <maximedevos <at> telenet.be>
To: Nicolas Graves <ngraves <at> ngraves.fr>, 55966 <at> debbugs.gnu.org
Subject: Re: [bug#55966] [PATCH 9/9] gnu: chromium extensions lighter make-crx.
Date: Fri, 24 Jun 2022 00:28:16 +0200

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

Nicolas Graves via Guix-patches via schreef op di 14-06-2022 om 11:49
[+0200]:
> -         (let ((chromium #$(file-append ungoogled-chromium "/bin/chromium"))
> -               (xvfb #$(file-append xorg-server "/bin/Xvfb"))
> +         (let ((crx3 #$(file-append node-crx3 "/bin/crx3"))
>                 (packdir (string-append (getcwd) "/extension")))

You're invoking crx3 below, so it shouldn't be cross-compiled even if
the extension is cross-compiled, hence the #$(file-append node-crx3
...) should probably be #+(file-append node-crx ...) instead.

(Also was an issue in the old code that used chromium and xorg-server
..., and probably the chromium build system doesn't implement cross-
compilation yet ...)

Greetings,
Maxime.

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

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

From: Nicolas Graves <ngraves <at> ngraves.fr>
To: 55966 <at> debbugs.gnu.org
Cc: ngraves <at> ngraves.fr
Subject: [PATCH] gnu: modifying make-chromium-extension to rely on node-crx3.
Date: Wed, 20 Jul 2022 12:39:34 +0200

* gnu/build/chromium-extension.scm (make-crx): Lift Xorg and Chromium
dependencies, rely on node-crx3 instead.
---
 gnu/build/chromium-extension.scm | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/gnu/build/chromium-extension.scm b/gnu/build/chromium-extension.scm
index 8ca5251957..28449a1e1d 100644
--- a/gnu/build/chromium-extension.scm
+++ b/gnu/build/chromium-extension.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020, 2021 Marius Bakke <marius <at> gnu.org>
+;;; Copyright © 2022 Nicolas Graves <ngraves <at> ngraves.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,10 +20,9 @@
 (define-module (gnu build chromium-extension)
   #:use-module (guix gexp)
   #:use-module (guix packages)
-  #:use-module (gnu packages chromium)
   #:use-module (gnu packages gnupg)
   #:use-module (gnu packages tls)
-  #:use-module (gnu packages xorg)
+  #:use-module (gnu packages node-xyz)
   #:use-module (guix build-system trivial)
   #:export (make-chromium-extension))
 
@@ -69,24 +69,14 @@ (define version (package-version package))
    (string-append name "-" version ".crx")
    (with-imported-modules '((guix build utils))
      #~(begin
-         ;; This is not great.  We pull Xorg and Chromium just to Zip and
-         ;; sign an extension.  This should be implemented with something
-         ;; lighter.  (TODO: where is the CRXv3 documentation..?)
          (use-modules (guix build utils))
-         (let ((chromium #$(file-append ungoogled-chromium "/bin/chromium"))
-               (xvfb #$(file-append xorg-server "/bin/Xvfb"))
+         (let ((crx3 #+(file-append node-crx3 "/bin/crx3"))
                (packdir (string-append (getcwd) "/extension")))
            (mkdir packdir)
            (copy-recursively (ungexp package package-output) packdir
                              ;; Ensure consistent file modification times.
                              #:keep-mtime? #t)
-           (system (string-append xvfb " :1 &"))
-           (setenv "DISPLAY" ":1")
-           (sleep 2)                    ;give Xorg some time to initialize...
-           (invoke chromium
-                   "--user-data-dir=chromium-profile"
-                   (string-append "--pack-extension=" packdir)
-                   (string-append "--pack-extension-key=" #$signing-key))
+           (invoke crx3 "--keyPath" #$signing-key packdir)
            (copy-file (string-append packdir ".crx") #$output))))
    #:local-build? #t))
 
-- 
2.37.0

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

From: Nicolas Graves <ngraves <at> ngraves.fr>
To: 55966 <at> debbugs.gnu.org
Cc: ngraves <at> ngraves.fr, marius <at> gnu.org
Subject: Updated series
Date: Wed, 20 Jul 2022 12:46:33 +0200

Hi Marius,

Sorry for the time it took me, here's the updated series.

Everything has been checked with guix lint. I use emacs, but couldn't get guix
style to work the way you counselled, hope it's ok.

Also thanks for your advice! 

-- 
Best regards,
Nicolas Graves

Message #27 received at 55966-done <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <marius <at> gnu.org>
To: Nicolas Graves <ngraves <at> ngraves.fr>, 55966-done <at> debbugs.gnu.org
Cc: ngraves <at> ngraves.fr
Subject: Re: Updated series
Date: Wed, 20 Jul 2022 17:16:48 +0200

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

Nicolas Graves <ngraves <at> ngraves.fr> skriver:

> Hi Marius,
>
> Sorry for the time it took me, here's the updated series.

No worries, thanks a lot for this work.

> Everything has been checked with guix lint. I use emacs, but couldn't get guix
> style to work the way you counselled, hope it's ok.

What was the issue?  :-)

I ran 'guix style' for each since I had to edit the commits anyway to
get the author right (for some reason it showed up as "Nicolas Graves
via Guix-patches <guix-patches <at> gnu.org>" -- NYF!).

> Also thanks for your advice! 

:-)

Some more advice for future pull requests, please first send a message
to 'guix-patches <at> gnu.org' to get a bug ID assigned (can be anything,
although often a 'git format-patch --cover-letter').  Then send the
patch series to NNNNN <at> debbugs.gnu.org, otherwise the patches will
be scattered across different issues and difficult to track.

Also, use "-n" with send-email/format-patch so that the ordering is
preserved.  It was lacking in the second series, but I used the
information from the first round to get it right.

Anyway, great work, pushed as c8f33b613e..cda3de3b7d!

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

Previous Next

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