[PATCH] searchidx: avoid modification of read-only `$_'

[Eric Wong]

Jörg Rödel <j...@8bytes.org> wrote:
> [   92s] 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/t/data-gen/reindex-time-range.v1-master
>  index failed: Modification of a read-only value attempted at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/SearchIdx.pm
>  line 900, <$r> line 1.
> [   92s]  at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/SearchIdx.pm
>  line 942, <$r> line 1.
> [   92s]      PublicInbox::SearchIdx::prepare_stack(HASH(0x564bfd2f7678), 
> "refs/heads/master") called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/SearchIdx.pm
>  line 1038
> [   92s]      
> PublicInbox::SearchIdx::_index_sync(PublicInbox::SearchIdx=HASH(0x564bfcf5be78),
>  HASH(0x564bfd3f7bc0)) called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/InboxWritable.pm
>  line 224
> [   92s]      eval {...} called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/InboxWritable.pm
>  line 224
> [   92s]      
> PublicInbox::InboxWritable::with_umask(PublicInbox::InboxWritable=HASH(0x564bfcccbb10),
>  CODE(0x564bfd49d0f8), PublicInbox::SearchIdx=HASH(0x564bfcf5be78), 
> HASH(0x564bfd3f7bc0)) called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/SearchIdx.pm
>  line 760
> [   92s]      
> PublicInbox::SearchIdx::with_umask(PublicInbox::SearchIdx=HASH(0x564bfcf5be78),
>  CODE(0x564bfd49d0f8), PublicInbox::SearchIdx=HASH(0x564bfcf5be78), 
> HASH(0x564bfd3f7bc0)) called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/SearchIdx.pm
>  line 767
> [   92s]      
> PublicInbox::SearchIdx::index_sync(PublicInbox::SearchIdx=HASH(0x564bfcf5be78),
>  HASH(0x564bfd3f7bc0)) called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/Import.pm 
> line 194
> [   92s]      eval {...} called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/Import.pm 
> line 192
> [   92s]      
> PublicInbox::Import::_update_git_info(PublicInbox::Import=HASH(0x564bfccbd5f8),
>  1) called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/Import.pm 
> line 494
> [   92s]      eval {...} called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/Import.pm 
> line 494
> [   92s]      
> PublicInbox::Import::done(PublicInbox::Import=HASH(0x564bfccbd5f8)) called at 
> /home/abuild/rpmbuild/BUILD/public-inbox-1.7.0/blib/lib/PublicInbox/TestCommon.pm
>  line 707
> [   92s]      PublicInbox::TestCommon::create_inbox("v1", "version", 1, 
> "indexlevel", "basic", "tmpdir", "/tmp/pi-reindex-time-range-22646-lChw/v1") 
> called at t/reindex-time-range.t line 18

Aha, so it was the $_ from the map {...} block in t/reindex-time-range.t
which only gets called in the initial run due to caching.  I totally
forgot about that :x  Thanks for that backtrace, this should fix it:

---------8<---------
Subject: [PATCH] searchidx: avoid modification of read-only `$_'

This fixes the "Modification of a read-only value attempted at ..."
error in an initial run of t/reindex-time-range.t.  It was
reproducible by running `rm -rf t/data-gen/reindex-time-range.v*'
before `make && prove -bvw t/reindex-time-range.t'.  Thanks to
Jörg Rödel for providing the backtrace which helped find this.

Debugged-by: Jörg Rödel <j...@8bytes.org>
Link: https://public-inbox.org/meta/yzuzey+wsnm4w...@8bytes.org/
---
 lib/PublicInbox/SearchIdx.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index 6e2e614c..4e5d7d44 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -896,20 +896,20 @@ sub log2stack ($$$) {
                push @cmd, "--$k=$v";
        }
        my $fh = $git->popen(@cmd, $range);
-       my ($at, $ct, $stk, $cmt);
-       while (<$fh>) {
+       my ($at, $ct, $stk, $cmt, $l);
+       while (defined($l = <$fh>)) {
                return if $sync->{quit};
-               if (/\A([0-9]+)-([0-9]+)-($OID)$/o) {
+               if ($l =~ /\A([0-9]+)-([0-9]+)-($OID)$/o) {
                        ($at, $ct, $cmt) = ($1 + 0, $2 + 0, $3);
                        $stk //= PublicInbox::IdxStack->new($cmt);
-               } elsif (/$del/) {
+               } elsif ($l =~ /$del/) {
                        my $oid = $1;
                        if ($D) { # reindex case
                                $D->{pack('H*', $oid)}++;
                        } else { # non-reindex case:
                                $stk->push_rec('d', $at, $ct, $oid, $cmt);
                        }
-               } elsif (/$add/) {
+               } elsif ($l =~ /$add/) {
                        my $oid = $1;
                        if ($D) {
                                my $oid_bin = pack('H*', $oid);
--
unsubscribe: one-click, see List-Unsubscribe header
archive: https://public-inbox.org/meta/