[bug#57927] [PATCH] gnu: source-highlight: Fix lesspipe file name and use gexps.

[bug#57927] [PATCH] gnu: source-highlight: Fix lesspipe file name and use gexps.

[kiasoc5 via Guix-patches via]

This fixes src-hilite-lesspipe.sh so that lesspipe.sh is called instead of lesspipe.

* gnu/packages/pretty-print.scm (source-highlight):
[arguments]: Use gexps, remove trailing #ts.
[phases]: Add phase to make src-highlight-lesspipe.sh work.
---
 gnu/packages/pretty-print.scm | 70 ++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 34 deletions(-)

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

base-commit: 25adb336bcb0188a92ecbe6b9c1d9d3e3a8b59e4
-- 
2.37.2

bug#57927: [PATCH] gnu: source-highlight: Fix lesspipe file name and use gexps.

[Mathieu Othacehe]

> * 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

[bug#57927] [PATCH] gnu: source-highlight: Fix lesspipe file name and use gexps.

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#57927] [PATCH] gnu: source-highlight: Fix lesspipe file name and use gexps.

[kiasoc5 via Guix-patches via]

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?

[bug#57927] [PATCH] gnu: source-highlight: Fix lesspipe file name and use gexps.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 821 bytes --]



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.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#57927] [PATCH] gnu: source-highlight: Fix lesspipe file name and use gexps.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 404 bytes --]

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.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]