| Message ID | 29d55f693128126558e2e3e0a030194453312187.1663570649.git.kiasoc5@disroot.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#57927] gnu: source-highlight: Fix lesspipe file name and use gexps. | 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 |
> * gnu/packages/pretty-print.scm (source-highlight): > [arguments]: Use gexps, remove trailing #ts. > [phases]: Add phase to make src-highlight-lesspipe.sh work. Applied, thanks! Mathieu
Hi kiasoc5, kiasoc5 via Guix-patches via 写道： > This fixes src-hilite-lesspipe.sh so that lesspipe.sh is called > instead of lesspipe. Thanks! I reverted this change on master. It caused over 4000 rebuilds (per architecture), which means it's core-updates material. You can test this yourself by running ‘guix refresh -l source-highlight’. It's not perfect: it can fail to detect some paths, especially when inheritance is involved. But it's a good sanity check. When you notice a rebuild count higher than the thresholds given here[0], please add a ‘[PATCH core-updates]’ or ‘[PATCH staging]’ warning to your patch subject. It reduces the chance of these slipping through. > * gnu/packages/pretty-print.scm (source-highlight): > [arguments]: Use gexps, remove trailing #ts. > [phases]: Add phase to make src-highlight-lesspipe.sh work. These unrelated changes should be separated into at least two patches next time: one to gexp and remove the #ts, the other to make the functional change. ‘At least’, because Gexpification often leaves the output hash unchanged, so a separate gexp patch might have been able to go straight to master. > + (arguments This introduced a whitespace error: there are extra trailing spaces. Git should highlight these when showing the diff. I removed them. Kind regards, T G-R [0]: https://guix.gnu.org/manual/en/guix.html#Submitting-Patches
Hi Tobias, On Tue, Sep 27 2022, 03:22:19 PM +0200 Tobias Geerinckx-Rice <me@tobias.gr> wrote: > Hi kiasoc5, > > kiasoc5 via Guix-patches via 写道： > > This fixes src-hilite-lesspipe.sh so that lesspipe.sh is called > > instead of lesspipe. > > When you notice a rebuild count higher than the thresholds given > here[0], please add a ‘[PATCH core-updates]’ or ‘[PATCH staging]’ > warning to your patch subject. It reduces the chance of these > slipping through. I didn't realize source-highlight was used by so many projects, I'll be more careful next time. Just so I understand the rebuilds are occuring because the store path of source-highlight changed, and the result could not be grafted? > > * gnu/packages/pretty-print.scm (source-highlight): > > [arguments]: Use gexps, remove trailing #ts. > > [phases]: Add phase to make src-highlight-lesspipe.sh work. > > These unrelated changes should be separated into at least two > patches next time: one to gexp and remove the #ts, the other to > make the functional change. > < to go > straight to master. 1. Does trailing #t change the hash? 2. Why might Gexpification change the hash?
On 28-09-2022 06:34, kiasoc5 via Guix-patches via wrote: >>> * gnu/packages/pretty-print.scm (source-highlight): >>> [arguments]: Use gexps, remove trailing #ts. >>> [phases]: Add phase to make src-highlight-lesspipe.sh work. >> These unrelated changes should be separated into at least two >> patches next time: one to gexp and remove the #ts, the other to >> make the functional change. >> < to go >> straight to master. > 1. Does trailing #t change the hash? Yes. > 2. Why might Gexpification change the hash? When the resulting staged code changes. This could happen if you were to do, say, #$(this-package-input "boost") instead of (assoc-ref %build-inputs "boost") for example, though AFAICT you didn't make any G-exp-related changes that would change the hash. Greetings, Maxime.
On 28-09-2022 06:34, kiasoc5 via Guix-patches via wrote: > Just so I understand the rebuilds are occuring because the store path > of source-highlight changed, and the result could not be grafted? On the second question: adding a graft is not done automatically, you need to ask for it by _keeping_ the original package and adding the modified package to 'replacement'. Greetings, Maxime.
diff --git a/gnu/packages/pretty-print.scm b/gnu/packages/pretty-print.scm index 9745a9ba10..13108fe7db 100644 --- a/gnu/packages/pretty-print.scm +++ b/gnu/packages/pretty-print.scm @@ -32,6 +32,7 @@ (define-module (gnu packages pretty-print) #:use-module (guix download) #:use-module (guix build-system cmake) #:use-module (guix build-system gnu) + #:use-module (guix gexp) #:use-module (guix utils) #:use-module (gnu packages) #:use-module (gnu packages bison) @@ -276,40 +277,41 @@ (define-public source-highlight (list boost)) (native-inputs (list bison flex)) - (arguments - `(#:configure-flags - (list (string-append "--with-boost=" - (assoc-ref %build-inputs "boost"))) - #:parallel-tests? #f ;There appear to be race conditions - #:phases - (modify-phases %standard-phases - ,@(if (%current-target-system) - ;; 'doc/Makefile.am' tries to run stuff even when - ;; cross-compiling. Explicitly skip it. - ;; XXX: Inline this on next rebuild cycle. - `((add-before 'build 'skip-doc-directory - (lambda _ - (substitute* "Makefile" - (("^SUBDIRS = (.*) doc(.*)$" _ before after) - (string-append "SUBDIRS = " before - " " after "\n"))) - #t))) - '()) - (add-before 'check 'patch-test-files - (lambda _ - ;; Unpatch shebangs in test input so that source-highlight - ;; is still able to infer input language - (substitute* '("tests/test.sh" - "tests/test2.sh" - "tests/test.tcl") - (((string-append "#! *" (which "sh"))) "#!/bin/sh")) - ;; Initial patching unrecoverably removes whitespace, so - ;; remove it also in the comparison output. - (substitute* '("tests/test.sh.html" - "tests/test2.sh.html" - "tests/test.tcl.html") - (("#! */bin/sh") "#!/bin/sh")) - #t))))) + (arguments + (list #:configure-flags + #~(list (string-append "--with-boost=" (assoc-ref %build-inputs "boost"))) + #:parallel-tests? #f ;There appear to be race conditions + #:phases + #~(modify-phases %standard-phases + (add-before 'build 'rename-lesspipe-to-lesspipe.sh.in + (lambda _ + (substitute* "src/src-hilite-lesspipe.sh.in" + (("lesspipe") "lesspipe.sh")))) + #$@(if (%current-target-system) + ;; 'doc/Makefile.am' tries to run stuff even when + ;; cross-compiling. Explicitly skip it. + ;; XXX: Inline this on next rebuild cycle. + #~((add-before 'build 'skip-doc-directory + (lambda _ + (substitute* "Makefile" + (("^SUBDIRS = (.*) doc(.*)$" _ before after) + (string-append "SUBDIRS = " before + " " after "\n")))))) + '()) + (add-before 'check 'patch-test-files + (lambda _ + ;; Unpatch shebangs in test input so that source-highlight + ;; is still able to infer input language + (substitute* '("tests/test.sh" + "tests/test2.sh" + "tests/test.tcl") + (((string-append "#! *" (which "sh"))) "#!/bin/sh")) + ;; Initial patching unrecoverably removes whitespace, so + ;; remove it also in the comparison output. + (substitute* '("tests/test.sh.html" + "tests/test2.sh.html" + "tests/test.tcl.html") + (("#! */bin/sh") "#!/bin/sh"))))))) (home-page "https://www.gnu.org/software/src-highlite/") (synopsis "Produce a document with syntax highlighting from a source file") (description