Re: [PATCH v4 2/4] Makefile: add support for "perllibdir"

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Nov 29 2017, Dan Jacques jotted:

> Add the "perllibdir" Makefile variable, which allows the customization
> of the Perl library installation path.
>
> The Perl library installation path is currently left entirely to the
> Perl Makefile implementation, either MakeMaker (default) or a fixed path
> when NO_PERL_MAKEMAKER is enabled. This patch introduces "perllibdir", a
> Makefile variable that can override that Perl module installation path.
>
> As with some other Makefile variables, "perllibdir" may be either
> absolute or relative. In the latter case, it is treated as relative to
> "$(prefix)".
>
> Add some incidental documentation to perl/Makefile.
>
> Explicitly specifying an installation path is necessary for Perl runtime
> prefix support, as runtime prefix resolution code must know in advance
> where the Perl support modules are installed.
>
> Signed-off-by: Dan Jacques <dnj@google.com>
> ---
>  Makefile      | 18 +++++++++++++-----
>  perl/Makefile | 52 ++++++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 59 insertions(+), 11 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f7c4ac207..80904f8b0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -462,6 +462,7 @@ ARFLAGS = rcs
>  #   mandir
>  #   infodir
>  #   htmldir
> +#   perllibdir
>  # This can help installing the suite in a relocatable way.
>
>  prefix = $(HOME)
> @@ -1721,6 +1722,7 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
>  template_dir_SQ = $(subst ','\'',$(template_dir))
>  htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
>  prefix_SQ = $(subst ','\'',$(prefix))
> +perllibdir_SQ = $(subst ','\'',$(perllibdir))
>  gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
>
>  SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> @@ -1955,17 +1957,22 @@ $(SCRIPT_PERL_GEN): perl/perl.mak
>
>  perl/perl.mak: perl/PM.stamp
>
> -perl/PM.stamp: FORCE
> +perl/PM.stamp: GIT-PERL-DEFINES FORCE
>  	@$(FIND) perl -type f -name '*.pm' | sort >$@+ && \
> +	cat GIT-PERL-DEFINES >>$@+ && \
>  	$(PERL_PATH) -V >>$@+ && \
>  	{ cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \
>  	$(RM) $@+
>
> -perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
> -	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
> -
>  PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template
> -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ)
> +
> +PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ)
> +PERL_DEFINES += $(NO_PERL_MAKEMAKER)
> +PERL_DEFINES += $(perllibdir)
> +
> +perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
> +	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' \
> +	  prefix='$(prefix_SQ)' perllibdir='$(perllibdir_SQ)' $(@F)
>
>  $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
>  	$(QUIET_GEN)$(RM) $@ $@+ && \
> @@ -1979,6 +1986,7 @@ $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GI
>  	chmod +x $@+ && \
>  	mv $@+ $@
>
> +PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
>  GIT-PERL-DEFINES: FORCE
>  	@FLAGS='$(PERL_DEFINES)'; \
>  	    if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
> diff --git a/perl/Makefile b/perl/Makefile
> index f657de20e..b2aeeb0d8 100644
> --- a/perl/Makefile
> +++ b/perl/Makefile
> @@ -1,6 +1,22 @@
>  #
>  # Makefile for perl support modules and routine
>  #
> +# This Makefile generates "perl.mak", which contains the actual build and
> +# installation directions.
> +#
> +# PERL_PATH must be defined to be the path of the Perl interpreter to use.
> +#
> +# prefix must be defined as the Git installation prefix.
> +#
> +# localedir must be defined as the path to the locale data.
> +#
> +# perllibdir may be optionally defined to override the default Perl module
> +# installation directory, which is relative to prefix. If perllibdir is not
> +# absolute, it will be treated as relative to prefix.
> +#
> +# NO_PERL_MAKEMAKER may be defined to use a built-in Makefile generation method
> +# instead of Perl MakeMaker.
> +
>  makfile:=perl.mak
>  modules =
>
> @@ -12,6 +28,16 @@ ifndef V
>  	QUIET = @
>  endif
>
> +# If a library directory is provided, and it is not an absolute path, resolve
> +# it relative to prefix.
> +ifneq ($(perllibdir),)
> +ifneq ($(filter /%,$(firstword $(perllibdir))),)
> +perllib_instdir = $(perllibdir)
> +else
> +perllib_instdir = $(prefix)/$(perllibdir)
> +endif
> +endif

Maybe I'm missing something, but isn't this "perllibdir can be relative"
aim orthagonal to what this patch series is about? I.e. we could just
avoid this work by saying you must say "prefix=/usr
perllibdir=/usr/perl" instead of "prefix=/usr perllibdir=perl" as this
allows.

I started going down this route with my own patch and just threw it
away.

>  all install instlibdir: $(makfile)
>  	$(QUIET)$(MAKE) -f $(makfile) $@
>
> @@ -25,7 +51,12 @@ clean:
>  $(makfile): PM.stamp
>
>  ifdef NO_PERL_MAKEMAKER
> -instdir_SQ = $(subst ','\'',$(prefix)/lib)
> +
> +ifeq ($(perllib_instdir),)
> +perllib_instdir = $(prefix)/lib
> +endif
> +
> +instdir_SQ = $(subst ','\'',$(perllib_instdir))
>
>  modules += Git
>  modules += Git/I18N
> @@ -42,7 +73,7 @@ modules += Git/SVN/Prompt
>  modules += Git/SVN/Ra
>  modules += Git/SVN/Utils
>
> -$(makfile): ../GIT-CFLAGS Makefile
> +$(makfile): ../GIT-CFLAGS ../GIT-PERL-DEFINES Makefile
>  	echo all: private-Error.pm Git.pm Git/I18N.pm > $@
>  	set -e; \
>  	for i in $(modules); \
> @@ -79,12 +110,21 @@ $(makfile): ../GIT-CFLAGS Makefile
>  	echo '	cp private-Error.pm "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
>  	echo instlibdir: >> $@
>  	echo '	echo $(instdir_SQ)' >> $@
> +
>  else
> -$(makfile): Makefile.PL ../GIT-CFLAGS
> -	$(PERL_PATH) $< PREFIX='$(prefix_SQ)' INSTALL_BASE='' --localedir='$(localedir_SQ)'
> +
> +# This may be empty if perllibdir was empty.
> +instdir_SQ = $(subst ','\'',$(perllib_instdir))
> +
> +$(makfile): Makefile.PL ../GIT-CFLAGS ../GIT-PERL-DEFINES
> +	$(PERL_PATH) $< \
> +	  PREFIX='$(prefix_SQ)' INSTALL_BASE='' \
> +	  LIB='$(instdir_SQ)' \
> +	  --localedir='$(localedir_SQ)'
> +
>  endif
>
>  # this is just added comfort for calling make directly in perl dir
>  # (even though GIT-CFLAGS aren't used yet. If ever)
> -../GIT-CFLAGS:
> -	$(MAKE) -C .. GIT-CFLAGS
> +../GIT-CFLAGS ../GIT-PERL-DEFINES:
> +	$(MAKE) -C .. $(@F)