Bug#1007842: Bug#909124: quilt: Please fail gracefully on 'quilt series' when less(1) is not installed
Daniel Shahaf wrote:
> > FAILS: env PAGER=cat quilt series
> > WORKS: env -u LESS PAGER=cat quilt series
> >
> >
> > This is actually a separate but related bug in quilt.
> > If $LESS is set, quilt ignores $PAGER and forces less.
> > This is wrong.
> ⋮
> > 18:38 [ -n "$LESS" -a -z "${QUILT_PAGER+x}" ] &&
> > QUILT_PAGER="less -FRX"
>
> Agreed. If Alice normally uses «export PAGER=less LESS=S» and then sets
> PAGER=foo, that's the pager quilt shoult use.
>
> Cloned as -2. The above patch does _not_ fix it.
>
> > If quilt wants to override the user's requested $LESS,
> > it should do so with "export LESS=FRX",
> > entirely independent of $QUILT_PAGER'.
>
> This particular approach would be lossy: it would overwrite the user's
> value of $LESS. Instead, quilt could _append_ to $LESS, or pass -R into
> less(1)'s argv, or only use $LESS as a hint if PAGER and GIT_PAGER are
> also unset and less(1) is installed, or document that the user should
> configure their $PAGER / $LESS / $QUILT_PAGER envvars with -R, or…
OK that sounds reasonable.
The part I care about is "don't force PAGER=less when LESS=x".
I don't really care about the EXACT way that is achieved.
All else being equal, I think quilt should mimic git's equivalent logic.
I guess that's here:
https://sources.debian.org/src/git/1:2.35.1-1/Documentation/config/core.txt/?hl=508#L496-L519
Bug#909124: quilt: Please fail gracefully on 'quilt series' when less(1) is not installed
Control: clone 909124 -2
Control: severity -2 normal
Control: retitle -2 quilt: don't set QUILT_PAGER=less when $LESS is set
Control: tags -2 upstream
Control: found -2 0.66-2.1
Control: tags 909124 upstream patch
Control: found 909124 0.66-2.1
Control: severity 909124 minor
Trent W. Buck wrote on Thu, Mar 17, 2022 at 18:43:20 +1100:
> Trent W. Buck wrote:
> > +++ b/usr/share/quilt/scripts/patchfns
> > @@ -,7 +,7 @@ setup_pager()
> > - QUILT_PAGER=${QUILT_PAGER-${GIT_PAGER-${PAGER-less -R}}}
> > + QUILT_PAGER=${QUILT_PAGER:-${GIT_PAGER:-${PAGER:-less -R}}}
Thanks. I'm posting a patch for this bug (#909124 == #-1), and bumping
it to "minor" after re-reviewing
.
[[[
--- a/quilt/scripts/patchfns.in
+++ b/quilt/scripts/patchfns.in
@@ -,7 +,7 @@ setup_pager()
# QUILT_PAGER = QUILT_PAGER | GIT_PAGER | PAGER | less -R
# NOTE: QUILT_PAGER='' is significant
- QUILT_PAGER=${QUILT_PAGER-${GIT_PAGER-${PAGER-less -R}}}
+ QUILT_PAGER=${QUILT_PAGER-${GIT_PAGER-${PAGER-/usr/bin/sensible-pager}}}
[ -z "$QUILT_PAGER" -o "$QUILT_PAGER" = "cat" ] && return 0
]]]
More below.
> FAILS: env PAGER=cat quilt series
> WORKS: env -u LESS PAGER=cat quilt series
>
>
> This is actually a separate but related bug in quilt.
> If $LESS is set, quilt ignores $PAGER and forces less.
> This is wrong.
⋮
> 18:38 [ -n "$LESS" -a -z "${QUILT_PAGER+x}" ] && QUILT_PAGER="less
> -FRX"
Agreed. If Alice normally uses «export PAGER=less LESS=S» and then sets
PAGER=foo, that's the pager quilt shoult use.
Cloned as -2. The above patch does _not_ fix it.
> If quilt wants to override the user's requested $LESS,
> it should do so with "export LESS=FRX",
> entirely independent of $QUILT_PAGER'.
This particular approach would be lossy: it would overwrite the user's
value of $LESS. Instead, quilt could _append_ to $LESS, or pass -R into
less(1)'s argv, or only use $LESS as a hint if PAGER and GIT_PAGER are
also unset and less(1) is installed, or document that the user should
configure their $PAGER / $LESS / $QUILT_PAGER envvars with -R, or…
Cheers,
Daniel
P.S. «sed -i s/Upstream-status/Forwarded/ debian/patches/check_SERIES_exists»
would make that patch's headers compatible with DEP-3 parsers.
> 18:32 And for comparison, "PAGER=cat git diff" *is* working for git in
> the same environment.
> 18:32 I wonder if quilt is getting confused because $HOME doesn't exist?
> 18:33 Nope. But "env -i PAGER=cat quilt series" behaves
> 18:34 Bizarre...
> 18:36 Get Fucked.
> 18:36 env -u LESS fixes it
> 18:36 But why?
> 18:37 because quilt is broken I guess
> 18:37 $LESS should be read by less(1), not by quilt(1)
> 18:38 [ -n "$LESS" -a -z "${QUILT_PAGER+x}" ] && QUILT_PAGER="less
> -FRX"
> 18:38 Ugh.
> 18:39 "env" didn't show anything, because it wasn't exported. We
> should have used "set" instead.
>
Bug#909124: quilt: Please fail gracefully on 'quilt series' when less(1) is not installed
Trent W. Buck wrote:
> I ran into the same bug, except PAGER=cat also fails.
>
> I also tried fixing ${x-fallback} to the more normal ${x:-fallback}, but it
> did not help.
>
> I do not understand why that is happening.
>
> root@hera:/gdk-pixbuf# quilt series
> /usr/share/quilt/scripts/patchfns: line 1128: less: command not found
>
> root@hera:/gdk-pixbuf# PAGER=cat quilt series
> /usr/share/quilt/scripts/patchfns: line 1128: less: command not found
I have some more info.
FAILS: env PAGER=cat quilt series
WORKS: env -u LESS PAGER=cat quilt series
This is actually a separate but related bug in quilt.
If $LESS is set, quilt ignores $PAGER and forces less.
This is wrong.
If quilt wants to override the user's requested $LESS,
it should do so with "export LESS=FRX",
entirely independent of $QUILT_PAGER'.
18:32 And for comparison, "PAGER=cat git diff" *is* working for git in
the same environment.
18:32 I wonder if quilt is getting confused because $HOME doesn't exist?
18:33 Nope. But "env -i PAGER=cat quilt series" behaves
18:34 Bizarre...
18:36 Get Fucked.
18:36 env -u LESS fixes it
18:36 But why?
18:37 because quilt is broken I guess
18:37 $LESS should be read by less(1), not by quilt(1)
18:38 [ -n "$LESS" -a -z "${QUILT_PAGER+x}" ] && QUILT_PAGER="less
-FRX"
18:38 Ugh.
18:39 "env" didn't show anything, because it wasn't exported. We
should have used "set" instead.
Bug#909124: quilt: Please fail gracefully on 'quilt series' when less(1) is not installed
I ran into the same bug, except PAGER=cat also fails.
I also tried fixing ${x-fallback} to the more normal ${x:-fallback}, but it did
not help.
I do not understand why that is happening.
root@hera:/gdk-pixbuf# quilt series
/usr/share/quilt/scripts/patchfns: line 1128: less: command not found
root@hera:/gdk-pixbuf# PAGER=cat quilt series
/usr/share/quilt/scripts/patchfns: line 1128: less: command not found
root@hera:/gdk-pixbuf# env | grep PAGER
PAGER=cat
root@hera:/gdk-pixbuf# emacs /usr/share/quilt/scripts/patchfns +1128
root@hera:/gdk-pixbuf# git diff --no-index
/usr/share/quilt/scripts/patchfns.~1~ /usr/share/quilt/scripts/patchfns
diff --git a/usr/share/quilt/scripts/patchfns.~1~
b/usr/share/quilt/scripts/patchfns
index 4a36335d..a7e6e54d 100644
--- a/usr/share/quilt/scripts/patchfns.~1~
+++ b/usr/share/quilt/scripts/patchfns
@@ -,7 +,7 @@ setup_pager()
# QUILT_PAGER = QUILT_PAGER | GIT_PAGER | PAGER | less -R
# NOTE: QUILT_PAGER='' is significant
- QUILT_PAGER=${QUILT_PAGER-${GIT_PAGER-${PAGER-less -R}}}
+ QUILT_PAGER=${QUILT_PAGER:-${GIT_PAGER:-${PAGER:-less -R}}}
[ -z "$QUILT_PAGER" -o "$QUILT_PAGER" = "cat" ] && return 0
root@hera:/gdk-pixbuf# PAGER=cat quilt series
/usr/share/quilt/scripts/patchfns: line 1128: less: command not found
Bug#909124: quilt: Please fail gracefully on 'quilt series' when less(1) is not installed
Package: quilt Version: 0.65-2 Severity: wishlist Dear Maintainer, In a minimal sid chroot, where less(1) is not installed: % QUILT_PATCHES=debian/patches quilt series /usr/share/quilt/scripts/patchfns: line 1109: less: command not found % Please consider using a fallback (such as /usr/bin/pager or no pager at all) when less(1) is not installed. Cheers, Daniel P.S. Filing this as a wishlist bug because I see that quilt Recommends: less.)
