[bug#55966,9/9] gnu: chromium extensions lighter make-crx.
Commit Message
---
gnu/build/chromium-extension.scm | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
Comments
Nicolas Graves via Guix-patches via <guix-patches@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!
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.
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!
Nicolas Graves <ngraves@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@gnu.org>" -- NYF!).
> 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!
@@ -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))