On Thu, Oct 22, 2020 at 01:18:11AM +0000, brian m. carlson wrote:
> The commit 9ab33150a0 ("perl: create and switch variables for hash
> constants", 2020-06-22) converted each instance of the variable
> $sha1_short into $oid_short in the Subversion code, since git-svn now
> understands SHA-256. However, one conversion was missed.
>
> As a result, Perl complains about the use of this variable:
>
> Use of uninitialized value $sha1_short in regexp compilation at
> /usr/lib64/perl5/vendor_perl/5.30.3/Git/SVN/Log.pm line 301, <$fh>
> line 6.
>
> Because we're parsing raw diff output here, the likelihood is very low
> that we'll actually misparse the data, since the only lines we're going
> to get starting with colons are the ones we're expecting. Even if we
> had a newline in a path, we'd end up with a quoted path. Our regex is
> just less strict than we'd like it to be.
I agree this is unlikely to matter much in the happy path, but I
wondered how confused things could get. I'd never looked at this code
before, but it looks like we take git-log @args from the user. So:
git svn log --format=":123456 123456 foo"
gets mis-parsed. But not only is that exceedingly unlikely in the first
place, AFAICT the command was never meant to allow arbitrary formats
anyway. It's expecting its own "--pretty=raw" to be respected, so the
command above is broken even with your fix.
None of that changes the fix, which is obviously correct, but I wondered
if we ought to have better test coverage here. And I've convinced myself
the answer is "no"; there's no reasonable-to-test functional impact of
this bug or its fix (aside from generating the warning, but it would be
silly to write a test for this one warning; if we do anything it should
be to complain about any warnings during the test run).
-Peff
