[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

Debbugs page

kiasoc5 wrote 2 years ago
(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
Mathieu Othacehe wrote 2 years ago
(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
Tobias Geerinckx-Rice wrote 2 years ago
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-----

kiasoc5 wrote 2 years ago
(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?
Maxime Devos wrote 2 years ago
(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_0x49E3EE22191725EE.asc
Attachment: OpenPGP_signature
Maxime Devos wrote 2 years ago
(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_0x49E3EE22191725EE.asc
Attachment: OpenPGP_signature
?
Your comment

This issue is archived.

To comment on this conversation send an email to 57927@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 57927
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help