BBC: Blead Breaks File::Replace, Log::Log4perl

[cjg-cguevara]

This is a bug report for perl from "Carlos Guevara" [email protected], generated with the help of perlbug 1.42 running under perl 5.37.4.

---

[Please describe your issue here]

BBC: Blead Breaks File::Replace

Please see http://matrix.cpantesters.org/?dist=File::Replace

[Please do not change anything below this line]

---

Flags: category=core severity=low

Site configuration information for perl 5.37.4:

Configured by cpan at Wed Aug 31 13:05:00 EDT 2022.

Summary of my perl5 (revision 5 version 37 subversion 4) configuration: Commit id: e4bbbfe02b9e9aae521b164eba0e518ca478945f Platform: osname=openbsd osvers=7.1 archname=OpenBSD.amd64-openbsd-thread-multi uname='openbsd cjg-openbsd7 7.1 generic#3 amd64 ' config_args='-des -Dprefix=~/bin/perl-blead -Dscriptdir=~/bin/perl-blead/bin -Dusedevel -Duse64bitall -Duseithreads' hint=recommended useposix=true d_sigaction=define useithreads=define usemultiplicity=define use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define Compiler: cc='cc' ccflags ='-pthread -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' optimize='-O2' cppflags='-pthread -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='OpenBSD Clang 13.0.0' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='cc' ldflags ='-pthread -Wl,-E -fstack-protector-strong -L/usr/local/lib' libpth=/usr/lib/clang/13.0.0/lib /usr/lib /usr/local/lib libs=-lpthread -lm -lutil -lc perllibs=-lpthread -lm -lutil -lc libc=/usr/lib/libc.so.96.1 so=so useshrplib=false libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags=' ' cccdlflags='-DPIC -fPIC ' lddlflags='-shared -fPIC -L/usr/local/lib -fstack-protector-strong'

---

@INC for perl 5.37.4: /home/cpan/bin/perl-blead/lib/site_perl/5.37.4/OpenBSD.amd64-openbsd-thread-multi /home/cpan/bin/perl-blead/lib/site_perl/5.37.4 /home/cpan/bin/perl-blead/lib/5.37.4/OpenBSD.amd64-openbsd-thread-multi /home/cpan/bin/perl-blead/lib/5.37.4

---

Environment for perl 5.37.4: HOME=/home/cpan LANG (unset) LANGUAGE (unset) LC_ALL=C LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/cpan/bin/perl-blead/bin:/home/cpan/bin:/home/cpan/bin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/bin:/usr/local/sbin:/usr/games PERL_BADLANG (unset) SHELL=/usr/local/bin/bash

[jkeenan]

Bisection points to 80c1f1e45e8ef8c27d170fae7ade41971fe20218.

80c1f1e45e8ef8c27d170fae7ade41971fe20218 is the first bad commit
commit 80c1f1e45e8ef8c27d170fae7ade41971fe20218
Author: Tony Cook <[email protected]>
Date:   Tue Aug 16 15:52:04 2022 +1000
Commit:     Tony Cook <[email protected]>
CommitDate: Wed Aug 31 10:51:09 2022 +1000

    only clear the stream error state in readline() for glob()
...

@tonycoz, can you take a look?

[jkeenan]

