Re: [PATCH v4 3/4] RUNTIME_PREFIX relocatable Git

[Dan Jacques <dnj@google.com>]

My initial set of replies are inline here:
https://public-inbox.org/git/20171129223807.91343-1-dnj@google.com/T/#m1ff5cda787eaea4a4015a8f00a8be5966122c73a

I put together this solution for the I18N module. It exports "localedir" to
the Perl header and injects the correct value into Git::I18N. It also
suppresses the emission of the full ++LOCALEDIR++ when in runtime prefix
mode.

One downside of this approach is that, since the point of resolution happens
at the beginning of the script execution, external users of the runtime
prefix Git::I18N module will not be able to resolve the locale directory.
I think this is okay, though, since RUNTIME_PREFIX_PERL is decidedly *not*
intended for system installation and is not trying to offer system module
import portability.

After looking a bit at your $ENV{GIT_EXEC_PATH} optimization suggestion, I
ended up favoring the current usage of FindBin::Bin for a few reasons:

- Encoding the relative GITEXECDIR into the script is more consistent with
  how "exec_cmd.c" does its own self-resolution.
- The resolution code is a bit more readable, both in terms of what it's
  doing and where it starts from.
- It makes the scripts individually executable, rather than having to go
  through the Git wrapper. This is, minimally, useful for testing.

I admittedly don't know much about FindBin, so if it has a downside that
could outweigh this I'm more than happy to reevaluate.

With respect to testing, I did find "GIT_TEST_INSTALLED". Unfortunately,
it doesn't seem to work against the full test suite even on master, and
individual tests, especially Perl and locale, set their own path
expectations. I think including installation-relative testing here would
take a fair amount of work.

Note that this is a replacement for [PATCH v4 3/4], so if you're trying to
apply it, you will need to build it on top of the preceding patches.

Many thanks,
-Dan

---
 Makefile                               | 59 +++++++++++++++++++++++++++++++---
 perl/Git/I18N.pm                       |  2 +-
 perl/Makefile                          | 17 ++++++----
 perl/header_runtime_prefix.pl.template | 31 ++++++++++++++++++
 t/test-lib.sh                          | 13 +++++---
 5 files changed, 107 insertions(+), 15 deletions(-)
 create mode 100644 perl/header_runtime_prefix.pl.template

diff --git a/Makefile b/Makefile
index 80904f8b0..558bd3ebb 100644
--- a/Makefile
+++ b/Makefile
@@ -425,6 +425,10 @@ all::
 #
 # to say "export LESS=FRX (and LV=-c) if the environment variable
 # LESS (and LV) is not set, respectively".
+#
+# Define RUNTIME_PREFIX_PERL to configure Git's PERL commands to locate Git
+# support libraries relative to their filesystem path instead of hard-coding
+# it.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -462,6 +466,7 @@ ARFLAGS = rcs
 #   mandir
 #   infodir
 #   htmldir
+#   localedir
 #   perllibdir
 # This can help installing the suite in a relocatable way.
 
@@ -483,9 +488,12 @@ lib = lib
 # DESTDIR =
 pathsep = :
 
+gitexecdir_relative = $(patsubst $(prefix)/%,%,$(gitexecdir))
 mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
 infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
+localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))
 htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
+perllibdir_relative = $(patsubst $(prefix)/%,%,$(perllibdir))
 
 export prefix bindir sharedir sysconfdir gitwebdir localedir
 
@@ -1718,11 +1726,13 @@ bindir_relative_SQ = $(subst ','\'',$(bindir_relative))
 mandir_relative_SQ = $(subst ','\'',$(mandir_relative))
 infodir_relative_SQ = $(subst ','\'',$(infodir_relative))
 localedir_SQ = $(subst ','\'',$(localedir))
+localedir_relative_SQ = $(subst ','\'',$(localedir_relative))
 gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
+gitexecdir_relative_SQ = $(subst ','\'',$(gitexecdir_relative))
 template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
 prefix_SQ = $(subst ','\'',$(prefix))
