Fix exporting LESSOPEN

[stroggoslav]

Do not use eval and export LESSOPEN directly only when is empty.
This will prevent problems on distributions using different lesspipe
and/or defining LESSOPEN variable on their own.

Issue #46 #69

---

Fixes #46 / Fixes #69 / Fixes #296

[akinomyoga]

Thank you for your contribution! I think the check for an empty LESSOPEN is nice. In which systems have you tested this patch? Actually, I'm not familiar with these different lesspipe implementations, but I have several questions:

• It seems like eval "$(lesspipe)" sets also LESSCLOSE for cleanup in some systems.
• I'm wondering if /usr/bin/lesspipe, if any, always outputs the same value for LESSOPEN in any systems.
• Have you tested it with Gentoo? Does this fix also solve Issue lesspipe in gentoo differs from ubuntu #46 (BASH ERROR #69), which was originally reported for Gentoo? There seems to be already a fix lib/bourne-shell : Blind fix for gentoo compatibility #74, which I don't think the right fix but seems to work in Gentoo.

[stroggoslav]

* It seems like `eval "$(lesspipe)"` sets also `LESSCLOSE` for cleanup in some systems.

I didn't found it in lesspipe script code.

* I'm wondering if `/usr/bin/lesspipe`, if any, always outputs the same value for `LESSPIPE` in any systems.

If we are talking about "original" lesspipe then this is code responsible for output when no arguments are supplied to script:

   if [[ "$SHELL" = *csh ]]; then
     echo "setenv LESSOPEN \"|$pat$0 %s\""
   else
     echo "LESSOPEN=\"|$pat$0 %s\""
     echo "export LESSOPEN"
   fi

Only difference is when dealing with csh. Since we are dealing here with bash exclusively we can just use this code. I think it is safer not to use eval as much as possible and it solves problems between lesspipe implementations. It also causes less overhead as we do not spawn another processes. Moreover Gentoo handles setting LESSOPEN itself and maybe some others distributions do this also.

