[PATCH 9/9] gnu: chromium extensions lighter make-crx.

DoneSubmitted by Nicolas Graves.
Details
4 participants
  • Liliana Marie Prikler
  • Marius Bakke
  • Maxime Devos
  • Nicolas Graves
Owner
unassigned
Severity
normal
Merged with
N
N
Nicolas Graves wrote on 14 Jun 11:49 +0200
(address . guix-patches@gnu.org)(name . Nicolas Graves)(address . ngraves@ngraves.fr)
20220614094954.15197-9-ngraves@ngraves.fr
---
gnu/build/chromium-extension.scm | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)

Toggle diff (45 lines)
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
L
L
Liliana Marie Prikler wrote on 15 Jun 11:21 +0200
(address . control@debbugs.gnu.org)
c511c7cd5bad5cf933cfee73d92d8253d8b3c9ef.camel@ist.tugraz.at
merge 55958 55959 55960 55961 55962 55963 55964 55965 55966
thanks
M
M
Marius Bakke wrote on 23 Jun 23:20 +0200
(name . Nicolas Graves)(address . ngraves@ngraves.fr)
87r13fqdom.fsf@gnu.org
Nicolas Graves via Guix-patches via <guix-patches@gnu.org> skriver:

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

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

Toggle quote (18 lines)
> (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!
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYrTZOQ8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHcmwQD8CMK+pxlSWKSUz/ZS4n6M37C/yTwXqJn4qeGC
hws+52wBAIw/6zFLo4RX6PFqyK9JZQaT/A5uCqEB4F62aO+AFJYB
=NJqd
-----END PGP SIGNATURE-----

M
M
Maxime Devos wrote on 24 Jun 00:28 +0200
e06e02fc7980f2d67048ec9fda1ab191618d42f4.camel@telenet.be
Nicolas Graves via Guix-patches via schreef op di 14-06-2022 om 11:49
[+0200]:
Toggle quote (5 lines)
> -         (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.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYrTpABccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7shaAQDKfsSTPYKqGefXZWBP7/lTOdnN
xdneYKiEfwFrdeLmEAEA3g1gmk8AmQgQQl8rj5hJnKG7UxqzArFus+JZpEjQ/Q4=
=elet
-----END PGP SIGNATURE-----


N
N
Nicolas Graves wrote on 20 Jul 12:39 +0200
[PATCH] gnu: modifying make-chromium-extension to rely on node-crx3.
(address . 55966@debbugs.gnu.org)(address . ngraves@ngraves.fr)
20220720103934.17768-1-ngraves@ngraves.fr
* 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(-)

Toggle diff (52 lines)
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@gnu.org>
+;;; Copyright © 2022 Nicolas Graves <ngraves@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
N
N
Nicolas Graves wrote on 20 Jul 12:46 +0200
Updated series
(address . 55966@debbugs.gnu.org)
875yjsawom.fsf@ngraves.fr
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
M
M
Marius Bakke wrote on 20 Jul 17:16 +0200
(address . ngraves@ngraves.fr)
87zgh3n7a7.fsf@gnu.org
Nicolas Graves <ngraves@ngraves.fr> skriver:

Toggle quote (4 lines)
> Hi Marius,
>
> Sorry for the time it took me, here's the updated series.

No worries, thanks a lot for this work.

Toggle quote (3 lines)
> 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@gnu.org>" -- NYF!).

Toggle quote (2 lines)
> Also thanks for your advice!

:-)

Some more advice for future pull requests, please first send a message
to 'guix-patches@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@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!
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYtgcYA8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHfkhQEApnu+gvniJodoVSEGCvOCqohHxhsuWyKo2eD9
OdQtYuQA/jzNb4PjbOq1gn1JJlq6yFQziUr9i3Vy2W7CIKloCBcG
=YG2K
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

To comment on this conversation send email to 55966@debbugs.gnu.org