-perllibdir_SQ = $(subst ','\'',$(perllibdir))
+perllibdir_relative_SQ = $(subst ','\'',$(perllibdir_relative))
 gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
@@ -1965,14 +1975,52 @@ perl/PM.stamp: GIT-PERL-DEFINES FORCE
 	$(RM) $@+
 
 PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template
+perl_localedir = $(localedir)
 
 PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ)
-PERL_DEFINES += $(NO_PERL_MAKEMAKER)
-PERL_DEFINES += $(perllibdir)
+PERL_DEFINES += $(NO_PERL_MAKEMAKER) $(RUNTIME_PREFIX_PERL)
+
+# Support Perl runtime prefix. In this mode, a different header is installed
+# into Perl scripts. This header expects both the scripts and their support
+# library to be installed relative to $(prefix), and resolves the path to
+# the Perl libraries (perllibdir) from the executable's current path
+# (gitexecdir).
+#
+# This configuration requires both $(perllibdir) and $(gitexecdir) to be
+# relative paths, and will error if this is not the case.
+ifdef RUNTIME_PREFIX_PERL
+
+PERL_HEADER_TEMPLATE = perl/header_runtime_prefix.pl.template
+
+# RUNTIME_PREFIX_PERL requires a $(perllibdir) value.
+ifeq ($(perllibdir),)
+perllibdir = $(sharedir)/perl5
+endif
+
+ifneq ($(filter /%,$(firstword $(perllibdir_relative))),)
+$(error RUNTIME_PREFIX_PERL requires a relative perllibdir, not: $(perllibdir))
+endif
+
+ifneq ($(filter /%,$(firstword $(gitexecdir_relative))),)
+$(error RUNTIME_PREFIX_PERL requires a relative gitexecdir, not: $(gitexecdir))
+endif
+
+ifneq ($(filter /%,$(firstword $(localedir_relative))),)
+$(error RUNTIME_PREFIX_PERL requires a prefix-relative localedir, not: $(localedir))
+endif
+
+# Don't need to export a locale directory to Perl scripts, since the runtime
+# header will resolve it.
+perl_localedir =
+
+endif
+
+PERL_DEFINES += $(gitexecdir) $(perllibdir) $(localedir)
+export perl_localedir
 
 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)
+	  prefix='$(prefix_SQ)' perllibdir='$(perllibdir_relative_SQ)' $(@F)
 
 $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
 	$(QUIET_GEN)$(RM) $@ $@+ && \
@@ -2001,6 +2049,9 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES perl/perl.mak Makefile
 	INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
 	sed -e 's=@@PATHSEP@@=$(pathsep)=g' \
 	    -e 's=@@INSTLIBDIR@@='$$INSTLIBDIR'=g' \
+	    -e 's=@@GITEXECDIR@@=$(gitexecdir_relative_SQ)=g' \
+	    -e 's=@@PERLLIBDIR@@=$(perllibdir_relative_SQ)=g' \
+	    -e 's=@@LOCALEDIR@@=$(localedir_relative_SQ)=g' \
 	    $< >$@+ && \
 	mv $@+ $@
 
