Re: git svn log: Use of uninitialized value $sha1_short

From: Jeff King <peff@peff.net>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Nikos Chantziaras <realnc@gmail.com>,
	git@vger.kernel.org
Subject: Re: git svn log: Use of uninitialized value $sha1_short
Date: Wed, 21 Oct 2020 22:56:44 -0400	[thread overview]
Message-ID: <20201022025644.GA1480820@coredump.intra.peff.net> (raw)
In-Reply-To: <20201021222901.GK490427@camp.crustytoothpaste.net>

On Wed, Oct 21, 2020 at 10:29:01PM +0000, brian m. carlson wrote:

> >   - I wonder if it's suitable for production use (i.e., would it become
> >     annoying when a newer version of perl issues a harmless warning;
> >     right now that's a minor inconvenience, but aborting the whole
> >     program might be a show-stopper).
> 
> No, that's not suitable for production use.  Perl does add new warnings
> from time to time and breaking things when Perl gets upgraded will
> definitely not make us the friends of Linux distros.  Doing this is like
> using -Werror: fine for your personal development needs, but not
> suitable for shipping to others.

OK, that matches my general sense.

> We could run "perl -w" on each file and look for a single-line output
> with "OK"; that's what we did at a previous job.  However, any change we
> make here needs to be conditional on DEVELOPER, because otherwise anyone
> who needs to build an Git with a new version of Perl will potentially
> have a broken testsuite.

Yeah, I've done something similar as well. I agree it would be
potentially annoying for distros building. But unlike the -Wall/-Werror
distinction, where the builder may be annoyed by extra messages but the
end result is presumably OK, this _does_ affect end users. I.e., if it
triggers, it also means the users of your package are going to see
annoying warnings. The change I'm proposing is just whether that's fatal
or not.

So it might actually be of interest to distro builders to know that the
version of Git they're building is going to produce annoying warnings.
And the escape hatch there is likely not to turn warnings from fatal to
non-fatal, but to suppress the particular warning entirely with a patch
to the code.

I dunno. I admit I don't really care enough about our perl code in
general (which I consider mostly legacy; I doubt we'd introduce new perl
scripts lightly). So it may not even be worth putting a lot of effort
into it. But it is unfortunate that this bug _could_ have been caught
automatically but wasn't.

-Peff