| Message ID | 20200622180418.2418483-15-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SHA-256 CVS and SVN patches | expand |
On Jun 22 2020, brian m. carlson wrote: > diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl > index fc00d5946a..6483d792d3 100755 > --- a/git-cvsexportcommit.perl > +++ b/git-cvsexportcommit.perl > @@ -22,6 +22,10 @@ > my $repo = Git->repository(); > $opt_w = $repo->config('cvsexportcommit.cvsdir') unless defined $opt_w; > > +my $tmpdir = File::Temp->newdir; File::Temp in perl 5.10 doesn't have the newdir method. Andreas.
On 2020-08-03 at 18:37:27, Andreas Schwab wrote: > On Jun 22 2020, brian m. carlson wrote: > > > diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl > > index fc00d5946a..6483d792d3 100755 > > --- a/git-cvsexportcommit.perl > > +++ b/git-cvsexportcommit.perl > > @@ -22,6 +22,10 @@ > > my $repo = Git->repository(); > > $opt_w = $repo->config('cvsexportcommit.cvsdir') unless defined $opt_w; > > > > +my $tmpdir = File::Temp->newdir; > > File::Temp in perl 5.10 doesn't have the newdir method. That method was added in File::Temp 0.19, which was added in 2007. Does my $tmpdir = File::Temp::tempdir(CLEANUP => 1); do the right thing on your Perl 5.10? I no longer use CentOS 6 at work and it won't run in Docker on modern Debian, so I can't test. If so, I'll send a patch. For the record, I plan to propose that we drop support for Perl versions earlier than 5.14 on December 2, since CentOS 6 will be dead at that point. I think a ten-year lifespan for an OS is quite generous and we're still considering Perl 5.8.8, which nobody is publicly supporting anymore.
"brian m. carlson" <sandals@crustytoothpaste.net> wrote: > On 2020-08-03 at 18:37:27, Andreas Schwab wrote: > > On Jun 22 2020, brian m. carlson wrote: > > > > > diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl > > > index fc00d5946a..6483d792d3 100755 > > > --- a/git-cvsexportcommit.perl > > > +++ b/git-cvsexportcommit.perl > > > @@ -22,6 +22,10 @@ > > > my $repo = Git->repository(); > > > $opt_w = $repo->config('cvsexportcommit.cvsdir') unless defined $opt_w; > > > > > > +my $tmpdir = File::Temp->newdir; > > > > File::Temp in perl 5.10 doesn't have the newdir method. > > That method was added in File::Temp 0.19, which was added in 2007. Does 5.10.0 doesn't have ->newdir, but 5.10.1 does. I figure nobody used 5.10.0 anymore since 5.10.1 exists and (IIRC) fixed many things wrong in 5.10.0. > my $tmpdir = File::Temp::tempdir(CLEANUP => 1); > > do the right thing on your Perl 5.10? I no longer use CentOS 6 at work > and it won't run in Docker on modern Debian, so I can't test. If so, > I'll send a patch. I haven't touched CentOS 6 in several years, either. I've been testing public-inbox against 5.10.1 built using patches from Devel::PatchPerl (libdevel-patchperl-perl in Debian) > For the record, I plan to propose that we drop support for Perl versions > earlier than 5.14 on December 2, since CentOS 6 will be dead at that > point. I think a ten-year lifespan for an OS is quite generous and > we're still considering Perl 5.8.8, which nobody is publicly supporting > anymore. That's probably fine. I haven't looked at 5.12 and 5.14 changes enough to comment, but just moving rom 5.8 to 5.10.1 last year made my life considerably better (e.g. `//', `//=') In any case, it's probably a good idea to put use "v5.$SOMETHING" with Perl 7 on the way (and Perl 8 will probably break v5 code :<)
On 2020-08-04 at 00:13:26, Eric Wong wrote: > "brian m. carlson" <sandals@crustytoothpaste.net> wrote: > > On 2020-08-03 at 18:37:27, Andreas Schwab wrote: > > > On Jun 22 2020, brian m. carlson wrote: > > > > > > > diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl > > > > index fc00d5946a..6483d792d3 100755 > > > > --- a/git-cvsexportcommit.perl > > > > +++ b/git-cvsexportcommit.perl > > > > @@ -22,6 +22,10 @@ > > > > my $repo = Git->repository(); > > > > $opt_w = $repo->config('cvsexportcommit.cvsdir') unless defined $opt_w; > > > > > > > > +my $tmpdir = File::Temp->newdir; > > > > > > File::Temp in perl 5.10 doesn't have the newdir method. > > > > That method was added in File::Temp 0.19, which was added in 2007. Does > > 5.10.0 doesn't have ->newdir, but 5.10.1 does. I figure nobody > used 5.10.0 anymore since 5.10.1 exists and (IIRC) fixed many > things wrong in 5.10.0. Technically we support 5.8.8, I believe, which means this probably does need to be fixed. I'll let Andreas verify the proposed solution works for him, and then send a patch. > > For the record, I plan to propose that we drop support for Perl versions > > earlier than 5.14 on December 2, since CentOS 6 will be dead at that > > point. I think a ten-year lifespan for an OS is quite generous and > > we're still considering Perl 5.8.8, which nobody is publicly supporting > > anymore. > > That's probably fine. I haven't looked at 5.12 and 5.14 changes > enough to comment, but just moving rom 5.8 to 5.10.1 last year > made my life considerably better (e.g. `//', `//=') 5.14 has nice things like s///r: # 5.12 # We have to make a copy since s/// modifies its left side. my $foo = $ARGV[0]; $foo =~ s/a/b/; # 5.14 # Now we have non-destructive substitution. my $foo = $ARGV[0] =~ s/a/b/r;
On Aug 03 2020, brian m. carlson wrote: > On 2020-08-03 at 18:37:27, Andreas Schwab wrote: >> On Jun 22 2020, brian m. carlson wrote: >> >> > diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl >> > index fc00d5946a..6483d792d3 100755 >> > --- a/git-cvsexportcommit.perl >> > +++ b/git-cvsexportcommit.perl >> > @@ -22,6 +22,10 @@ >> > my $repo = Git->repository(); >> > $opt_w = $repo->config('cvsexportcommit.cvsdir') unless defined $opt_w; >> > >> > +my $tmpdir = File::Temp->newdir; >> >> File::Temp in perl 5.10 doesn't have the newdir method. > > That method was added in File::Temp 0.19, which was added in 2007. Does > > my $tmpdir = File::Temp::tempdir(CLEANUP => 1); > > do the right thing on your Perl 5.10? With that change, the testsuite runs sucessfully. Andreas.
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl index fc00d5946a..6483d792d3 100755 --- a/git-cvsexportcommit.perl +++ b/git-cvsexportcommit.perl @@ -22,6 +22,10 @@ my $repo = Git->repository(); $opt_w = $repo->config('cvsexportcommit.cvsdir') unless defined $opt_w; +my $tmpdir = File::Temp->newdir; +my $hash_algo = $repo->config('extensions.objectformat') || 'sha1'; +my $hexsz = $hash_algo eq 'sha256' ? 64 : 40; + if ($opt_w || $opt_W) { # Remember where GIT_DIR is before changing to CVS checkout unless ($ENV{GIT_DIR}) { @@ -96,7 +100,7 @@ } if ($stage eq 'headers') { - if ($line =~ m/^parent (\w{40})$/) { # found a parent + if ($line =~ m/^parent ([0-9a-f]{$hexsz})$/) { # found a parent push @parents, $1; } elsif ($line =~ m/^author (.+) \d+ [-+]\d+$/) { $author = $1; @@ -111,7 +115,7 @@ } } -my $noparent = "0000000000000000000000000000000000000000"; +my $noparent = "0" x $hexsz; if ($parent) { my $found; # double check that it's a valid parent @@ -174,7 +178,7 @@ print "Checking if patch will apply\n"; my @stat; -open APPLY, "GIT_DIR= git-apply $context --summary --numstat<.cvsexportcommit.diff|" || die "cannot patch"; +open APPLY, "GIT_INDEX_FILE=$tmpdir/index git-apply $context --summary --numstat<.cvsexportcommit.diff|" || die "cannot patch"; @stat=<APPLY>; close APPLY || die "Cannot patch"; my (@bfiles,@files,@afiles,@dfiles); @@ -329,7 +333,7 @@ if ($opt_W) { system("git checkout -q $commit^0") && die "cannot patch"; } else { - `GIT_DIR= git-apply $context --summary --numstat --apply <.cvsexportcommit.diff` || die "cannot patch"; + `GIT_INDEX_FILE=$tmpdir/index git-apply $context --summary --numstat --apply <.cvsexportcommit.diff` || die "cannot patch"; } print "Patch applied successfully. Adding new files and directories to CVS\n"; @@ -407,7 +411,7 @@ if ($opt_W) { system("git checkout $go_back_to") && die "cannot move back to $go_back_to"; - if (!($go_back_to =~ /^[0-9a-fA-F]{40}$/)) { + if (!($go_back_to =~ /^[0-9a-fA-F]{$hexsz}$/)) { system("git symbolic-ref HEAD $go_back_to") && die "cannot move back to $go_back_to"; }
When we apply a binary patch, we must have the full object ID in the header in order to apply it; without that, any attempt to apply it will fail. If we set GIT_DIR to empty, git apply does not know about the hash algorithm we're using, and consequently any attempt to apply a patch using SHA-256 will fail, since the object ID is the wrong length. The reason we set the GIT_DIR environment variable is because we don't want to modify the index; we just want to know whether the patch applies. Instead, let's just use a temporary file for the index, which will be cleaned up automatically when the object goes out of scope. Additionally, read the configuration for the repository and compute the length of an object ID based on it. Use that when matching object IDs with a regex or computing the all-zeros object ID. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- git-cvsexportcommit.perl | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)