🐛 Setting $PAGER to use delta results in an infinite loop

[zachriggle]

It looks like we wend up in an infinite fork/exec loop if you do the following.

$ export PAGER='delta -n'
$ git -c core.pager='delta -n' log -p

Because delta itself is trying to invoke the $PAGER to paginate its own output.

delta should detect this situation (e.g. in Python it would be os.environ.get('PAGER', '').startswith('delta ') and emit an error message in this case.

Otherwise, it's just an infinite hang and all CPU cores spinning as each delta process tries to spawn yet another delta process.

I discovered this while using $PAGER within a script to be what I would set Git's pager to, and it turns out that this error cropped up because of the specific choice of my variable name.

Similar gotchas with shell scripting happen with other important, magic variables, e.g. $LINES is the number of lines in your terminal and cannot be overridden. LINES, like PAGER, are important and should not be used for holding random variables!

$ echo $LINES
47
$ LINES=1
$ echo $LINES
47

[dandavison]

Hi @zachriggle I'm not saying your suggestion to protect against this is a bad one, but, you intended to use GIT_PAGER I think? delta is not a general purpose pager so I think one would only set PAGER=delta by accident (since as you say it's a special variable name); however GIT_PAGER=delta absolutely makes sense (and doesn't cause a loop).

We can do what you suggest. bat does it (but bat is a general-purpose pager). We'd have to pattern match to handle e.g. PAGER=/usr/local/bin/delta as well as PAGER=delta, with and without arguments. Furthermore it may not be possible to get Delta into Ubuntu/Debian unless it is installed with a different name, so I guess we'd have to handle that name also. (Or perhaps we can look at argv[0]?)

Incidentally, with lines, it seems that a newline / prompt display makes a difference? (this is zsh on macos but my bash behaves similarly albeit with echo resolving to an executable):

~ which echo
echo: shell built-in command
~ LINES=1; echo $LINES
1
~ echo $LINES
58
~ LINES=1
~ echo $LINES
58

[zachriggle]

It was set intentionally, but I had forgotten that $PAGER is a special variable. I was just using it to hold the value of the pager passed to -c core.pager=....

Diagnosing the issue took a long while, so it would be nice to have protection against it. It could work even if delta is named differently, just check against argv[0] in $PAGER.

In any case, I did end up using GIT_PAGER instead -- although I didn't know that was also a special variable. These were just variable names I was using in a script.

[dandavison]

OK, thanks, let's do it then! (It has been raised once before so this seems fairly clearly justified). Let me know if you'd like to make the PR, otherwise I can.

[zachriggle]

My employer forbids FOSS contributions so unfortunately I cannot add a PR

On Tue, Mar 2, 2021 at 3:48 PM Dan Davison ***@***.***> wrote: OK, thanks, let's do it then! (It has been raised once before so this seems fairly clearly justified). Let me know if you'd like to make the PR, otherwise I can. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#529 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA3IGBD6527AX7PQAJNPZTTBVFI5ANCNFSM4YOKLODQ> .

-- *Zach Riggle*

[dandavison]

My employer forbids FOSS contributions so unfortunately I cannot add a PR

Ah right, I think I remember that from before. No problem, done in #531 (to be included in next release).