Re: [PATCH] git-send-email: fix get_maintainer.pl regression

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 12, 2017, 21:25 UTC
Message-ID: <87d13jd4fd.fsf@evledraar.gmail.com>
In-Reply-To: <xmqqwp1r20zx.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 12 2017, Junio C. Hamano jotted:

Show 19 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> My "Makefile: replace perl/Makefile.PL with simple make rules" currently
>> cooking in pu changes that so that:
>>
>>  * We always at runtime test for the system CPAN module.
>>
>>  * In the case of Error.pm we happen to ship a fallback, in the case of
>>    Mail::Address etc. we don't and have fallback code, but we could also
>>    just ship a copy and remove the fallback code.
>>
>> This makes more sense, we always "dynamically link" as it were, we'll
>> just change the target to (a presumably newer) system module in the case
>> of Error.pm if it's found on the system, otherwise use our fallback.
>
> "When to fallback" aside, I think the above makes sense for the
> send-email simply because we would be replacing "our own" fallback
> we may need to maintain forever with something with an upstream that
> we do not have to worry too much about.

I'll see about submitting something that replaces the fallback with just using the CPAN modules + bundling them once the Makefile patch has cooked to master.

> A tangent; I thought I heard that use of Error.pm is strongly
> discouraged several years ago---am I mistaken, or if I am not,
> perhaps we should start looking into updating the users?

I'm not a fan of it, 41c01693ac ("git-svn: handle merge-base failures", 2010-01-06) shows how you can do that rather simply with just perl's built-in exceptions.

My TODO list of "perl stuff in git" is now:

 - Get my Makefile.PL thing through
 - Make sure Dan Jacques's relocatable stuff is OK wrt perl on top of that
 - Upgrade the required version from 5.8 to 5.10
 - Update Error.pm itself, our copy is ancient
 - Add more stuff to Git::FromCPAN + remove fallbacks

I could add "rip out Error.pm" to that, it looks rather easy, however given previous discussion about me needing to build a manpage from Git.pm I understand that Git.pm is used by code outside of Git itself.

Ripping out Error.pm for our few internal callers is one thing, trying to maintain bugwards compatibility with how it throws exceptions for users expecting Error.pm objects is another. I think at that point it's easier to just stay with Error.pm.

Probably easier to stay with it either way, don't poke sleeping dragons and all that, it's working code, even if we wouldn't write it like that today the churn probably isn't worth it.