git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Makefile: replace perl/Makefile.PL with simple make rules

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 30, 2017, 22:30 UTC
Message-ID
<87bmjjv1pu.fsf@evledraar.booking.com>
In-Reply-To
<20171130213105.GA8861@sigill.intra.peff.net>
On Thu, Nov 30 2017, Jeff King jotted:
Show 28 quoted lines
> 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[1].
>
> 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[2] building a perl C binding to interface with libgit,
>> this functionality, that was removed[3] 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.

I think so, but I'm not 100% sure. I'm hoping downstream maintainers will catch this one if it's an issue, but in many cases it'll just work, e.g. I have /usr/share/perl5 in /usr/bin/perl's @INC on Debian even though that's not the target install dir EU::MM would have picked.

Whether there's perl configurations that don't have $prefix/share/perl5 in @INC at all I don't know.

Show 8 quoted lines
>>  * 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.
Indeed, we'll need to ship at least the Git.3pm manpage.

I tried coming up with something that worked like the current Makefile.PL but couldn't figure out how I'd generate a::file::like::this.3pm from a/file/like/this.pm

So I hacked up the following minimal patch to just build Git.3pm:
	diff --git a/Makefile b/Makefile
	index 48f1b868d1..ba6607b7e7 100644
	--- a/Makefile
	+++ b/Makefile
	@@ -1709,6 +1709,7 @@ ETC_GITATTRIBUTES_SQ = $(subst ','\'',$(ETC_GITATTRIBUTES))
	 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
	 bindir_SQ = $(subst ','\'',$(bindir))
	 bindir_relative_SQ = $(subst ','\'',$(bindir_relative))
	+mandir_SQ = $(subst ','\'',$(mandir))
	 mandir_relative_SQ = $(subst ','\'',$(mandir_relative))
	 infodir_relative_SQ = $(subst ','\'',$(infodir_relative))
	 perllibdir_SQ = $(subst ','\'',$(perllibdir))
	@@ -2284,6 +2285,10 @@ perl/build/lib/%.pmc: perl/%.pm
	 	$(QUIET_GEN)mkdir -p $(dir $@) && \
	 	sed -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' < $< > $@
	+perl/build/man/man3/Git.3pm: perl/Git.pm
	+	$(QUIET_GEN)mkdir -p $(dir $@) && \
	+	pod2man $< $@
	+
	 FIND_SOURCE_FILES = ( \
	 	git ls-files \
	 		'*.[hcS]' \
	@@ -2595,12 +2600,17 @@ endif
	 install-gitweb:
	 	$(MAKE) -C gitweb install
	-install-doc:
	+install-doc: install-man-perl
	 	$(MAKE) -C Documentation install
	-install-man:
	+install-man: install-man-perl
	 	$(MAKE) -C Documentation install-man
	+install-man-perl: perl/build/man/man3/Git.3pm
	+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mandir_SQ)/man3'
	+	(cd perl/build/man/man3 && $(TAR) cf - .) | \
	+	(cd '$(DESTDIR_SQ)$(mandir_SQ)/man3' && umask 022 && $(TAR) xof -)
	+
	 install-html:
	 	$(MAKE) -C Documentation install-html

Maybe this approach is bad, I don't know. This might be a better fit for Documentation/Makefile since it has some "man" logic already, but do we want it to be depending on stuff in ../perl?

On my Debian install I have these from existing Git:
    /usr/share/man/man3/Git.3pm.gz
    /usr/share/man/man3/Git::I18N.3pm.gz
    /usr/share/man/man3/private-Error.3pm.gz

We definitely don't want the 2nd two ones to have manpages, that was always an accident. git-svn has a few more:

    /usr/share/man/man3/Git::SVN::Editor.3pm.gz
    /usr/share/man/man3/Git::SVN::Fetcher.3pm.gz
    /usr/share/man/man3/Git::SVN::Memoize::YAML.3pm.gz
    /usr/share/man/man3/Git::SVN::Prompt.3pm.gz
    /usr/share/man/man3/Git::SVN::Ra.3pm.gz
    /usr/share/man/man3/Git::SVN::Utils.3pm.gz

But those all say that they're internal API only. So maybe this hack of mine is fine. What do others think?

In any case, a solution that tries to build all the manpages with a glob is going to suck currently since some of them have POD, and others don't, which gets us into the headache of trying to distinguish whether something has POD v.s. whether we just had pod2man fail.

So it seems better just to whitelist the man3 pages that get build, which to me seems like just one currently.

Show 24 quoted lines
>> @@ -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
> similar.
>
> 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.
>
> -Peff
Previous: Jeff KingNext: Alex Vandiver
Message 8 of 35 in “Makefile: replace the overly complex perl build system with something simple”
  1. Makefile: replace the overly complex perl build system with something simpleÆvar Arnfjörð Bjarmason, Nov 29, 2017
  2. Makefile: replace perl/Makefile.PL with simple make rulesÆvar Arnfjörð Bjarmason, Nov 29, 2017
  3. Jonathan NiederNov 30, 2017
  4. Ævar Arnfjörð BjarmasonNov 30, 2017
  5. Jonathan NiederNov 30, 2017
  6. Eric WongNov 30, 2017
  7. Jeff KingNov 30, 2017
  8. Ævar Arnfjörð BjarmasonNov 30, 2017
  9. Alex VandiverDec 21, 2017
  10. Makefile: replace perl/Makefile.PL with simple make rulesÆvar Arnfjörð Bjarmason, Dec 3, 2017
  11. Junio C HamanoDec 4, 2017
  12. Ævar Arnfjörð BjarmasonDec 4, 2017
  13. Junio C HamanoDec 4, 2017
  14. Dan JacquesDec 4, 2017
  15. Makefile: replace perl/Makefile.PL with simple make rulesÆvar Arnfjörð Bjarmason, Dec 10, 2017
  16. Junio C HamanoDec 11, 2017
  17. Randall S. BeckerDec 12, 2017
  18. Ævar Arnfjörð BjarmasonDec 12, 2017
  19. Michael J GruberDec 15, 2017
  20. Todd ZullingerDec 15, 2017
  21. Ævar Arnfjörð BjarmasonDec 15, 2017
  22. Junio C HamanoDec 19, 2017
  23. Makefile: replace perl/Makefile.PL with simple make rulesÆvar Arnfjörð Bjarmason, Dec 19, 2017
  24. Todd ZullingerDec 20, 2017
  25. Makefile: replace perl/Makefile.PL with simple make rulesÆvar Arnfjörð Bjarmason, Dec 20, 2017
  26. Todd ZullingerDec 20, 2017
  27. Makefile: replace perl/Makefile.PL with simple make rulesÆvar Arnfjörð Bjarmason, Dec 20, 2017
  28. Todd ZullingerDec 20, 2017
  29. Alex RiesenDec 21, 2017
  30. Junio C HamanoDec 22, 2017
  31. Ævar Arnfjörð BjarmasonDec 27, 2017
  32. Junio C HamanoDec 28, 2017
  33. Jonathan NiederJan 2, 2018
  34. perl: treat PERLLIB_EXTRA as an extra path again (Re: [PATCH v6] Makefile: replace perl/Makefile.PL with simple make rules)Jonathan Nieder, Jan 2, 2018
  35. Ævar Arnfjörð BjarmasonJan 2, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.