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

  • Done
  • quality assurance status badge
Details
4 participants
  • kiasoc5
  • Maxime Devos
  • Tobias Geerinckx-Rice
  • Mathieu Othacehe
Owner
unassigned
Submitted by
kiasoc5
Severity
normal
K
K
kiasoc5 wrote on 19 Sep 2022 08:57
(address . guix-patches@gnu.org)(name . kiasoc5)(address . kiasoc5@disroot.org)
29d55f693128126558e2e3e0a030194453312187.1663570649.git.kiasoc5@disroot.org
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(-)

Toggle diff (92 lines)
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
M
M
Mathieu Othacehe wrote on 27 Sep 2022 11:11
(name . kiasoc5)(address . kiasoc5@disroot.org)(address . 57927-done@debbugs.gnu.org)
87k05p89m9.fsf@gnu.org
Toggle quote (4 lines)
> * 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
Closed
T
T
Tobias Geerinckx-Rice wrote on 27 Sep 2022 15:22
Re: [bug#57927] [PATCH] gnu: source-highlight: Fix lesspipe file name and use gexps.
(name . kiasoc5)(address . kiasoc5@disroot.org)(address . 57927@debbugs.gnu.org)
87bkr1ey8x.fsf@nckx
Hi kiasoc5,

kiasoc5 via Guix-patches via ???
Toggle quote (3 lines)
> 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.

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

Toggle quote (2 lines)
> + (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

-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYzL8Hw0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW15Hk8BAIhX/toTe0VN9rKtBgilHq1O92C2catpmlYdvRbq
hjwpAQCnKiUPahy4P94xg3EpYpdsQya31Bz7j6uZq2YfkhXCAQ==
=H94z
-----END PGP SIGNATURE-----

K
K
kiasoc5 wrote on 28 Sep 2022 06:34
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 57927@debbugs.gnu.org)
20220928043407.181d9060@aria
Hi Tobias,

On Tue, Sep 27 2022, 03:22:19 PM +0200
Tobias Geerinckx-Rice <me@tobias.gr> wrote:

Toggle quote (11 lines)
> 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?

Toggle quote (10 lines)
> > * 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?
M
M
Maxime Devos wrote on 28 Sep 2022 23:14
(address . 57927@debbugs.gnu.org)
0052b9b3-099e-260f-10d5-48e6e92e1a55@telenet.be
On 28-09-2022 06:34, kiasoc5 via Guix-patches via wrote:
Toggle quote (9 lines)
>>> * 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.
Toggle quote (1 lines)
> 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: OpenPGP_signature
M
M
Maxime Devos wrote on 28 Sep 2022 23:16
(address . 57927@debbugs.gnu.org)
7c07a1ad-56e2-f9bd-d656-7f9575f54311@telenet.be
On 28-09-2022 06:34, kiasoc5 via Guix-patches via wrote:
Toggle quote (2 lines)
> 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: OpenPGP_signature
?