From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@80x24.org>
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net
X-Spam-Level: 
X-Spam-ASN:  
X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00
	shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2
Received: from localhost (dcvr.yhbt.net [127.0.0.1])
	by dcvr.yhbt.net (Postfix) with ESMTP id 2A5011F9F4;
	Mon, 22 Nov 2021 18:23:52 +0000 (UTC)
Date: Mon, 22 Nov 2021 18:23:52 +0000
From: Eric Wong <e@80x24.org>
To: =?utf-8?B?SsO2cmcgUsO2ZGVs?= <joro@8bytes.org>
Cc: meta@public-inbox.org
Subject: [PATCH] searchidx: avoid modification of read-only `$_'
Message-ID: <20211122182352.GA16963@dcvr>
References: <YZebmAxlFJy4lqAw@8bytes.org>
 <20211119185411.M766819@dcvr>
 <YZgbMSUTSayOijaz@8bytes.org>
 <20211122065545.GA31379@dcvr>
 <YZuZEY+WSnm4wlrS@8bytes.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <YZuZEY+WSnm4wlrS@8bytes.org>
List-Id: <meta.public-inbox.org>

Jörg Rödel <joro@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 <joro@8bytes.org>
Link: https://public-inbox.org/meta/YZuZEY+WSnm4wlrS@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);