* Have you tested it with Gentoo? Does this fix also solve Issue [lesspipe in gentoo differs from ubuntu #46](https://github.com/ohmybash/oh-my-bash/issues/46) ([BASH ERROR #69](https://github.com/ohmybash/oh-my-bash/issues/69)), which was originally reported for Gentoo? There seems to be already a fix [lib/bourne-shell : Blind fix for gentoo compatibility #74](https://github.com/ohmybash/oh-my-bash/pull/74), which I don't think the right fix but seems to work in Gentoo.

Yes I am Gentoo user and I've made this fix because of those errors. This blind fix is what is says: blind. It does not solve anything because error is generated by Gentoo flavor of lesspipe not by defining which SHELL we are going to use.

[akinomyoga]

If we are talking about "original" lesspipe then this is code responsible for output when no arguments are supplied to script:

I'm not sure which is the original lesspipe and which version of the original lesspipe you are talking about, but there seem to be systems whose /usr/bin/lesspipe produces a different result according to the pages e.g. (mcorbin.fr): One day one manpage: lesspipe and ohmyzsh/ohmyzsh#262. This means that this PR potentially breaks the behavior in such a system.

Yes I am Gentoo user and I've made this fix because of those errors. This blind fix is what is says: blind. It does not solve anything because error is generated by Gentoo flavor of pipeline not defining which SHELL we are going to use.

Do you mean these errors are still present after "the blind fix #74"?

If so, I'm not sure if this PR is the right fix. According to the man pages, eval "$(lesspipe)" is the expected way of use. If this produces the error, there must be problems elsewhere.

Could you explain why eval "$(lesspipe)" causes the error in Gentoo and why this PR replacing eval "$(lesspipe)" with the result of lesspipe of the specific version in the specific system is the right fix? If you don't know these answers, I must say this PR is also another blind fix, i.e., a fix made without knowing the root causes. [ Edit: If you don't know the answer, that's fine. There is no problem. We can just start finding out what is the root cause now. Can you help me with finding it out? ]

[stroggoslav]

If we are talking about "original" lesspipe then this is code responsible for output when no arguments are supplied to script:

I'm not sure which is the original lesspipe and which version of the original lesspipe you are talking about, but there seem to be systems whose /usr/bin/lesspipe produces a different result according to the pages e.g. (mcorbin.fr): One day one manpage: lesspipe and ohmyzsh/ohmyzsh#262. This means that this PR potentially breaks the behavior in such a system.

I was referring to https://github.com/wofr06/lesspipe. I used quotes for word "original" because I didn't found better maybe I should use "most popular". Gentoo have available this version in packages and saves it as lesspipe.sh. According to links You have provided the only difference in output is existence of aforementioned LESSCLOSE variable which seems be missing in today implementations at least those I've managed to find out - others must be not very popular it seems. Other thing is that links are very old also and represents outdated informations.

Yes I am Gentoo user and I've made this fix because of those errors. This blind fix is what is says: blind. It does not solve anything because error is generated by Gentoo flavor of lesspipe not defining which SHELL we are going to use.

Do you mean these errors are still present after "the blind fix #74"?

Yes they are.

If so, I'm not sure if this PR is the right fix. According to the man pages, eval "$(lesspipe)" is the expected way of use. If this produces the error, there must be problems elsewhere.

If so then problem exists in not standardized lesspipe implementations. Man page is referring to implementation that You are mentioning I think it is not standard - fix me if I am wrong on this.

Could you explain why eval "$(lesspipe)" causes the error in Gentoo and why this PR replacing eval "$(lesspipe)" with the result of lesspipe of the specific version in the specific system is the right fix? If you don't know these answers, I must say this PR is also another blind fix, i.e., a fix made without knowing the root causes.

Gentoo use it's own implementation of lesspipe which expects arguments and produces message if not provided. This message is evaluated and this produces error message "bash: syntax error near unexpected token `newline'". Gentoo sets itself LESSOPEN variable, never LESSCLOSE. Using eval is insecure in my opinion. We are trusting that all implementations will behave the same and output of executed command without arguments will be valid and safe shell code.

[stroggoslav]

For readability I will put my edit in separate message:

[...]replacing eval "$(lesspipe)" with the result of lesspipe of the specific version in the specific system[...]

I am not using "result of lesspipe of the specific version in the specific system" - Value of LESSOPEN for less is consistent between all versions from at least 10 years. It contains pipe character '|' then path to or name of script lesspipe and %s parameter. Gentoo set is as:

LESSOPEN="|lesspipe %s"

I think if devs of Gentoo put this to base system env variables we can also trust that it will not change.

Aforementioned:
echo "LESSOPEN=\"|$pat$0 %s\""
Generates path to script from actual script name but since we are checking for it with fixed path and filename
[ -x /usr/bin/lesspipe ]
Then as well we can set it manually.

[akinomyoga]

Thank you for your explanation! OK, I think I now see the problem. So, the Gentoo lesspipe is not what you say "the original/most popular lesspipe" but is Gentoo's own implementation and produces error messages to stdout when it's called without arguments? Anyway, I'm still not sure if we should merge this PR as is. See the following discussion:

---

I was referring to https://github.com/wofr06/lesspipe. I used quotes for word "original" because I didn't found better maybe I should use "most popular". Gentoo have available this version in packages and saves it as lesspipe.sh. [...] at least those I've managed to find out - others must be not very popular it seems.

I have checked lesspipe and lesspipe.sh of different systems, but I can hardly say wofr06/lesspipe is the original nor the most popular lesspipe. Ubuntu and Debian have their own implementation of lesspipe. Fedora / RHEL / CentOS also have another version of lesspipe.sh which is independent of wofr06/lesspipe. OpenSUSE doesn't seem to support lesspipe. Arch provides wofr06/lesspipe as lesspipe.sh. FreeBSD provides another very simple lesspipe.sh which is similar to the example provided in the man page of the original less (I here wrote original, but actually there is only one implementation of less by M. Nudelman AFAIK). Solaris, Minix and Cygwin do not provide lesspipe. I'm not sure what is your measure of defining the most popular one, but I doubt you have actually tried different systems.

Maybe wofr06/lesspipe is the most popular repository on GitHub, but traditional packages are not necessarily on GitHub. And, search results do not reflect the actual share in general. Also, most lesspipe.sh implementations seem to be historically maintained as just a configuration file of each system, so they don't usually have their own repository and official pages.

If so, I'm not sure if this PR is the right fix. According to the man pages, eval "$(lesspipe)" is the expected way of use. If this produces the error, there must be problems elsewhere.

If so then problem exists in not standardized lesspipe implementations.

Yes, I think so.

Man page is referring to implementation that You are mentioning I think it is not standard - fix me if I am wrong on this.

You are right. Last night I've quickly checked several man pages of different lesspipe implementations and found that at least two or three implementations accept this type of initialization. But this morning, I checked the real implementations and found that it seems the Fedora/RHEL/CentOS implementation and the FreeBSD don't output anything when it's called without arguments. Instead, they provide /etc/profile.d/less.sh, etc. to set up the environment variable in the login shells.

Gentoo use it's own implementation of lesspipe which expects arguments and produces message if not provided. This message is evaluated and this produces error message "bash: syntax error near unexpected token `newline'".

Thanks for the explanation. As I have written above, I now understood the situation.

Now I'm wondering why Gentoo's implementation outputs the error message to stdout instead of stderr. Maybe that is not actually an error message but some contents that are supposed to be shown in less.

• Could you provide us with the exact output of $ lesspipe in Gentoo?

Using eval is insecure in my opinion. We are trusting that all implementations will behave the same and output of executed command without arguments will be valid and safe shell code.

I'm not sure if we can say insecure in the sense that we are finally going to run the lesspipe command through LESSOPEN which means that we trust lesspipe after all. But, yes, it's insecure, or at least a problem, in the sense that the implementations are not necessarily designed to output the initialization code without arguments and outputs something other to stdout, which causes unexpected results. Anyway, we are receiving the issues only from the Gentoo lesspipe implementation although there are many different implementations of lesspipe. In fact, all the other implementations that I have tested either produce a shell initialization code or output nothing to stdout/stderr, which does not cause the present problem.

I am not using "result of lesspipe of the specific version in the specific system" - Value of LESSOPEN for less is consistent between all versions from at least 10 years.

What do you mean by all versions? I don't think you have checked "all" the Unix-like systems of the last decade, which include Linux distributions, *BSD, macOS/OS X/Mac OS X, Minix, AIX, Solaris, Cygwin/MSYS2/MSYS, etc. It's evident from the fact that lesspipe / lesspipe.sh in the most popular branches of Linux distributions, Ubuntu/Debian and Fedora/RHEL/CentOS, uses different setups from that in Gentoo you provided. In both Ubuntu 20.0 LTS and Debian 10, lesspipe outputs

$ lesspipe
export LESSOPEN="| /usr/bin/lesspipe %s";
export LESSCLOSE="/usr/bin/lesspipe %s %s";

In the five systems I've checked---Fedora 35 and 30, RHEL 8.5, CentOS 8.3, and Scientific Linux 7.7 (where the initialization is made in /etc/profile.d/less.sh)---, the following command revealed the same setup with the double vertical bars:

$ declare -p ${!LESS@}
declare -x LESSOPEN="||/usr/bin/lesspipe.sh %s"

Aforementioned:
echo "LESSOPEN=\"|$pat$0 %s\""
Generates path to script from actual script name but since we are checking for it with fixed path and filename
[ -x /usr/bin/lesspipe ]

Yeah, I've also noticed this inconsistency in working on this PR and was thinking of addressing it as a separate issue. Also, it seems there are two variations, lesspipe and lesspipe.sh, so I think we can check both names. I'm rather wondering why we need to use the fixed path /usr/bin/lesspipe. I don't have real experiences with NixOS, but what happens in NixOS where all the binaries are placed in different places for different environments.

---

I feel we need to collect more information on Gentoo's own implementation. Why do they output error messages to stdout instead of stderr? Is there a chance that the configuration of the upstream Gentoo would be updated? Besides, we also need to think about how to work around the problem in existing Gentoo systems.

[stroggoslav]

Wow great job checking all those systems! I should do a better research. I will change this PR to Draft for now.

First quick clarification on one thing:

Now I'm wondering why Gentoo's implementation outputs the error message to stdout instead of stderr.

It is not error message it is just message.
Usage: lesspipe <file>
When it is feed to eval it produces error message then.

In fact, all the other implementations that I have tested either produce a shell initialization code or output nothing to stdout/stderr, which does not cause the present problem.

But from our point of view this behavior is also unwelcome because we are need to set up LESSOPEN.

So summarizing differences between those implementations and systems that You have checked are:

• scriptname: lesspipe vs lesspipe.sh
• called without arguments produce either output which can be evaluated or no output or message with usage.
• when lesspipe output code to evaluation it can either be LESSOPEN or LESSOPEN and LESSCLOSE
• when lesspipe do not output code systems usually take cares for setting those variables
• and the one little confusing: values of LESSOPEN and LESSCLOSE generally are same for all those versions except some use double pipe instead of one LESSOPEN="| /usr/bin/lesspipe %s" vs LESSOPEN="||/usr/bin/lesspipe.sh %s"

I feel we need to collect more information on Gentoo's own implementation. Why do they output error messages to stdout instead of stderr? Is there a chance that the configuration of the upstream Gentoo would be updated? Besides, we also need to think about how to work around the problem in existing Gentoo systems.

As I said above this isn't error message it's just usage message. Personally I do not see anything wrong with this.

If we must stay with eval then my proposed solution (based on points above):
Check if LESSOPEN is empty. Then check if either lesspipe or lesspipe.sh is present on the system. Then run old code eval "$(SHELL=/bin/sh lesspipe)" or eval "$(SHELL=/bin/sh lesspipe.sh)" depending on scriptname.

[akinomyoga]

Thank you for the nice summary!

It is not error message it is just message.
Usage: lesspipe <file>
When it is feed to eval it produces error message then.

Oh, I see. Then > caused the syntax error.

• when lesspipe do not output code systems usually take cares for setting those variables

Some remark: In some systems, the default .bash_profile or .bashrc contains the initialization code for lesspipe while the latter file (.bashrc) is replaced by oh-my-bash.

As I said above this isn't error message it's just usage message. Personally I do not see anything wrong with this.

OK, the behavior isn't that strange as a Unix command. But I still think we may ask if they would change the behavior or not since outputting usage to stderr for the unexpected usage is also another understandable behavior. Or maybe we can suggest outputting the shell initialization code for the execution without arguments in line with other implementations, which is more constructive.

If we must stay with eval then my proposed solution (based on points above):
Check if LESSOPEN is empty. Then check if either lesspipe or lesspipe.sh is present on the system. Then run old code eval "$(SHELL=/bin/sh lesspipe)" or eval "$(SHELL=/bin/sh lesspipe.sh)" depending on scriptname.

What happens in Gentoo with this treatment? I think this solves the problem if LESSOPEN is already set elsewhere in Gentoo, but we need an additional treatment if it's not set.

In addition, maybe we can check the output of lesspipe/lesspipe.sh before passing it to eval.

For example, we store the result of lesspipe in the variable code and then check its syntax using "$BASH" -n - <<< "$code" 2>/dev/null. But bash -n only checks the syntax and cannot check if it is the expected code or not. In the case of Gentoo lesspipe, Usage: lesspipe <file> becomes a syntax error, but it wouldn't become a syntax error if it was Usage: lesspipe FILE as Usage: is a possible command name. I'm not sure if this is a robust assumption, but we may check whether the result contains the string LESS.

[stroggoslav]

• when lesspipe do not output code systems usually take cares for setting those variables

Some remark: In some systems, the default .bash_profile or .bashrc contains the initialization code for lesspipe while the latter file (.bashrc) is replaced by oh-my-bash.

As I said above this isn't error message it's just usage message. Personally I do not see anything wrong with this.

OK, the behavior isn't that strange as a Unix command. But I still think we may ask if they would change the behavior or not since outputting usage to stderr for the unexpected usage is also another understandable behavior. Or maybe we can suggest outputting the shell initialization code for the execution without arguments in line with other implementations, which is more constructive.

You are right - incorrect usage / missing argument should be reported to stderr.

If we must stay with eval then my proposed solution (based on points above):
Check if LESSOPEN is empty. Then check if either lesspipe or lesspipe.sh is present on the system. Then run old code eval "$(SHELL=/bin/sh lesspipe)" or eval "$(SHELL=/bin/sh lesspipe.sh)" depending on scriptname.

What happens in Gentoo with this treatment? I think this solves the problem if LESSOPEN is already set elsewhere in Gentoo, but we need an additional treatment if it's not set.

We can recognize Gentoo system and do according so. AFAIK file /etc/gentoo-release should be present on Gentoo system.

In addition, maybe we can check the output of lesspipe/lesspipe.sh before passing it to eval.

For example, we store the result of lesspipe in the variable code and then check its syntax using "$BASH" -n - <<< "$code" 2>/dev/null. But bash -n only checks the syntax and cannot check if it is the expected code or not. In the case of Gentoo lesspipe, Usage: lesspipe <file> becomes a syntax error, but it wouldn't become a syntax error if it was Usage: lesspipe FILE as Usage: is a possible command name. I'm not sure if this is a robust assumption, but we may check whether the result contains the string LESS.

I feel it is a hacky solution. But if we are suppose to search for keywords it's better to search for export LESSOPEN and hope no line like "Add this to .bashrc" exists.

I was curious why double pipe || is present on RedHat-like distros and found this:
https://bugzilla.redhat.com/show_bug.cgi?id=615303
https://bugzilla.redhat.com/show_bug.cgi?id=1254837

I've recreated mentioned bug and fixed it by adding second pipe at the begin. I've tested also behavior on some pdfs and bins and it also worked correctly as it should.

I've also studied less man pages (which should I do at the very beginning) where everything is nicely explained why and when we use LESSOPEN, LESSCLOSE, single, double and no pipes.

So basing on all this I am pretty sure that we could use version with double pipe export LESSOPEN="|| lesspipe(.sh) %s" as "default" one.

[akinomyoga]

The nice investigation, thank you!

OK, the behavior isn't that strange as a Unix command. But I still think we may ask if they would change the behavior or not since outputting usage to stderr for the unexpected usage is also another understandable behavior. Or maybe we can suggest outputting the shell initialization code for the execution without arguments in line with other implementations, which is more constructive.

You are right - incorrect usage / missing argument should be reported to stderr.

Are you going to ask it to the upstream? If not, maybe I can ask it to the Gentoo upstream, but I don't know which Gentoo package provides /usr/bin/lesspipe. The package name of the file seems to be able to be obtained by equery belongs -e /usr/bin/lesspipe in Gentoo, but I don't have currently working Gentoo systems, unfortunately.

So basing on all this I am pretty sure that we could use version with double pipe export LESSOPEN="|| lesspipe(.sh) %s" as "default" one.

What do you mean by the default one? If I'd write a new implementation of lesspipe, I feel we should by default design it so that it can be used with the double pipes. But if we are talking about the settings for an unknown lesspipe, I'm not sure whether the double pipes would be a good default setting. If a lesspipe designed for the single pipe (and don't care about its exit status) is used with the double pipes, we cannot open even a simple text file in less. <-- I'm not sure about the Gentoo lesspipe implementation, but if it outputs nothing for a simple text file and exits with the status 0. --> Actually, any of these three different lesspipe protocols are incompatible with one another, so we cannot really decide which is the good default, but I think the single pipe is the least problematic choice as it only causes the problem with binary files of the empty content. The double pipes will cause problems for all text files when lesspipe is designed for a single pipe.

Anyway, I think now you can update the PR to cover the above discussions. Then I'll again review it.

[stroggoslav]

The nice investigation, thank you!

OK, the behavior isn't that strange as a Unix command. But I still think we may ask if they would change the behavior or not since outputting usage to stderr for the unexpected usage is also another understandable behavior. Or maybe we can suggest outputting the shell initialization code for the execution without arguments in line with other implementations, which is more constructive.

You are right - incorrect usage / missing argument should be reported to stderr.

Are you going to ask it to the upstream? If not, maybe I can ask it to the Gentoo upstream, but I don't know which Gentoo package provides /usr/bin/lesspipe. The package name of the file seems to be able to be obtained by equery belongs -e /usr/bin/lesspipe in Gentoo, but I don't have currently working Gentoo systems, unfortunately.

I've already found where this file came from. Theoretically it's part of the sys-apps/less package but script itself is added after compilation during intallation from portage tree files directly. It is not in less source files. I will take care of this in a free moment.

So basing on all this I am pretty sure that we could use version with double pipe export LESSOPEN="|| lesspipe(.sh) %s" as "default" one.

What do you mean by the default one? If I'd write a new implementation of lesspipe, I feel we should by default design it so that it can be used with the double pipes. But if we are talking about the settings for an unknown lesspipe, I'm not sure whether the double pipes would be a good default setting. If a lesspipe designed for the single pipe (and don't care about its exit status) is used with the double pipes, we cannot open even a simple text file in less. <-- I'm not sure about the Gentoo lesspipe implementation, but if it outputs nothing for a simple text file and exits with the status 0. --> Actually, any of these three different lesspipe protocols are incompatible with one another, so we cannot really decide which is the good default, but I think the single pipe is the least problematic choice as it only causes the problem with binary files of the empty content. The double pipes will cause problems for all text files when lesspipe is designed for a single pipe.

I mean that if We are going NOT to use (it's only hypothetical) eval and use "export LESSOPEN" directly then version with double pipes should be working best on most of available distros and with most lesspipes.

But I've tested this double pipes with "lesspipe designed for a single pipe". Gentoo's setting for LESSOPEN is single pipe but it worked perfectly (if not better for binary and empty files ) with double. I will test it for others lesspipes on different distros.

Scratch that. It does not works.

[stroggoslav]

Et voilà monsieur!

[akinomyoga]

Thank you! I think this is mostly fine. I have additional comments.

[akinomyoga]

Thank you! Now LGTM. This is still marked as a draft, but do you have any plan to update it further?

[stroggoslav]

Nope.

[akinomyoga]

OK, thank you for your PR and quick responses to requests. Let us now merge the PR.