| Message ID | 20220614094954.15197-9-ngraves@ngraves.fr |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#55958,1/9] gnu: Add node-buffer-crc32. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
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!
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))