Re: [PATCH] Makefile: replace perl/Makefile.PL with simple make rules
- Date: Thu, 30 Nov 2017 16:31:05 -0500
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH] Makefile: replace perl/Makefile.PL with simple make rules
On Wed, Nov 29, 2017 at 07:54:30PM +0000, Ævar Arnfjörð Bjarmason wrote:
> Replace the perl/Makefile.PL and the fallback perl/Makefile used under
> NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily
> inspired by how the i18n infrastructure's build process works.
I'm very happy to see the recursive make invocation go away. The perl
makefile generation was one of the few places where parallel make could
racily get confused (though I haven't seen that for a while, so maybe it
was fixed alongside some of the other .stamp work you did).
> The reason for having the Makefile.PL in the first place is that it
> was initially building a perl C binding to interface with libgit,
> this functionality, that was removed before Git.pm ever made it to
> the master branch.
Thanks for doing all this history digging. I agree that it doesn't seem
like there's really any reason to carry the complexity. Of your
functional changes, the only one that gives me pause is:
> * This will not always install into perl's idea of its global
> "installsitelib". This only potentially matters for packagers that
> need to expose Git.pm for non-git use, and as explained in the
> INSTALL file there's a trivial workaround.
This could be a minor hiccup for people using Git.pm from other scripts.
But maybe only in funny setups? It seems like $prefix/share/perl5 would
be in most people's @INC unless they are doing something exotic.
> * We don't build the Git(3) Git::I18N(3) etc. man pages from the
> embedded perldoc. I suspect nobody really cares, these are mostly
> internal APIs, and if someone's developing against them they likely
> know enough to issue a "perldoc" against the installed file to get
> the same result.
I don't have a real opinion on this, but it sounds from the rest of the
thread like we should maybe build these to be on the safe side.
> @@ -2291,6 +2273,17 @@ endif
> po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
> $(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
> +PMFILES := $(wildcard perl/*.pm perl/*/*.pm perl/*/*/*.pm perl/*/*/*/*.pm)
Yuck. :) I don't think there's a better wildcard solution within make,
though. And I'd rather see this than doing a $(shell) to "find" or
The other option is to actually list the files, as we do for .o files.
That's a minor pain to update, but it would allow things like
differentiating which ones get their documentation built.
> +PMCFILES := $(patsubst perl/%.pm,perl/build/%.pmc,$(PMFILES))
TIL about pmc files. It sounds like they've had a storied history, but
should be supported everywhere.
The rest of the patch all looked good to me. Thanks for working on this.