Also affected by this commit: Log::Log4perl (as originally reported in https://github.com/Perl/perl5/issues/20208).

[bram-perl]

Reducing t/50_inplace.t of File::Replace to a minimal test case:

#!/usr/bin/perl -l

use strict;
use warnings;

use POSIX qw/dup dup2/;


sub s1 {
    print "\teof() is " . (eof() ? "true" : "false");
    while (<>) {
        chomp;
        print "\t<>: $ARGV: $.: $_";
    }
}

print "First call";
{
    open my $fh, "<", "foobar1" or die "Failed to open foobar1: $!";
    my $saved_fd = dup(0);
    dup2(fileno $fh, 0) or die "dup2 failed: $!";

    s1();

    dup2( $saved_fd, 0 ) or die "restore dup2: $!";
    POSIX::close( $saved_fd) or die "close saved: $!";
}

print "Second call";
{
    open my $fh, "<", "foobar2" or die "Failed to open foobar2: $!";
    my $saved_fd = dup(0);
    dup2(fileno $fh, 0) or die "dup2 failed: $!";

    s1();

    dup2( $saved_fd, 0 ) or die "restore dup2: $!";
    POSIX::close( $saved_fd) or die "close saved: $!";
}

Before running create the necessary files:

$ echo "BBBB: foobar :BBBB" > foobar2
$ echo "AAAA: foobar :AAAA" > foobar1

Running with 80c1f1e45e8ef8c27d170fae7ade41971fe20218:

$ ./perl -Ilib gh-20207.pl
First call
        eof() is false
        <>: -: 1: AAAA: foobar :AAAA
Second call
        eof() is true

Running with 80c1f1e45e8ef8c27d170fae7ade41971fe20218~1:

$ ./perl -Ilib gh-20207.pl
First call
        eof() is false
        <>: -: 1: AAAA: foobar :AAAA
Second call
        eof() is false
        <>: -: 1: BBBB: foobar :BBBB

With the old code 'eof' of STDIN was cleared, with the new code it no longer is.

[tonycoz]

Reducing t/50_inplace.t of File::Replace to a minimal test case:

I see what's happening, but I think it's case of the test depending on the now fixed bug.

We read to end of file on *STDIN and the eof flag is set on PerlIO_stdin(), the underlying fd is modified to point at another file which isn't at end of file. With the old code clearerr() is called once we get to eof() (the bug) which also clears eof, but with the fix that no longer happens.

We could modify readline() to clear eof() rather than calling clearerr() (which clears both error and eof state), but I don't think it would be correct behaviour.

[andk]

Also affected: WHYNOT/File-AptFetch-v0.1.14.tar.gz Sample report: http://www.cpantesters.org/cpan/report/caa6bb86-3b0e-11ed-a22f-a51cc21cab27

[tonycoz]

Also affected: WHYNOT/File-AptFetch-v0.1.14.tar.gz

Patch attached to ticket at https://rt.cpan.org/Ticket/Display.html?id=144442

[andk]

Also affected:

• JANPAZ/Logfile-Read-0.6.tar.gz http://www.cpantesters.org/cpan/report/8ef83580-3f21-11ed-8a39-7b29e48453bc
• TOBEYA/Nagios-Cmd-0.05.tar.gz http://www.cpantesters.org/cpan/report/ded30156-3ec3-11ed-84dd-5990dd8453bc

[andk]

Also affected: BOOK/Log-Procmail-0.12.tar.gz http://www.cpantesters.org/cpan/report/ef21d450-40c2-11ed-b3c4-fccdf10bed05

[tonycoz]

Logfile::Read patch at https://rt.cpan.org/Ticket/Display.html?id=144592

[tonycoz]

Nagios::Cmd patch at https://rt.cpan.org/Ticket/Display.html?id=144593

[tonycoz]

Log::Procmail patch at https://github.com/book/Log-Procmail/pull/2

[jkeenan]

Log-Log4perl-1.57 and Log-Procmail-0.14 have been released and are installable against v5.37.5. However, the other modules cited in this ticket are still failing.

[jkeenan]

File-AptFetch and Logfile-Read are still showing failing tests. Nagios-Cmd is showing a mixture of PASSes and FAILes.

[haukex]

I just came across this issue. Test failures like this and this appear to me to indicate that eof() on tied filehandles (ARGV, specifically), has stopped working, which worked since 5.12. Test cases like this failing one are designed to test the behavior normal ARGV against a tied ARGV to make sure they match, so at first glance this seems like a regression to me.

[demerphq]

Im finding it quite difficult to decode that test code. Can you provide a simple reproduction case @haukex ?

[haukex]

Yes, it's indeed quite a complex test case, I'm still working on seeing if I can simplify it myself.

[tonycoz]

The patch here only modifies the PerlIO state, it shouldn't have any effect on the behaviour of a tied EOF/READLINE implementation.

If the test is comparing the two that might result in a failure, since eof()/error() untied now returns true in cases where it didn't before.

[haukex]

Test script is now available here: https://github.com/haukex/File-Replace/tree/issue20207

This is the most minimal test script I was able to produce from my code. A run of Porting/bisect.pl Porting/bisect.pl --start=v5.36.0 --end=v5.37.5 points to 80c1f1e45e8ef8c27d170fae7ade41971fe20218, the same commit as noted near the top of this thread. I haven't looked into what this commit is doing yet.

By the way, I didn't get any kind of notification of this issue from GitHub or per email or anything, I had to find it myself after noticing the CPAN Testers results. It would of course be nice if there was some kind of notification system :-)

[haukex]

By the way, I have decided to break the modules File::Replace::Inplace and Tie::Handle::Argv out of the File-Replace distribution. I use File::Replace all the time, while the other two are "nice to have" and are kind of a testing and maintenance nightmare, as this thread shows (plus #16904). Once I've done so, is there a way to exclude the upcoming File-Replace-Inplace distro from Blead Breaks CPAN checks?

[haukex]

I have decided to break the modules File::Replace::Inplace and Tie::Handle::Argv out of the File-Replace distribution. I use File::Replace all the time, while the other two are "nice to have" and are kind of a testing and maintenance nightmare, as this thread shows (plus #16904). Once I've done so, is there a way to exclude the upcoming File-Replace-Inplace distro from Blead Breaks CPAN checks?

I've now done this and split File-Replace-0.14 into four distros:

• Tie-Handle-Base-0.16
• File-Replace-0.16
• File-Replace-Fancy-0.16
• File-Replace-Inplace-0.16 - This is the problematic distro that caused the failure currently under discussion here. AFAICT, the tests still fail on the current bleadperl. I've documented that the modules in it are experimental. This distro can be excluded from further Blead Breaks CPAN checks.

The first three modules should work on any Perl version from 5.8.1 up to bleadperl and should still be included in Blead Breaks CPAN.

[jkeenan]

I have decided to break the modules File::Replace::Inplace and Tie::Handle::Argv out of the File-Replace distribution. I use File::Replace all the time, while the other two are "nice to have" and are kind of a testing and maintenance nightmare, as this thread shows (plus #16904). Once I've done so, is there a way to exclude the upcoming File-Replace-Inplace distro from Blead Breaks CPAN checks?

I've now done this and split File-Replace-0.14 into four distros:

* [Tie-Handle-Base-0.16](https://metacpan.org/release/HAUKEX/Tie-Handle-Base-0.16)

* [File-Replace-0.16](https://metacpan.org/release/HAUKEX/File-Replace-0.16)

* [File-Replace-Fancy-0.16](https://metacpan.org/release/HAUKEX/File-Replace-Fancy-0.16)

* [File-Replace-Inplace-0.16](https://metacpan.org/release/HAUKEX/File-Replace-Inplace-0.16) - This is the problematic distro that caused the failure currently under discussion here. AFAICT, the tests still fail on the current bleadperl. I've documented that the modules in it are experimental. This distro can be excluded from further Blead Breaks CPAN checks.

The first three modules should work on any Perl version from 5.8.1 up to bleadperl and should still be included in Blead Breaks CPAN.

That's a really nice piece of module maintenance! I was able to get passing tests on blead for 3 of the 4 distros, failing only, as expected, on File-Replace-Inplace: http://www.cpantesters.org/cpan/report/82014152-975d-11ed-8658-064aaaff8ba7

Given your work I am going to change the Subject line of this BBC ticket to reflect the distros which are still broken on CPAN: File-AptFetch and Logfile-Read.

[haukex]

@jkeenan Thanks!

@tonycoz and @demerphq If you happen to get the chance, I'd still be interested on your opinion whether the test script (https://github.com/haukex/File-Replace/tree/issue20207) is exposing a bug in the Perl commit (80c1f1e45e8ef8c27d170fae7ade41971fe20218), or if it's actually my script that is incorrect.

[tonycoz]

@jkeenan Thanks!

@tonycoz and @demerphq If you happen to get the chance, I'd still be interested on your opinion whether the test script (https://github.com/haukex/File-Replace/tree/issue20207) is exposing a bug in the Perl commit (80c1f1e), or if it's actually my script that is incorrect.

For your test case, the eof flag is still set on the PerlIO stdin handle when you try to read from it again.

If you remove the "non-tied" test case the tied test case passes.

If you clearerr() STDIN, all of the tests pass.

[demerphq]

@haukex you still need me on this? I somehow think there isnt much i can add to what @tonycoz has said.

[haukex]

@tonycoz and @demerphq Thank you both! A simple STDIN->clearerr; after restoring STDIN in the test seems to fix the issue, both in my test case and in the actual module.

[rjbs]

It is not clear to me whether we think there is a bug or blocker here. Maybe somebody would like to chime in on that? @tonycoz I'm looking your way…

[tonycoz]

So far each case has been assuming the bug, ie that eof (or an error in some cases) wasn't set when we read to end-of-file with readline. Two cases were for and error rather than eof, where readline was used on a non-blocking handle.

I've supplied patches to the dists for most of the cases (haukex fixed his own code), but some haven't been applied yet.

I don't think it's neither a perl bug nor a blocker.