Bug 615303

It is too late for exceptions. Moving to 6.1.

Created attachment 467496 [details]
Patch to fix empty lessopen.sh pipe output

Created attachment 467503 [details]
New version of lesspipe.sh with correct exit statuses (needed by previous patch)

Testing the patch:

$ tcsh # because of previous issues with printexitstatus
$ set printexitvalue=1
$ env SHELL="/bin/tcsh" LESSOPEN="|`pwd`/lesspipe.sh %s" ./less <file>

<file> is for example "empty.gz"
(created by $ touch empty; gzip empty)

Comment on attachment 467496 [details]
Patch to fix empty lessopen.sh pipe output

looks good to me, feel free to commit it and try to send patch to upstream ;)

Thanks for the work on this. :)

Unfortunately, however, the update on Fedora (less-436-8.fc14.i686) seems to have introduced a new regression. It will no longer decompress files ending in .1.gz -- empty or not:

# echo test | gzip > messages.1.gz
# less messages.1.gz
"messages.1.gz" may be a binary file.  See it anyway?

messages.1.gz = a rotated copy of /var/log/messages produced by logrotate with the "compress" option enabled.

Thank you for reporting this, Jordan.
I have fixed the regression in current Fedora releases, so you can get an update soon.

Waiting for upstream comment.

I have prepared backward-compatibility patch as well (requiring two vertical bars in row), to preserve functionality of third party LESSOPEN scripts that disrespect correct exit statuses.

This issue was proposed for RHEL 6.1 FasTrack but did not get resolved in time.
It has been moved to RHEL 6.2 FasTrack.

This issue was proposed for RHEL 6.1 FasTrack but did not get resolved in time.
It has been moved to RHEL 6.2 FasTrack.

Created attachment 514738 [details]
1) lesspipe.sh with correct exit statuses

This file is now used in current Fedora releases..

Created attachment 514739 [details]
2) Backward-compatible patch to fix empty lessopen.sh pipe output

This patch is enabled by using two '|' colons in row at the beginning of the LESSOPEN variable to prevent backward-compatibility of third party LESSOPEN scripts/commands.
See patched man pages for more information.

Moving to 6.3, as we still don't have any response from the upstream.

Even though the patch is backward-compatible also with 3rd party scripts (and Fedora proves it's functionality), we can't be sure the patch won't break LESSOPEN string compatibility with future upstream versions. For example, this could eventually cause some difficulties while upgrading to RHEL-7 and using customized or 3rd party scripts.

Setting Cond NACK UPSTREAM as well.

Upstream response:

> > Hi Vojtech,
> > Sorry I haven't responded earlier.  I did receive your patch but haven't had much
> > time to work on less issues lately.  My major concerns would be the two points that
> > you yourself raised -- its use of /dev/null is Unix-specific, and the potential
> > incompatibility with existing LESSOPEN scripts that may return random exit codes.
> > The second issue is the most serious, and I think we'd need to have some way to 
> > distinguish "new" LESSOPEN scripts that use the exit code for this purpose from
> > old ones.
> Actually, in Fedora, we are using *two vertical bars* in row as a workaround:
>     eg. LESSOPEN="||/usr/bin/lesspipe.sh %s"
I see.  I'm a little leery of the proliferation of cryptic characters appearing at the start of the LESSOPEN string, first "|", then "-|", now "||" and I suppose "-||" is also possible.  But it's not bad.  If we can find a way to solve the /dev/null issue, it would be fine.

--Mark
> 
> See the following link for the whole Fedora changeset:
> http://pkgs.fedoraproject.org/gitweb/?p=less.git;a=blob;f=less-436-empty-lessopen-pipe.patch;h=69f84ca1287c55e5d948d0be3763797941fc3523;hb=HEAD
> 
> I think it would be great if we could unify this "new" exit code behaviour in the similar manner,
> as it is backward-compatible with old scripts, and also it provides the "correct" behaviour.
> 
> So if you agreed with similar solution, we would have to deal just with /dev/null issue.
>  
> - Vojtech
> >
> > --Mark

Upstream won't merge the changes until we have the "/dev/null" solution fixed for non-unix systems. Still Cond NACK UPSTREAM..

This request was evaluated by Red Hat Product Management for
inclusion in the current release of Red Hat Enterprise Linux.
Because the affected component is not scheduled to be updated
in the current release, Red Hat is unfortunately unable to
address this request at this time. Red Hat invites you to
ask your support representative to propose this request, if
appropriate and relevant, in the next release of Red Hat
Enterprise Linux. If you would like it considered as an
exception in the current release, please ask your support
representative.

The component has changed its owner to me, so I'm assigning the bug to myself.

I wrote a portable patch and sent it to upstream to check it. Mark Nudelmann (the author) replied on Friday:

>Hi Martin,
>Thank you for the patch.  I have actually implemented this feature in a
>different way, using a special filename similar to the way FAKE_HELPFILE
>is used.  It's kludgy, but at least it has precedent.
>
>I'll release this in a beta soon.
>
>--Mark
>
>On 6/13/2012 6:56 AM, Martin Briza wrote:
>> Hello,
>>
>> I'm the new maintainer of less in RHEL and Fedora and Vojtech told me
>> about your communication regarding empty files as output from input
>> preprocessor.
>> I think I solved the portability problem with opening /dev/null to get
>> empty output to print with a small hack - opening the input file again
>> (as it should still be there to be read) and seeking to its end to get
>> EOF immediately.
>> There is the patch (based upon the one Vojtech has sent you) attached to
>> this mail.
>> I know it's not a clean solution but I'm afraid there isn't much more
>> options. Anyway, I'll understand perfectly if you refuse it.
>>
>> Thanks in advance,
>>
>> Martin Briza

I'm absolutely sure his implementation is better so I will wait until he releases his version and propose merging it into our tree.



-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Created attachment 694089 [details]
Patch extracted from upstream less to fix the issue

This issue can be fixed with this upstream fix (extracted from less-451 as the author releases only the full package).
Fixing this will also need modifying less.csh and less.sh to set the LESSOPEN variable to contain "||" instead of "|" and change lesspipe.sh to attachment 514738 [details].

It seems, the attached patches wasn't including fixes for rhbz #718498.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2014-0755.html