diff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm
index 836a5c238..25141d27d 100644
--- a/perl/Git/I18N.pm
+++ b/perl/Git/I18N.pm
@@ -18,7 +18,7 @@ our @EXPORT_OK = @EXPORT;
 
 sub __bootstrap_locale_messages {
 	our $TEXTDOMAIN = 'git';
-	our $TEXTDOMAINDIR = $ENV{GIT_TEXTDOMAINDIR} || '++LOCALEDIR++';
+	our $TEXTDOMAINDIR ||= $ENV{GIT_TEXTDOMAINDIR} || '++LOCALEDIR++';
 
 	require POSIX;
 	POSIX->import(qw(setlocale));
diff --git a/perl/Makefile b/perl/Makefile
index b2aeeb0d8..14bdda99d 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -8,7 +8,7 @@
 #
 # prefix must be defined as the Git installation prefix.
 #
-# localedir must be defined as the path to the locale data.
+# perl_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
@@ -22,7 +22,7 @@ modules =
 
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 prefix_SQ = $(subst ','\'',$(prefix))
-localedir_SQ = $(subst ','\'',$(localedir))
+localedir_SQ = $(subst ','\'',$(perl_localedir))
 
 ifndef V
 	QUIET = @
@@ -116,11 +116,16 @@ else
 # This may be empty if perllibdir was empty.
 instdir_SQ = $(subst ','\'',$(perllib_instdir))
 
+MAKEMAKER_PARAMS = PREFIX='$(prefix_SQ)' INSTALL_BASE=''
+ifneq ($(instdir_SQ),)
+MAKEMAKER_PARAMS += LIB='$(instdir_SQ)'
+endif
+ifneq ($(localedir_SQ),)
+MAKEMAKER_PARAMS += --localedir='$(localedir_SQ)'
+endif
+
 $(makfile): Makefile.PL ../GIT-CFLAGS ../GIT-PERL-DEFINES
-	$(PERL_PATH) $< \
-	  PREFIX='$(prefix_SQ)' INSTALL_BASE='' \
-	  LIB='$(instdir_SQ)' \
-	  --localedir='$(localedir_SQ)'
+	$(PERL_PATH) $< $(MAKEMAKER_PARAMS)
 
 endif
 
diff --git a/perl/header_runtime_prefix.pl.template b/perl/header_runtime_prefix.pl.template
new file mode 100644
index 000000000..9cc63cfb6
--- /dev/null
+++ b/perl/header_runtime_prefix.pl.template
@@ -0,0 +1,31 @@
+# BEGIN RUNTIME_PREFIX_PERL generated code.
+#
+# This finds our Git::* libraries relative to the script's runtime path.
+sub __git_system_path {
+	my ($relpath) = @_;
+	my $gitexecdir_relative = '@@GITEXECDIR@@';
+
+	require FindBin;
+	require File::Spec;
+	($FindBin::Bin =~ m=${gitexecdir_relative}$=) || die('Unrecognized runtime path.');
+	my $prefix = substr($FindBin::Bin, 0, -length($gitexecdir_relative));
+	return File::Spec->catdir($prefix, $relpath);
+}
+
+BEGIN {
+	use lib split /@@PATHSEP@@/,
+	(
+		$ENV{GITPERLLIB} ||
+		do {
+			my $perllibdir = __git_system_path('@@PERLLIBDIR@@');
+			(-e $perllibdir) || die("Invalid system path ($relpath): $path");
+			$perllibdir;
+		}
+	);
+
+	# Export the system locale directory to the I18N module. The locale directory
+	# is only installed if NO_GETTEXT is set.
+	$Git::I18N::TEXTDOMAINDIR = __git_system_path('@@LOCALEDIR@@');
+}
+
+# END RUNTIME_PREFIX_PERL generated code.
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 116bd6a70..d6ea423af 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -919,11 +919,16 @@ then
 	fi
 fi
 
-GITPERLLIB="$GIT_BUILD_DIR"/perl/blib/lib:"$GIT_BUILD_DIR"/perl/blib/arch/auto/Git
+if test -n "$GIT_TEST_PERLLIB"
+then
+  GITPERLLIB="$GIT_TEST_PERLLIB"
+else
+	GITPERLLIB="$GIT_BUILD_DIR"/perl/blib/lib:"$GIT_BUILD_DIR"/perl/blib/arch/auto/Git
+	test -d "$GIT_BUILD_DIR"/templates/blt || {
+		error "You haven't built things yet, have you?"
+	}
+fi
 export GITPERLLIB
-test -d "$GIT_BUILD_DIR"/templates/blt || {
-	error "You haven't built things yet, have you?"
-}
 
 if ! test -x "$GIT_BUILD_DIR"/t/helper/test-chmtime
 then
-- 
2.15.0.chromium12