[PATCH v2 0/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[PATCH v2 0/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Dan Jacques]

Previous round:
- https://public-inbox.org/git/20171116170523.28696-1-dnj@google.com/
- https://public-inbox.org/git/20171116170523.28696-2-dnj@google.com/

Junio,

Thanks for taking the time to review this patch. Responses inline:

> The "regardless of whether the user has overridden it" part sounded
> alarming and made me wince twice.  I think you meant...
>
> As I have multiple installations of various versions of Git, I find
> the latter somewhat disturbing.

That's an excellent point. You also raised other concerns farther down
with respect to PERL and gettext() environment variables having a similar
behavior of overriding explicit user values. I had regarded these
environment variables as internal, but if they are part of the user
interface then forcefully overriding them is probably unacceptable.

I took some time and restructured the patch to avoid leveraging
environment variables:

- For Git code, each ancillary tool will now just resolve its own
  executable path.
- For PERL code, this patch introduces "RUNTIME_PREFIX_PERL", which
  injects an alternative header into PERL tooling enabling it to resolve
  against its installation path.
- gettext() now resolves using system_path(), as it should have in the
  first place.

The net result of this is that this patch should no longer presume
ownership of environment variables, nor modify them any differently
than Git always has.

The PERL change is more significant now, so I defined a
"RUNTIME_PREFIX_PERL" flag that makes PERL specifically relocatable. I
moved the behavior of forcefullyl setting NO_PERL_MAKEMAKER from
RUNTIME_PREFIX (used by Git-for-Windows) to RUNTIME_PREFIX_PERL, so this
change should change how Git-for-Windows handles PERL anymore.

> We usually frown upon these because they often gets distracting, but
> I didn't find the ones in this patch too bad.

My apologies - I was emboldened by this line in "CodingGuidelines":

  "Fixing style violations while working on a real change as a
  preparatory clean-up step is good, but otherwise avoid useless code
  churn for the sake of conforming to the style."

I suppose "preparatory" probably means "in a preceding separate commit".

> I presume that this is because we may need to know where to find the
> locale stuff before calling git_setup_gettext(); makes sense.

Correct.

> OK, so that is a more appropriate name for the variable that is a
> logical successor of argv0_path.  I wonder if the file-scope static
> variable argv_exec_path we see above would want to move to somewhere
> closer to one of these "platform specific methods", though.

Looking into this, I realized that the method-local static
"cached_exec_path" variable and the global "argv_exec_path" are
redundant, and that "argv_exec_path" escaped my renaming sweep. I
consolidated the two and moved the new global, "exec_path_value",
closer to the only two methods that reference it.

> I think this is our first use of realpath(), which is XSI.

While looking for examples in the code, I ran into `strbuf_realpath`,
which is a Git-native implementation of "realpath". I switched over to
this, making "procfs" resolution more idiomatic and also slightly
simpler, and this patch no longer uses a new API!

> I wonder why argv0 (i.e. the full-path case) is not
> the first one to try, though---isn't that one the simplest?

I briefly covered this in the cover letter, but "argv[0]" is not reliable
on most (POSIX) systems, since "argv" is a user-supplied value to the
`execve` system call that launches a process. In many cases, "argv[0]"
will be the path of the executable, but in some it will just be the name
of the executable (shells do this when executing via PATH resolution)
and nothing is stopping an `execve` caller from supplying a completely
arbitrary value.

On Windows, "argv[0]" seems to always be the full path of the binary,
and this is something that Git-for-Windows relies on; however, on POSIX,
it is an unreliable source of information. The resolution sled falls
back onto it if all else fails, but strongly prefers a more
authoritative method when avaliable.

I've added some documentation to this effect to hopefully clarify the
rationale in the code.

> Also, I wonder if this caller gets simpler to read and understand if
> each of these "platform specific" ones are done like so[...]

I considered your idea, and while it does declutter the sled code, I
think it has a net detrimental effect because it also removes the
platform-specific context from the sled code: A cursory read of that
function would suggest that every resolution method is attempted on all
platforms, and I think that this is the wrong impression to give.

I've added some comments and formatting around that code to hopefully
address the clutter. I'm not opposed to restructuring it (e.g., use a
resolution function vector), but I thought I'd try formatting first.
Let me know what you think!

===

Changes in "v1" from previous version:

- Added comments and formatting to improve readability of
  platform-sepecific executable path resolution sleds in
  `git_get_exec_path`.

- Consolidated "cached_exec_path" and "argv_exec_path" globals
  into "exec_path_value".

- Use `strbuf_realpath` instead of `realpath` for procfs resolution.

- Removed new environment variable exports. Git with RUNTIME_PREFIX no
  longer exports or consumes any additional environment information.

  - Updated PERL script resolution strategy: rather than having Git export
    the relative executable path to the PERL scripts, they now resolve
    it independently when RUNTIME_PREFIX_PERL is enabled.

  - Updated resolution strategy for "gettext()": use system_path() instead
    of special environment variable.

  - Added `sysctl` executable resolution support for BSDs that don't
    mount "procfs" by default (most of them).

Dan Jacques (1):
  exec_cmd: RUNTIME_PREFIX on some POSIX systems

 .gitignore       |   1 +
 Makefile         |  88 +++++++++++++++++---
 cache.h          |   1 +
 common-main.c    |   4 +-
 config.mak.uname |   7 ++
 exec_cmd.c       | 239 +++++++++++++++++++++++++++++++++++++++++++++++--------
 exec_cmd.h       |   4 +-
 gettext.c        |   8 +-
 git.c            |   2 +-
 9 files changed, 304 insertions(+), 50 deletions(-)

-- 
2.15.0.448.gf294e3d99a-goog

[PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Dan Jacques]

Enable Git to resolve its own binary location using a variety of
OS-specific and generic methods, including:

- procfs via "/proc/self/exe" (Linux)
- _NSGetExecutablePath (Darwin)
- KERN_PROC_PATHNAME sysctl on BSDs.
- argv0, if absolute (all, including Windows).

This is used to enable RUNTIME_PREFIX support for non-Windows systems,
notably Linux and Darwin. When configured with RUNTIME_PREFIX, Git will
do a best-effort resolution of its executable path and automatically use
this as its "exec_path" for relative helper and data lookups, unless
explicitly overridden.

Git's PERL tooling now responds to RUNTIME_PREFIX_PERL. When configured,
Git's generated PERL scripts resolve the Git library location relative to
their runtime paths instead of hard-coding them. Structural changes
were made to Makefile to support selective PERL header generation.

Small incidental formatting cleanup of "exec_cmd.c".

Signed-off-by: Dan Jacques <dnj@google.com>
---
 .gitignore       |   1 +
 Makefile         |  88 +++++++++++++++++---
 cache.h          |   1 +
 common-main.c    |   4 +-
 config.mak.uname |   7 ++
 exec_cmd.c       | 239 +++++++++++++++++++++++++++++++++++++++++++++++--------
 exec_cmd.h       |   4 +-
 gettext.c        |   8 +-
 git.c            |   2 +-
 9 files changed, 304 insertions(+), 50 deletions(-)

diff --git a/.gitignore b/.gitignore
index 833ef3b0b..89bd7bd8a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,7 @@
 /GIT-LDFLAGS
 /GIT-PREFIX
 /GIT-PERL-DEFINES
+/GIT-PERL-HEADER
 /GIT-PYTHON-VARS
 /GIT-SCRIPT-DEFINES
 /GIT-USER-AGENT
diff --git a/Makefile b/Makefile
index ee9d5eb11..6fddb8b8f 100644
--- a/Makefile
+++ b/Makefile
@@ -296,7 +296,8 @@ all::
 # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl).
 #
 # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
-# MakeMaker (e.g. using ActiveState under Cygwin).
+# MakeMaker (e.g. using ActiveState under Cygwin). NO_PERL_MAKEMAKER is
+# automatically enabled when using RUNTIME_PREFIX_PERL.
 #
 # Define NO_PERL if you do not want Perl scripts or libraries at all.
 #
@@ -416,6 +417,16 @@ all::
 #
 # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
 #
+# Define HAVE_BSD_KERN_PROC_SYSCTL if your platform supports the KERN_PROC BSD
+# sysctl function.
+#
+# Define PROCFS_EXECUTABLE_PATH if your platform mounts a "procfs" filesystem
+# capable of resolving the path of the current executable. If defined, this
+# must be the canonical path for the "procfs" current executable path.
+#
+# Define HAVE_NS_GET_EXECUTABLE_PATH if your platform supports calling
+# _NSGetExecutablePath to retrieve the path of the running executable.
+#
 # Define HAVE_GETDELIM if your system has the getdelim() function.
 #
 # Define PAGER_ENV to a SP separated VAR=VAL pairs to define
@@ -425,6 +436,16 @@ all::
 #
 # to say "export LESS=FRX (and LV=-c) if the environment variable
 # LESS (and LV) is not set, respectively".
+#
+# Define RUNTIME_PREFIX to configure Git to resolve its ancillary tooling and
+# support files relative to the location of the runtime binary, rather than
+# hard-coding them into the binary. Git installations built with RUNTIME_PREFIX
+# can be moved to arbitrary filesystem locations. Users may want to enable
+# RUNTIME_PREFIX_PERL as well (see below).
+#
+# 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. RUNTIME_PREFIX_PERL also sets NO_PERL_MAKEMAKER.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -462,6 +483,7 @@ ARFLAGS = rcs
 #   mandir
 #   infodir
 #   htmldir
+#   localedir
 # This can help installing the suite in a relocatable way.
 
 prefix = $(HOME)
@@ -485,6 +507,7 @@ pathsep = :
 mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
 infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
 htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
+localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))
 
 export prefix bindir sharedir sysconfdir gitwebdir localedir
 
@@ -1522,9 +1545,6 @@ ifdef SHA1_MAX_BLOCK_SIZE
 	LIB_OBJS += compat/sha1-chunked.o
 	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
 endif
-ifdef NO_PERL_MAKEMAKER
-	export NO_PERL_MAKEMAKER
-endif
 ifdef NO_HSTRERROR
 	COMPAT_CFLAGS += -DNO_HSTRERROR
 	COMPAT_OBJS += compat/hstrerror.o
@@ -1549,6 +1569,15 @@ ifdef RUNTIME_PREFIX
 	COMPAT_CFLAGS += -DRUNTIME_PREFIX
 endif
 
+ifdef RUNTIME_PREFIX_PERL
+	# Control PERL library location so its paths and contents are not dependent on
+	# the host's PERL version. See perl/Makefile for more information.
+	NO_PERL_MAKEMAKER = YesPlease
+endif
+ifdef NO_PERL_MAKEMAKER
+	export NO_PERL_MAKEMAKER
+endif
+
 ifdef NO_PTHREADS
 	BASIC_CFLAGS += -DNO_PTHREADS
 else
@@ -1628,10 +1657,23 @@ ifdef HAVE_BSD_SYSCTL
 	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
 
+ifdef HAVE_BSD_KERN_PROC_SYSCTL
+	BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL
+endif
+
 ifdef HAVE_GETDELIM
 	BASIC_CFLAGS += -DHAVE_GETDELIM
 endif
 
+ifneq ($(PROCFS_EXECUTABLE_PATH),)
+	procfs_executable_path_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH))
+	BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"'
+endif
+
+ifdef HAVE_NS_GET_EXECUTABLE_PATH
+	BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
@@ -1714,6 +1756,7 @@ 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))
 template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
@@ -1962,17 +2005,16 @@ 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_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ)
-$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE
+
+$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
 	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
 	INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
 	sed -e '1{' \
 	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
-	    -e '	h' \
-	    -e '	s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
-	    -e '	H' \
-	    -e '	x' \
+	    -e '	rGIT-PERL-HEADER' \
+	    -e '	G' \
 	    -e '}' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    $< >$@+ && \
@@ -1986,6 +2028,29 @@ GIT-PERL-DEFINES: FORCE
 		echo "$$FLAGS" >$@; \
 	    fi
 
+GIT-PERL-HEADER: perl/perl.mak GIT-PERL-DEFINES FORCE
+ifndef RUNTIME_PREFIX_PERL
+	# Hardcode the runtime path.
+	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
+	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
+	echo \
+	  'use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));' \
+	  >$@
+else
+	# Probe the runtime path relative to the PERL script. RUNTIME_PREFIX_PERL
+	# automatically sets NO_PERL_MAKEMAKER, causing PERL scripts to be installed
+	# to "$(prefix)/lib" (see "perl/Makefile"). This expectation is hard-coded
+	# into the generated code below.
+	GITEXECDIR='$(gitexecdir_SQ)' && \
+	echo \
+	  'sub _get_git_lib{'\
+	  'use FindBin;'\
+	  '(my $$p=$$FindBin::Bin)=~s=/'$${GITEXECDIR}'$$==;'\
+		'return File::Spec->catdir($$p,"'"lib"'");' \
+	  '};' \
+	  'use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB}||_get_git_lib()));'\
+	  >$@
+endif
 
 .PHONY: gitweb
 gitweb:
@@ -2130,6 +2195,7 @@ endif
 exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX
 exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
+	'-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
 	'-DPREFIX="$(prefix_SQ)"'
 
@@ -2147,7 +2213,7 @@ attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \
 
 gettext.sp gettext.s gettext.o: GIT-PREFIX
 gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \
-	-DGIT_LOCALE_PATH='"$(localedir_SQ)"'
+	-DGIT_LOCALE_PATH='"$(localedir_relative_SQ)"'
 
 http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS += \
 	-DCURL_DISABLE_TYPECHECK
@@ -2704,7 +2770,7 @@ ifndef NO_TCLTK
 endif
 	$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
 	$(RM) GIT-USER-AGENT GIT-PREFIX
-	$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS
+	$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS
 
 .PHONY: all install profile-clean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
diff --git a/cache.h b/cache.h
index cb7fb7c00..fb7795410 100644
--- a/cache.h
+++ b/cache.h
@@ -445,6 +445,7 @@ static inline enum object_type object_type(unsigned int mode)
 #define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS"
 #define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH"
 #define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
+#define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
 
 /*
  * This environment variable is expected to contain a boolean indicating
diff --git a/common-main.c b/common-main.c
index 6a689007e..6516a1f89 100644
--- a/common-main.c
+++ b/common-main.c
@@ -32,12 +32,12 @@ int main(int argc, const char **argv)
 	 */
 	sanitize_stdfds();
 
+	git_resolve_executable_dir(argv[0]);
+
 	git_setup_gettext();
 
 	attr_start();
 
-	git_extract_argv0_path(argv[0]);
-
 	restore_sigpipe_to_default();
 
 	return cmd_main(argc, argv);
diff --git a/config.mak.uname b/config.mak.uname
index 685a80d13..58fd62b4d 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -37,6 +37,7 @@ ifeq ($(uname_S),Linux)
 	HAVE_GETDELIM = YesPlease
 	SANE_TEXT_GREP=-a
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
+	PROCFS_EXECUTABLE_PATH = /proc/self/exe
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	HAVE_ALLOCA_H = YesPlease
@@ -111,6 +112,7 @@ ifeq ($(uname_S),Darwin)
 	BASIC_CFLAGS += -DPROTECT_HFS_DEFAULT=1
 	HAVE_BSD_SYSCTL = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
+	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
@@ -206,6 +208,7 @@ ifeq ($(uname_S),FreeBSD)
 	HAVE_PATHS_H = YesPlease
 	GMTIME_UNRELIABLE_ERRORS = UnfortunatelyYes
 	HAVE_BSD_SYSCTL = YesPlease
+	HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
 	PAGER_ENV = LESS=FRX LV=-c MORE=FRX
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 endif
@@ -218,6 +221,8 @@ ifeq ($(uname_S),OpenBSD)
 	BASIC_LDFLAGS += -L/usr/local/lib
 	HAVE_PATHS_H = YesPlease
 	HAVE_BSD_SYSCTL = YesPlease
+	HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
+	PROCFS_EXECUTABLE_PATH = /proc/curproc/file
 endif
 ifeq ($(uname_S),MirBSD)
 	NO_STRCASESTR = YesPlease
@@ -236,6 +241,8 @@ ifeq ($(uname_S),NetBSD)
 	USE_ST_TIMESPEC = YesPlease
 	HAVE_PATHS_H = YesPlease
 	HAVE_BSD_SYSCTL = YesPlease
+	HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
+	PROCFS_EXECUTABLE_PATH = /proc/curproc/exe
 endif
 ifeq ($(uname_S),AIX)
 	DEFAULT_PAGER = more
diff --git a/exec_cmd.c b/exec_cmd.c
index ce192a2d6..c9006c4c9 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -2,53 +2,232 @@
 #include "exec_cmd.h"
 #include "quote.h"
 #include "argv-array.h"
-#define MAX_ARGS	32
 
-static const char *argv_exec_path;
+#if defined(RUNTIME_PREFIX)
+
+#if defined(HAVE_NS_GET_EXECUTABLE_PATH)
+#include <mach-o/dyld.h>
+#endif
+
+#if defined(HAVE_BSD_KERN_PROC_SYSCTL)
+#include <sys/param.h>
+#include <sys/types.h>
+#include <sys/sysctl.h>
+#endif
+
+#endif /* RUNTIME_PREFIX */
+
+#define MAX_ARGS 32
+
+static const char *system_prefix(void);
 
 #ifdef RUNTIME_PREFIX
-static const char *argv0_path;
+
+/**
+ * When using a runtime prefix, Git dynamically resolves paths relative to its
+ * executable.
+ *
+ * The method for determining the path of the executable is highly
+ * platform-specific.
+ */
+
+/**
+ * Path to the current Git executable. Resolved on startup by
+ * 'git_resolve_executable_dir'.
+ */
+static const char *executable_dirname;
 
 static const char *system_prefix(void)
 {
 	static const char *prefix;
 
-	assert(argv0_path);
-	assert(is_absolute_path(argv0_path));
+	assert(executable_dirname);
+	assert(is_absolute_path(executable_dirname));
 
 	if (!prefix &&
-	    !(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) &&
-	    !(prefix = strip_path_suffix(argv0_path, BINDIR)) &&
-	    !(prefix = strip_path_suffix(argv0_path, "git"))) {
+	    !(prefix = strip_path_suffix(executable_dirname, GIT_EXEC_PATH)) &&
+	    !(prefix = strip_path_suffix(executable_dirname, BINDIR)) &&
+	    !(prefix = strip_path_suffix(executable_dirname, "git"))) {
 		prefix = PREFIX;
 		trace_printf("RUNTIME_PREFIX requested, "
-				"but prefix computation failed.  "
-				"Using static fallback '%s'.\n", prefix);
+			     "but prefix computation failed.  "
+			     "Using static fallback '%s'.\n",
+			     prefix);
 	}
 	return prefix;
 }
 
-void git_extract_argv0_path(const char *argv0)
+/*
+ * Resolves the executable path from argv[0], only if it is absolute.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int git_get_exec_path_from_argv0(struct strbuf *buf, const char *argv0)
 {
 	const char *slash;
 
 	if (!argv0 || !*argv0)
-		return;
+		return -1;
 
 	slash = find_last_dir_sep(argv0);
+	if (slash) {
+		trace_printf("trace: resolved executable path from argv0: %s\n",
+			     argv0);
+		strbuf_add_absolute_path(buf, argv0);
+		return 0;
+	}
+	return -1;
+}
+
+#ifdef PROCFS_EXECUTABLE_PATH
+/*
+ * Resolves the executable path by examining a procfs symlink.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int git_get_exec_path_procfs(struct strbuf *buf)
+{
+	if (strbuf_realpath(buf, PROCFS_EXECUTABLE_PATH, 0)) {
+		trace_printf(
+			"trace: resolved executable path from procfs: %s\n",
+			buf->buf);
+		return 0;
+	}
+	return -1;
+}
+#endif /* PROCFS_EXECUTABLE_PATH */
+
+#ifdef HAVE_BSD_KERN_PROC_SYSCTL
+/*
+ * Resolves the executable path using KERN_PROC_PATHNAME BSD sysctl.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int git_get_exec_path_bsd_sysctl(struct strbuf *buf)
+{
+	int mib[4];
+	char path[MAXPATHLEN];
+	size_t cb = sizeof(path);
+
+	mib[0] = CTL_KERN;
+	mib[1] = KERN_PROC;
+	mib[2] = KERN_PROC_PATHNAME;
+	mib[3] = -1;
+	if (!sysctl(mib, 4, path, &cb, NULL, 0)) {
+		trace_printf(
+			"trace: resolved executable path from sysctl: %s\n",
+			path);
+		strbuf_addstr(buf, path);
+		return 0;
+	}
+	return -1;
+}
+#endif /* HAVE_BSD_KERN_PROC_SYSCTL */
+
+#ifdef HAVE_NS_GET_EXECUTABLE_PATH
+/*
+ * Resolves the executable path by querying Darwin applicaton stack.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int git_get_exec_path_darwin(struct strbuf *buf)
+{
+	char path[PATH_MAX];
+	uint32_t size = sizeof(path);
+	if (!_NSGetExecutablePath(path, &size)) {
+		trace_printf(
+			"trace: resolved executable path from Darwin stack: %s\n",
+			path);
+		strbuf_addstr(buf, path);
+		return 0;
+	}
+	return -1;
+}
+#endif /* HAVE_NS_GET_EXECUTABLE_PATH */
+
+/*
+ * Resolves the absolute path of the current executable.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int git_get_exec_path(struct strbuf *buf, const char *argv0)
+{
+	/*
+	 * Identifying the executable path is operating system specific.
+	 * Selectively employ all available methods in order of preference,
+	 * preferring highly-available authoratative methods over
+	 * selectively-available or non- authoratative methods.
+	 *
+	 * All cases fall back on resolving against argv[0] if there isn't a
+	 * better functional method. However, note that argv[0] can be
+	 * used-supplied on many operating sysetems, and is not authoratative
+	 * in those cases.
+	 *
+	 * Each of these functions returns 0 on success, so evaluation will stop
+	 * after the first successful method.
+	 */
+	if (
+#ifdef HAVE_BSD_KERN_PROC_SYSCTL
+		git_get_exec_path_bsd_sysctl(buf) &&
+#endif /* HAVE_BSD_KERN_PROC_SYSCTL */
+
+#ifdef HAVE_NS_GET_EXECUTABLE_PATH
+		git_get_exec_path_darwin(buf) &&
+#endif /* HAVE_NS_GET_EXECUTABLE_PATH */
+
+#ifdef PROCFS_EXECUTABLE_PATH
+		git_get_exec_path_procfs(buf) &&
+#endif /* PROCFS_EXECUTABLE_PATH */
+
+		git_get_exec_path_from_argv0(buf, argv0)) {
+		return -1;
+	}
 
+	if (strbuf_normalize_path(buf)) {
+		trace_printf("trace: could not normalize path: %s\n", buf->buf);
+		return -1;
+	}
+
+	return 0;
+}
+
+void git_resolve_executable_dir(const char *argv0)
+{
+	struct strbuf buf = STRBUF_INIT;
+	char *resolved;
+	const char *slash;
+
+	if (git_get_exec_path(&buf, argv0)) {
+		trace_printf(
+			"trace: could not determine executable path from: %s\n",
+			argv0);
+		strbuf_release(&buf);
+		return;
+	}
+
+	resolved = strbuf_detach(&buf, NULL);
+	slash = find_last_dir_sep(resolved);
 	if (slash)
-		argv0_path = xstrndup(argv0, slash - argv0);
+		resolved[slash - resolved] = '\0';
+
+	executable_dirname = resolved;
+	trace_printf("trace: resolved executable dir: %s\n",
+		     executable_dirname);
 }
 
 #else
 
+/**
+ * When not using a runtime prefix, Git uses a hard-coded path, and there is
+ * nothing to resolve.
+ */
+
 static const char *system_prefix(void)
 {
 	return PREFIX;
 }
 
-void git_extract_argv0_path(const char *argv0)
+void git_resolve_executable_dir(const char *argv0)
 {
 }
 
@@ -65,32 +244,28 @@ char *system_path(const char *path)
 	return strbuf_detach(&d, NULL);
 }
 
-void git_set_argv_exec_path(const char *exec_path)
+static const char *exec_path_value;
+
+void git_set_exec_path(const char *exec_path)
 {
-	argv_exec_path = exec_path;
+	exec_path_value = exec_path;
 	/*
 	 * Propagate this setting to external programs.
 	 */
 	setenv(EXEC_PATH_ENVIRONMENT, exec_path, 1);
 }
 
-
-/* Returns the highest-priority, location to look for git programs. */
+/* Returns the highest-priority location to look for git programs. */
 const char *git_exec_path(void)
 {
-	static char *cached_exec_path;
-
-	if (argv_exec_path)
-		return argv_exec_path;
-
-	if (!cached_exec_path) {
+	if (!exec_path_value) {
 		const char *env = getenv(EXEC_PATH_ENVIRONMENT);
 		if (env && *env)
-			cached_exec_path = xstrdup(env);
+			exec_path_value = xstrdup(env);
 		else
-			cached_exec_path = system_path(GIT_EXEC_PATH);
+			exec_path_value = system_path(GIT_EXEC_PATH);
 	}
-	return cached_exec_path;
+	return exec_path_value;
 }
 
 static void add_path(struct strbuf *out, const char *path)
@@ -103,10 +278,12 @@ static void add_path(struct strbuf *out, const char *path)
 
 void setup_path(void)
 {
+	const char *exec_path = git_exec_path();
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
 
-	add_path(&new_path, git_exec_path());
+	git_set_exec_path(exec_path);
+	add_path(&new_path, exec_path);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
@@ -125,7 +302,8 @@ const char **prepare_git_cmd(struct argv_array *out, const char **argv)
 	return out->argv;
 }
 
-int execv_git_cmd(const char **argv) {
+int execv_git_cmd(const char **argv)
+{
 	struct argv_array nargv = ARGV_ARRAY_INIT;
 
 	prepare_git_cmd(&nargv, argv);
@@ -140,8 +318,7 @@ int execv_git_cmd(const char **argv) {
 	return -1;
 }
 
-
-int execl_git_cmd(const char *cmd,...)
+int execl_git_cmd(const char *cmd, ...)
 {
 	int argc;
 	const char *argv[MAX_ARGS + 1];
diff --git a/exec_cmd.h b/exec_cmd.h
index ff0b48048..2522453cd 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -3,8 +3,8 @@
 
 struct argv_array;
 
-extern void git_set_argv_exec_path(const char *exec_path);
-extern void git_extract_argv0_path(const char *path);
+extern void git_set_exec_path(const char *exec_path);
+extern void git_resolve_executable_dir(const char *path);
 extern const char *git_exec_path(void);
 extern void setup_path(void);
 extern const char **prepare_git_cmd(struct argv_array *out, const char **argv);
diff --git a/gettext.c b/gettext.c
index db727ea02..6b64d5c2e 100644
--- a/gettext.c
+++ b/gettext.c
@@ -2,7 +2,8 @@
  * Copyright (c) 2010 Ævar Arnfjörð Bjarmason
  */
 
-#include "git-compat-util.h"
+#include "cache.h"
+#include "exec_cmd.h"
 #include "gettext.h"
 #include "strbuf.h"
 #include "utf8.h"
@@ -157,10 +158,11 @@ static void init_gettext_charset(const char *domain)
 
 void git_setup_gettext(void)
 {
-	const char *podir = getenv("GIT_TEXTDOMAINDIR");
+	const char *podir = getenv(GIT_TEXT_DOMAIN_DIR_ENVIRONMENT);
 
 	if (!podir)
-		podir = GIT_LOCALE_PATH;
+		podir = system_path(GIT_LOCALE_PATH);
+
 	bindtextdomain("git", podir);
 	setlocale(LC_MESSAGES, "");
 	setlocale(LC_TIME, "");
diff --git a/git.c b/git.c
index 9e96dd409..dc4cc1419 100644
--- a/git.c
+++ b/git.c
@@ -65,7 +65,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 		 */
 		if (skip_prefix(cmd, "--exec-path", &cmd)) {
 			if (*cmd == '=')
-				git_set_argv_exec_path(cmd + 1);
+				git_set_exec_path(cmd + 1);
 			else {
 				puts(git_exec_path());
 				exit(0);
-- 
2.15.0.448.gf294e3d99a-goog

Re: [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Junio C Hamano]

Dan Jacques <dnj@google.com> writes:

> Enable Git to resolve its own binary location using a variety of
> OS-specific and generic methods, including:
>
> - procfs via "/proc/self/exe" (Linux)
> - _NSGetExecutablePath (Darwin)
> - KERN_PROC_PATHNAME sysctl on BSDs.
> - argv0, if absolute (all, including Windows).
>
> This is used to enable RUNTIME_PREFIX support for non-Windows systems,
> notably Linux and Darwin. When configured with RUNTIME_PREFIX, Git will
> do a best-effort resolution of its executable path and automatically use
> this as its "exec_path" for relative helper and data lookups, unless
> explicitly overridden.
>
> Git's PERL tooling now responds to RUNTIME_PREFIX_PERL. When configured,
> Git's generated PERL scripts resolve the Git library location relative to
> their runtime paths instead of hard-coding them. Structural changes
> were made to Makefile to support selective PERL header generation.

Ævar and Dscho Cc'ed for input on Perl and also for the following
piece from the cover letter of the previous iteration (which is not
in the cover for this iteration--because there ought to be no impact
to Windows unless the new RUNTIME_PREFIX_PERL is used):

    Please note that this patch affects Windows Git builds, since
    the Windows Git project uses RUNTIME_PREFIX to support arbitrary
    installation paths.  Notably, PERL scripts are now always
    installed without MakeMaker (if they weren't before), and
    EXEC_PATH_ENVIRONMENT is preferred by tools instead of
    re-resolving argv[0]. Chromium uses the stock redistributable
    Windows Git package, so I haven't had an opportunity to test
    this patch on that platform.

The rest of the patch kept for reference but I not yet have any
comment on it yet.

Thanks.

> Small incidental formatting cleanup of "exec_cmd.c".
>
> Signed-off-by: Dan Jacques <dnj@google.com>
> ---
>  .gitignore       |   1 +
>  Makefile         |  88 +++++++++++++++++---
>  cache.h          |   1 +
>  common-main.c    |   4 +-
>  config.mak.uname |   7 ++
>  exec_cmd.c       | 239 +++++++++++++++++++++++++++++++++++++++++++++++--------
>  exec_cmd.h       |   4 +-
>  gettext.c        |   8 +-
>  git.c            |   2 +-
>  9 files changed, 304 insertions(+), 50 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 833ef3b0b..89bd7bd8a 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -3,6 +3,7 @@
>  /GIT-LDFLAGS
>  /GIT-PREFIX
>  /GIT-PERL-DEFINES
> +/GIT-PERL-HEADER
>  /GIT-PYTHON-VARS
>  /GIT-SCRIPT-DEFINES
>  /GIT-USER-AGENT
> diff --git a/Makefile b/Makefile
> index ee9d5eb11..6fddb8b8f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -296,7 +296,8 @@ all::
>  # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl).
>  #
>  # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
> -# MakeMaker (e.g. using ActiveState under Cygwin).
> +# MakeMaker (e.g. using ActiveState under Cygwin). NO_PERL_MAKEMAKER is
> +# automatically enabled when using RUNTIME_PREFIX_PERL.
>  #
>  # Define NO_PERL if you do not want Perl scripts or libraries at all.
>  #
> @@ -416,6 +417,16 @@ all::
>  #
>  # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
>  #
> +# Define HAVE_BSD_KERN_PROC_SYSCTL if your platform supports the KERN_PROC BSD
> +# sysctl function.
> +#
> +# Define PROCFS_EXECUTABLE_PATH if your platform mounts a "procfs" filesystem
> +# capable of resolving the path of the current executable. If defined, this
> +# must be the canonical path for the "procfs" current executable path.
> +#
> +# Define HAVE_NS_GET_EXECUTABLE_PATH if your platform supports calling
> +# _NSGetExecutablePath to retrieve the path of the running executable.
> +#
>  # Define HAVE_GETDELIM if your system has the getdelim() function.
>  #
>  # Define PAGER_ENV to a SP separated VAR=VAL pairs to define
> @@ -425,6 +436,16 @@ all::
>  #
>  # to say "export LESS=FRX (and LV=-c) if the environment variable
>  # LESS (and LV) is not set, respectively".
> +#
> +# Define RUNTIME_PREFIX to configure Git to resolve its ancillary tooling and
> +# support files relative to the location of the runtime binary, rather than
> +# hard-coding them into the binary. Git installations built with RUNTIME_PREFIX
> +# can be moved to arbitrary filesystem locations. Users may want to enable
> +# RUNTIME_PREFIX_PERL as well (see below).
> +#
> +# 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. RUNTIME_PREFIX_PERL also sets NO_PERL_MAKEMAKER.
>  
>  GIT-VERSION-FILE: FORCE
>  	@$(SHELL_PATH) ./GIT-VERSION-GEN
> @@ -462,6 +483,7 @@ ARFLAGS = rcs
>  #   mandir
>  #   infodir
>  #   htmldir
> +#   localedir
>  # This can help installing the suite in a relocatable way.
>  
>  prefix = $(HOME)
> @@ -485,6 +507,7 @@ pathsep = :
>  mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
>  infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
>  htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
> +localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))
>  
>  export prefix bindir sharedir sysconfdir gitwebdir localedir
>  
> @@ -1522,9 +1545,6 @@ ifdef SHA1_MAX_BLOCK_SIZE
>  	LIB_OBJS += compat/sha1-chunked.o
>  	BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
>  endif
> -ifdef NO_PERL_MAKEMAKER
> -	export NO_PERL_MAKEMAKER
> -endif
>  ifdef NO_HSTRERROR
>  	COMPAT_CFLAGS += -DNO_HSTRERROR
>  	COMPAT_OBJS += compat/hstrerror.o
> @@ -1549,6 +1569,15 @@ ifdef RUNTIME_PREFIX
>  	COMPAT_CFLAGS += -DRUNTIME_PREFIX
>  endif
>  
> +ifdef RUNTIME_PREFIX_PERL
> +	# Control PERL library location so its paths and contents are not dependent on
> +	# the host's PERL version. See perl/Makefile for more information.
> +	NO_PERL_MAKEMAKER = YesPlease
> +endif
> +ifdef NO_PERL_MAKEMAKER
> +	export NO_PERL_MAKEMAKER
> +endif
> +
>  ifdef NO_PTHREADS
>  	BASIC_CFLAGS += -DNO_PTHREADS
>  else
> @@ -1628,10 +1657,23 @@ ifdef HAVE_BSD_SYSCTL
>  	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
>  endif
>  
> +ifdef HAVE_BSD_KERN_PROC_SYSCTL
> +	BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL
> +endif
> +
>  ifdef HAVE_GETDELIM
>  	BASIC_CFLAGS += -DHAVE_GETDELIM
>  endif
>  
> +ifneq ($(PROCFS_EXECUTABLE_PATH),)
> +	procfs_executable_path_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH))
> +	BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"'
> +endif
> +
> +ifdef HAVE_NS_GET_EXECUTABLE_PATH
> +	BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
> +endif
> +
>  ifeq ($(TCLTK_PATH),)
>  NO_TCLTK = NoThanks
>  endif
> @@ -1714,6 +1756,7 @@ 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))
>  template_dir_SQ = $(subst ','\'',$(template_dir))
>  htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
> @@ -1962,17 +2005,16 @@ 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_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ)
> -$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE
> +
> +$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
>  	$(QUIET_GEN)$(RM) $@ $@+ && \
>  	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
>  	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
>  	INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
>  	sed -e '1{' \
>  	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
> -	    -e '	h' \
> -	    -e '	s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
> -	    -e '	H' \
> -	    -e '	x' \
> +	    -e '	rGIT-PERL-HEADER' \
> +	    -e '	G' \
>  	    -e '}' \
>  	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
>  	    $< >$@+ && \
> @@ -1986,6 +2028,29 @@ GIT-PERL-DEFINES: FORCE
>  		echo "$$FLAGS" >$@; \
>  	    fi
>  
> +GIT-PERL-HEADER: perl/perl.mak GIT-PERL-DEFINES FORCE
> +ifndef RUNTIME_PREFIX_PERL
> +	# Hardcode the runtime path.
> +	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
> +	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
> +	echo \
> +	  'use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));' \
> +	  >$@
> +else
> +	# Probe the runtime path relative to the PERL script. RUNTIME_PREFIX_PERL
> +	# automatically sets NO_PERL_MAKEMAKER, causing PERL scripts to be installed
> +	# to "$(prefix)/lib" (see "perl/Makefile"). This expectation is hard-coded
> +	# into the generated code below.
> +	GITEXECDIR='$(gitexecdir_SQ)' && \
> +	echo \
> +	  'sub _get_git_lib{'\
> +	  'use FindBin;'\
> +	  '(my $$p=$$FindBin::Bin)=~s=/'$${GITEXECDIR}'$$==;'\
> +		'return File::Spec->catdir($$p,"'"lib"'");' \
> +	  '};' \
> +	  'use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB}||_get_git_lib()));'\
> +	  >$@
> +endif
>  
>  .PHONY: gitweb
>  gitweb:
> @@ -2130,6 +2195,7 @@ endif
>  exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX
>  exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
>  	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
> +	'-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \
>  	'-DBINDIR="$(bindir_relative_SQ)"' \
>  	'-DPREFIX="$(prefix_SQ)"'
>  
> @@ -2147,7 +2213,7 @@ attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \
>  
>  gettext.sp gettext.s gettext.o: GIT-PREFIX
>  gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \
> -	-DGIT_LOCALE_PATH='"$(localedir_SQ)"'
> +	-DGIT_LOCALE_PATH='"$(localedir_relative_SQ)"'
>  
>  http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS += \
>  	-DCURL_DISABLE_TYPECHECK
> @@ -2704,7 +2770,7 @@ ifndef NO_TCLTK
>  endif
>  	$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
>  	$(RM) GIT-USER-AGENT GIT-PREFIX
> -	$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS
> +	$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS
>  
>  .PHONY: all install profile-clean clean strip
>  .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
> diff --git a/cache.h b/cache.h
> index cb7fb7c00..fb7795410 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -445,6 +445,7 @@ static inline enum object_type object_type(unsigned int mode)
>  #define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS"
>  #define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH"
>  #define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
> +#define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
>  
>  /*
>   * This environment variable is expected to contain a boolean indicating
> diff --git a/common-main.c b/common-main.c
> index 6a689007e..6516a1f89 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -32,12 +32,12 @@ int main(int argc, const char **argv)
>  	 */
>  	sanitize_stdfds();
>  
> +	git_resolve_executable_dir(argv[0]);
> +
>  	git_setup_gettext();
>  
>  	attr_start();
>  
> -	git_extract_argv0_path(argv[0]);
> -
>  	restore_sigpipe_to_default();
>  
>  	return cmd_main(argc, argv);
> diff --git a/config.mak.uname b/config.mak.uname
> index 685a80d13..58fd62b4d 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -37,6 +37,7 @@ ifeq ($(uname_S),Linux)
>  	HAVE_GETDELIM = YesPlease
>  	SANE_TEXT_GREP=-a
>  	FREAD_READS_DIRECTORIES = UnfortunatelyYes
> +	PROCFS_EXECUTABLE_PATH = /proc/self/exe
>  endif
>  ifeq ($(uname_S),GNU/kFreeBSD)
>  	HAVE_ALLOCA_H = YesPlease
> @@ -111,6 +112,7 @@ ifeq ($(uname_S),Darwin)
>  	BASIC_CFLAGS += -DPROTECT_HFS_DEFAULT=1
>  	HAVE_BSD_SYSCTL = YesPlease
>  	FREAD_READS_DIRECTORIES = UnfortunatelyYes
> +	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
>  endif
>  ifeq ($(uname_S),SunOS)
>  	NEEDS_SOCKET = YesPlease
> @@ -206,6 +208,7 @@ ifeq ($(uname_S),FreeBSD)
>  	HAVE_PATHS_H = YesPlease
>  	GMTIME_UNRELIABLE_ERRORS = UnfortunatelyYes
>  	HAVE_BSD_SYSCTL = YesPlease
> +	HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
>  	PAGER_ENV = LESS=FRX LV=-c MORE=FRX
>  	FREAD_READS_DIRECTORIES = UnfortunatelyYes
>  endif
> @@ -218,6 +221,8 @@ ifeq ($(uname_S),OpenBSD)
>  	BASIC_LDFLAGS += -L/usr/local/lib
>  	HAVE_PATHS_H = YesPlease
>  	HAVE_BSD_SYSCTL = YesPlease
> +	HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
> +	PROCFS_EXECUTABLE_PATH = /proc/curproc/file
>  endif
>  ifeq ($(uname_S),MirBSD)
>  	NO_STRCASESTR = YesPlease
> @@ -236,6 +241,8 @@ ifeq ($(uname_S),NetBSD)
>  	USE_ST_TIMESPEC = YesPlease
>  	HAVE_PATHS_H = YesPlease
>  	HAVE_BSD_SYSCTL = YesPlease
> +	HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
> +	PROCFS_EXECUTABLE_PATH = /proc/curproc/exe
>  endif
>  ifeq ($(uname_S),AIX)
>  	DEFAULT_PAGER = more
> diff --git a/exec_cmd.c b/exec_cmd.c
> index ce192a2d6..c9006c4c9 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -2,53 +2,232 @@
>  #include "exec_cmd.h"
>  #include "quote.h"
>  #include "argv-array.h"
> -#define MAX_ARGS	32
>  
> -static const char *argv_exec_path;
> +#if defined(RUNTIME_PREFIX)
> +
> +#if defined(HAVE_NS_GET_EXECUTABLE_PATH)
> +#include <mach-o/dyld.h>
> +#endif
> +
> +#if defined(HAVE_BSD_KERN_PROC_SYSCTL)
> +#include <sys/param.h>
> +#include <sys/types.h>
> +#include <sys/sysctl.h>
> +#endif
> +
> +#endif /* RUNTIME_PREFIX */
> +
> +#define MAX_ARGS 32
> +
> +static const char *system_prefix(void);
>  
>  #ifdef RUNTIME_PREFIX
> -static const char *argv0_path;
> +
> +/**
> + * When using a runtime prefix, Git dynamically resolves paths relative to its
> + * executable.
> + *
> + * The method for determining the path of the executable is highly
> + * platform-specific.
> + */
> +
> +/**
> + * Path to the current Git executable. Resolved on startup by
> + * 'git_resolve_executable_dir'.
> + */
> +static const char *executable_dirname;
>  
>  static const char *system_prefix(void)
>  {
>  	static const char *prefix;
>  
> -	assert(argv0_path);
> -	assert(is_absolute_path(argv0_path));
> +	assert(executable_dirname);
> +	assert(is_absolute_path(executable_dirname));
>  
>  	if (!prefix &&
> -	    !(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) &&
> -	    !(prefix = strip_path_suffix(argv0_path, BINDIR)) &&
> -	    !(prefix = strip_path_suffix(argv0_path, "git"))) {
> +	    !(prefix = strip_path_suffix(executable_dirname, GIT_EXEC_PATH)) &&
> +	    !(prefix = strip_path_suffix(executable_dirname, BINDIR)) &&
> +	    !(prefix = strip_path_suffix(executable_dirname, "git"))) {
>  		prefix = PREFIX;
>  		trace_printf("RUNTIME_PREFIX requested, "
> -				"but prefix computation failed.  "
> -				"Using static fallback '%s'.\n", prefix);
> +			     "but prefix computation failed.  "
> +			     "Using static fallback '%s'.\n",
> +			     prefix);
>  	}
>  	return prefix;
>  }
>  
> -void git_extract_argv0_path(const char *argv0)
> +/*
> + * Resolves the executable path from argv[0], only if it is absolute.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +static int git_get_exec_path_from_argv0(struct strbuf *buf, const char *argv0)
>  {
>  	const char *slash;
>  
>  	if (!argv0 || !*argv0)
> -		return;
> +		return -1;
>  
>  	slash = find_last_dir_sep(argv0);
> +	if (slash) {
> +		trace_printf("trace: resolved executable path from argv0: %s\n",
> +			     argv0);
> +		strbuf_add_absolute_path(buf, argv0);
> +		return 0;
> +	}
> +	return -1;
> +}
> +
> +#ifdef PROCFS_EXECUTABLE_PATH
> +/*
> + * Resolves the executable path by examining a procfs symlink.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +static int git_get_exec_path_procfs(struct strbuf *buf)
> +{
> +	if (strbuf_realpath(buf, PROCFS_EXECUTABLE_PATH, 0)) {
> +		trace_printf(
> +			"trace: resolved executable path from procfs: %s\n",
> +			buf->buf);
> +		return 0;
> +	}
> +	return -1;
> +}
> +#endif /* PROCFS_EXECUTABLE_PATH */
> +
> +#ifdef HAVE_BSD_KERN_PROC_SYSCTL
> +/*
> + * Resolves the executable path using KERN_PROC_PATHNAME BSD sysctl.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +static int git_get_exec_path_bsd_sysctl(struct strbuf *buf)
> +{
> +	int mib[4];
> +	char path[MAXPATHLEN];
> +	size_t cb = sizeof(path);
> +
> +	mib[0] = CTL_KERN;
> +	mib[1] = KERN_PROC;
> +	mib[2] = KERN_PROC_PATHNAME;
> +	mib[3] = -1;
> +	if (!sysctl(mib, 4, path, &cb, NULL, 0)) {
> +		trace_printf(
> +			"trace: resolved executable path from sysctl: %s\n",
> +			path);
> +		strbuf_addstr(buf, path);
> +		return 0;
> +	}
> +	return -1;
> +}
> +#endif /* HAVE_BSD_KERN_PROC_SYSCTL */
> +
> +#ifdef HAVE_NS_GET_EXECUTABLE_PATH
> +/*
> + * Resolves the executable path by querying Darwin applicaton stack.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +static int git_get_exec_path_darwin(struct strbuf *buf)
> +{
> +	char path[PATH_MAX];
> +	uint32_t size = sizeof(path);
> +	if (!_NSGetExecutablePath(path, &size)) {
> +		trace_printf(
> +			"trace: resolved executable path from Darwin stack: %s\n",
> +			path);
> +		strbuf_addstr(buf, path);
> +		return 0;
> +	}
> +	return -1;
> +}
> +#endif /* HAVE_NS_GET_EXECUTABLE_PATH */
> +
> +/*
> + * Resolves the absolute path of the current executable.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +static int git_get_exec_path(struct strbuf *buf, const char *argv0)
> +{
> +	/*
> +	 * Identifying the executable path is operating system specific.
> +	 * Selectively employ all available methods in order of preference,
> +	 * preferring highly-available authoratative methods over
> +	 * selectively-available or non- authoratative methods.
> +	 *
> +	 * All cases fall back on resolving against argv[0] if there isn't a
> +	 * better functional method. However, note that argv[0] can be
> +	 * used-supplied on many operating sysetems, and is not authoratative
> +	 * in those cases.
> +	 *
> +	 * Each of these functions returns 0 on success, so evaluation will stop
> +	 * after the first successful method.
> +	 */
> +	if (
> +#ifdef HAVE_BSD_KERN_PROC_SYSCTL
> +		git_get_exec_path_bsd_sysctl(buf) &&
> +#endif /* HAVE_BSD_KERN_PROC_SYSCTL */
> +
> +#ifdef HAVE_NS_GET_EXECUTABLE_PATH
> +		git_get_exec_path_darwin(buf) &&
> +#endif /* HAVE_NS_GET_EXECUTABLE_PATH */
> +
> +#ifdef PROCFS_EXECUTABLE_PATH
> +		git_get_exec_path_procfs(buf) &&
> +#endif /* PROCFS_EXECUTABLE_PATH */
> +
> +		git_get_exec_path_from_argv0(buf, argv0)) {
> +		return -1;
> +	}
>  
> +	if (strbuf_normalize_path(buf)) {
> +		trace_printf("trace: could not normalize path: %s\n", buf->buf);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +void git_resolve_executable_dir(const char *argv0)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	char *resolved;
> +	const char *slash;
> +
> +	if (git_get_exec_path(&buf, argv0)) {
> +		trace_printf(
> +			"trace: could not determine executable path from: %s\n",
> +			argv0);
> +		strbuf_release(&buf);
> +		return;
> +	}
> +
> +	resolved = strbuf_detach(&buf, NULL);
> +	slash = find_last_dir_sep(resolved);
>  	if (slash)
> -		argv0_path = xstrndup(argv0, slash - argv0);
> +		resolved[slash - resolved] = '\0';
> +
> +	executable_dirname = resolved;
> +	trace_printf("trace: resolved executable dir: %s\n",
> +		     executable_dirname);
>  }
>  
>  #else
>  
> +/**
> + * When not using a runtime prefix, Git uses a hard-coded path, and there is
> + * nothing to resolve.
> + */
> +
>  static const char *system_prefix(void)
>  {
>  	return PREFIX;
>  }
>  
> -void git_extract_argv0_path(const char *argv0)
> +void git_resolve_executable_dir(const char *argv0)
>  {
>  }
>  
> @@ -65,32 +244,28 @@ char *system_path(const char *path)
>  	return strbuf_detach(&d, NULL);
>  }
>  
> -void git_set_argv_exec_path(const char *exec_path)
> +static const char *exec_path_value;
> +
> +void git_set_exec_path(const char *exec_path)
>  {
> -	argv_exec_path = exec_path;
> +	exec_path_value = exec_path;
>  	/*
>  	 * Propagate this setting to external programs.
>  	 */
>  	setenv(EXEC_PATH_ENVIRONMENT, exec_path, 1);
>  }
>  
> -
> -/* Returns the highest-priority, location to look for git programs. */
> +/* Returns the highest-priority location to look for git programs. */
>  const char *git_exec_path(void)
>  {
> -	static char *cached_exec_path;
> -
> -	if (argv_exec_path)
> -		return argv_exec_path;
> -
> -	if (!cached_exec_path) {
> +	if (!exec_path_value) {
>  		const char *env = getenv(EXEC_PATH_ENVIRONMENT);
>  		if (env && *env)
> -			cached_exec_path = xstrdup(env);
> +			exec_path_value = xstrdup(env);
>  		else
> -			cached_exec_path = system_path(GIT_EXEC_PATH);
> +			exec_path_value = system_path(GIT_EXEC_PATH);
>  	}
> -	return cached_exec_path;
> +	return exec_path_value;
>  }
>  
>  static void add_path(struct strbuf *out, const char *path)
> @@ -103,10 +278,12 @@ static void add_path(struct strbuf *out, const char *path)
>  
>  void setup_path(void)
>  {
> +	const char *exec_path = git_exec_path();
>  	const char *old_path = getenv("PATH");
>  	struct strbuf new_path = STRBUF_INIT;
>  
> -	add_path(&new_path, git_exec_path());
> +	git_set_exec_path(exec_path);
> +	add_path(&new_path, exec_path);
>  
>  	if (old_path)
>  		strbuf_addstr(&new_path, old_path);
> @@ -125,7 +302,8 @@ const char **prepare_git_cmd(struct argv_array *out, const char **argv)
>  	return out->argv;
>  }
>  
> -int execv_git_cmd(const char **argv) {
> +int execv_git_cmd(const char **argv)
> +{
>  	struct argv_array nargv = ARGV_ARRAY_INIT;
>  
>  	prepare_git_cmd(&nargv, argv);
> @@ -140,8 +318,7 @@ int execv_git_cmd(const char **argv) {
>  	return -1;
>  }
>  
> -
> -int execl_git_cmd(const char *cmd,...)
> +int execl_git_cmd(const char *cmd, ...)
>  {
>  	int argc;
>  	const char *argv[MAX_ARGS + 1];
> diff --git a/exec_cmd.h b/exec_cmd.h
> index ff0b48048..2522453cd 100644
> --- a/exec_cmd.h
> +++ b/exec_cmd.h
> @@ -3,8 +3,8 @@
>  
>  struct argv_array;
>  
> -extern void git_set_argv_exec_path(const char *exec_path);
> -extern void git_extract_argv0_path(const char *path);
> +extern void git_set_exec_path(const char *exec_path);
> +extern void git_resolve_executable_dir(const char *path);
>  extern const char *git_exec_path(void);
>  extern void setup_path(void);
>  extern const char **prepare_git_cmd(struct argv_array *out, const char **argv);
> diff --git a/gettext.c b/gettext.c
> index db727ea02..6b64d5c2e 100644
> --- a/gettext.c
> +++ b/gettext.c
> @@ -2,7 +2,8 @@
>   * Copyright (c) 2010 Ævar Arnfjörð Bjarmason
>   */
>  
> -#include "git-compat-util.h"
> +#include "cache.h"
> +#include "exec_cmd.h"
>  #include "gettext.h"
>  #include "strbuf.h"
>  #include "utf8.h"
> @@ -157,10 +158,11 @@ static void init_gettext_charset(const char *domain)
>  
>  void git_setup_gettext(void)
>  {
> -	const char *podir = getenv("GIT_TEXTDOMAINDIR");
> +	const char *podir = getenv(GIT_TEXT_DOMAIN_DIR_ENVIRONMENT);
>  
>  	if (!podir)
> -		podir = GIT_LOCALE_PATH;
> +		podir = system_path(GIT_LOCALE_PATH);
> +
>  	bindtextdomain("git", podir);
>  	setlocale(LC_MESSAGES, "");
>  	setlocale(LC_TIME, "");
> diff --git a/git.c b/git.c
> index 9e96dd409..dc4cc1419 100644
> --- a/git.c
> +++ b/git.c
> @@ -65,7 +65,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>  		 */
>  		if (skip_prefix(cmd, "--exec-path", &cmd)) {
>  			if (*cmd == '=')
> -				git_set_argv_exec_path(cmd + 1);
> +				git_set_exec_path(cmd + 1);
>  			else {
>  				puts(git_exec_path());
>  				exit(0);

Re: [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Ævar Arnfjörð Bjarmason]

On Sun, Nov 19 2017, Dan Jacques jotted:

> [...]

Firstly the promise of this is very neat. I'm happy to offer any help I
can give.

> Enable Git to resolve its own binary location using a variety of
> OS-specific and generic methods, including:
>
> - procfs via "/proc/self/exe" (Linux)
> - _NSGetExecutablePath (Darwin)
> - KERN_PROC_PATHNAME sysctl on BSDs.
> - argv0, if absolute (all, including Windows).
>
> This is used to enable RUNTIME_PREFIX support for non-Windows systems,
> notably Linux and Darwin. When configured with RUNTIME_PREFIX, Git will
> do a best-effort resolution of its executable path and automatically use
> this as its "exec_path" for relative helper and data lookups, unless
> explicitly overridden.
>
> Git's PERL tooling now responds to RUNTIME_PREFIX_PERL. When configured,
> Git's generated PERL scripts resolve the Git library location relative to
> their runtime paths instead of hard-coding them. Structural changes
> were made to Makefile to support selective PERL header generation.
>
> Small incidental formatting cleanup of "exec_cmd.c".

> +$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
>  	$(QUIET_GEN)$(RM) $@ $@+ && \
>  	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
>  	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
>  	INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
>  	sed -e '1{' \
>  	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
> -	    -e '	h' \
> -	    -e '	s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
> -	    -e '	H' \
> -	    -e '	x' \
> +	    -e '	rGIT-PERL-HEADER' \
> +	    -e '	G' \
>  	    -e '}' \
>  	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
>  	    $< >$@+ && \
> @@ -1986,6 +2028,29 @@ GIT-PERL-DEFINES: FORCE
>  		echo "$$FLAGS" >$@; \
>  	    fi
>
> +GIT-PERL-HEADER: perl/perl.mak GIT-PERL-DEFINES FORCE
> +ifndef RUNTIME_PREFIX_PERL
> +	# Hardcode the runtime path.
> +	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
> +	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
> +	echo \
> +	  'use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));' \
> +	  >$@
> +else
> +	# Probe the runtime path relative to the PERL script. RUNTIME_PREFIX_PERL
> +	# automatically sets NO_PERL_MAKEMAKER, causing PERL scripts to be installed
> +	# to "$(prefix)/lib" (see "perl/Makefile"). This expectation is hard-coded
> +	# into the generated code below.
> +	GITEXECDIR='$(gitexecdir_SQ)' && \
> +	echo \
> +	  'sub _get_git_lib{'\
> +	  'use FindBin;'\
> +	  '(my $$p=$$FindBin::Bin)=~s=/'$${GITEXECDIR}'$$==;'\
> +		'return File::Spec->catdir($$p,"'"lib"'");' \
> +	  '};' \
> +	  'use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB}||_get_git_lib()));'\
> +	  >$@
> +endif

If you run run:

    make -j8 prefix=/tmp/git RUNTIME_PREFIX=YesPlease RUNTIME_PREFIX_PERL= CFLAGS="-O0 -g" all install

And then:

    make -j8 prefix=/tmp/git RUNTIME_PREFIX=YesPlease RUNTIME_PREFIX_PERL=YesPlease CFLAGS="-O0 -g" all install

You end up with this:

    $ tree /tmp/git/{lib,share/perl}
    /tmp/git/lib
    └── x86_64-linux-gnu
        └── perl
            └── 5.26.1
                ├── auto
                │   └── Git
                └── perllocal.pod
    /tmp/git/share/perl
    └── 5.26.1
        [...]
        └── Git.pm

You need to bust the perl/PM.stamp cache as a function of your
RUNTIME_PREFIX_PERL variable (or NO_PERL_MAKEMAKER).

Other than that, is this whole NO_PERL_MAKEMAKER workaround just because
you couldn't figure out what the target RELPERLPATH is in
$prefix/$RELPERLPATH, which in this case is share/perl/5.26.1 ?

I don't remember offhand how to extract that, but htis is built into
perl itself, see e.g.:

    $ PERL5LIB= /usr/bin/perl -E 'say for grep { m[share|lib] } @INC'
    /usr/local/lib/x86_64-linux-gnu/perl/5.26.1
    /usr/local/share/perl/5.26.1
    /usr/lib/x86_64-linux-gnu/perl5/5.26
    /usr/share/perl5
    /usr/lib/x86_64-linux-gnu/perl/5.26
    /usr/share/perl/5.26
    /usr/local/lib/site_perl
    /usr/lib/x86_64-linux-gnu/perl-base

So it's in Config.pm somewhere IIRC. But your patch doesn't discuss why
you toggled NO_PERL_MAKEMAKER, is it purely to work around this issue,
and if we had some aesy way to figure out the target $RELPERLPATH we
wouldn't need to do that?

Seems a bit of baby & bathwater there :)

Aside from that:

1. The regex match you're doing to munge the dir could be done as a
   catdir($orig, '..', '..', 'lib'), that doesn't work as discussed
   above, but *might* be more portable. I say might because I don't know
   if the path string is always normalized to be unix-like, but if not
   this won't work e.g on Windows where it'll have \ not /.

2. You are 'use'-ing FindBin there unconditionally (use is not function
   local in perl), and implicitly assuming it loads File::Spec.

   Ignoring the NO_PERL_MAKEMAKER caveats above I'd suggest something
   like this:

       #!/usr/bin/perl

       # BEGIN RUNTIME_PREFIX_PERL=YesPlease generated code.
       #
       # This finds our Git::* libraries at relative locations.
       BEGIN {
           use lib split /:/,
           (
               $ENV{GITPERLLIB}
               ||
               do {
                   require FindBin;
                   require File::Spec;
                   File::Spec->catdir($FindBin::Bin, '..', '..', 'lib');
               }
           );
       }
       # END RUNTIME_PREFIX_PERL=YesPlease generated code.

       # Copyright (C) 2006, Eric Wong <normalperson@yhbt.net>
       # License: GPL v2 or later

   It's also nice to have some whitespace / comments to note that this
   is generated code.

 3. I may be squinting at this wrong but it seems to me that between
    your v1 and v2 reading GITPERLLIB here no longer makes any sense at
    all. You used to set it in git itself, now it takes priority but
    nothing sets it, presumably you'd have some external wrapper script
    that'll set it?

    Now if I compile with RUNTIME_PREFIX=YesPlease I get magic
    auto-discovery of C program paths, right? But it'll still fallback
    to the system perl libs (if any) unless
    RUNTIME_PREFIX_PERL=YesPlease is set. Shouldn't we just make
    RUNTIME_PREFIX=YesPlease Just Work for everything?

Re: [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 20 2017, Ævar Arnfjörð Bjarmason jotted:

> On Sun, Nov 19 2017, Dan Jacques jotted:
>
>> [...]
>
> Firstly the promise of this is very neat. I'm happy to offer any help I
> can give.
>
>> Enable Git to resolve its own binary location using a variety of
>> OS-specific and generic methods, including:
>>
>> - procfs via "/proc/self/exe" (Linux)
>> - _NSGetExecutablePath (Darwin)
>> - KERN_PROC_PATHNAME sysctl on BSDs.
>> - argv0, if absolute (all, including Windows).
>>
>> This is used to enable RUNTIME_PREFIX support for non-Windows systems,
>> notably Linux and Darwin. When configured with RUNTIME_PREFIX, Git will
>> do a best-effort resolution of its executable path and automatically use
>> this as its "exec_path" for relative helper and data lookups, unless
>> explicitly overridden.
>>
>> Git's PERL tooling now responds to RUNTIME_PREFIX_PERL. When configured,
>> Git's generated PERL scripts resolve the Git library location relative to
>> their runtime paths instead of hard-coding them. Structural changes
>> were made to Makefile to support selective PERL header generation.
>>
>> Small incidental formatting cleanup of "exec_cmd.c".
>
>> +$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
>>  	$(QUIET_GEN)$(RM) $@ $@+ && \
>>  	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
>>  	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
>>  	INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
>>  	sed -e '1{' \
>>  	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
>> -	    -e '	h' \
>> -	    -e '	s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
>> -	    -e '	H' \
>> -	    -e '	x' \
>> +	    -e '	rGIT-PERL-HEADER' \
>> +	    -e '	G' \
>>  	    -e '}' \
>>  	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
>>  	    $< >$@+ && \
>> @@ -1986,6 +2028,29 @@ GIT-PERL-DEFINES: FORCE
>>  		echo "$$FLAGS" >$@; \
>>  	    fi
>>
>> +GIT-PERL-HEADER: perl/perl.mak GIT-PERL-DEFINES FORCE
>> +ifndef RUNTIME_PREFIX_PERL
>> +	# Hardcode the runtime path.
>> +	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
>> +	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
>> +	echo \
>> +	  'use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));' \
>> +	  >$@
>> +else
>> +	# Probe the runtime path relative to the PERL script. RUNTIME_PREFIX_PERL
>> +	# automatically sets NO_PERL_MAKEMAKER, causing PERL scripts to be installed
>> +	# to "$(prefix)/lib" (see "perl/Makefile"). This expectation is hard-coded
>> +	# into the generated code below.
>> +	GITEXECDIR='$(gitexecdir_SQ)' && \
>> +	echo \
>> +	  'sub _get_git_lib{'\
>> +	  'use FindBin;'\
>> +	  '(my $$p=$$FindBin::Bin)=~s=/'$${GITEXECDIR}'$$==;'\
>> +		'return File::Spec->catdir($$p,"'"lib"'");' \
>> +	  '};' \
>> +	  'use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB}||_get_git_lib()));'\
>> +	  >$@
>> +endif
>
> If you run run:
>
>     make -j8 prefix=/tmp/git RUNTIME_PREFIX=YesPlease RUNTIME_PREFIX_PERL= CFLAGS="-O0 -g" all install
>
> And then:
>
>     make -j8 prefix=/tmp/git RUNTIME_PREFIX=YesPlease RUNTIME_PREFIX_PERL=YesPlease CFLAGS="-O0 -g" all install
>
> You end up with this:
>
>     $ tree /tmp/git/{lib,share/perl}
>     /tmp/git/lib
>     └── x86_64-linux-gnu
>         └── perl
>             └── 5.26.1
>                 ├── auto
>                 │ └── Git
>                 └── perllocal.pod
>     /tmp/git/share/perl
>     └── 5.26.1
>         [...]
>         └── Git.pm
>
> You need to bust the perl/PM.stamp cache as a function of your
> RUNTIME_PREFIX_PERL variable (or NO_PERL_MAKEMAKER).
>
> Other than that, is this whole NO_PERL_MAKEMAKER workaround just because
> you couldn't figure out what the target RELPERLPATH is in
> $prefix/$RELPERLPATH, which in this case is share/perl/5.26.1 ?
>
> I don't remember offhand how to extract that, but htis is built into
> perl itself, see e.g.:
>
>     $ PERL5LIB= /usr/bin/perl -E 'say for grep { m[share|lib] } @INC'
>     /usr/local/lib/x86_64-linux-gnu/perl/5.26.1
>     /usr/local/share/perl/5.26.1
>     /usr/lib/x86_64-linux-gnu/perl5/5.26
>     /usr/share/perl5
>     /usr/lib/x86_64-linux-gnu/perl/5.26
>     /usr/share/perl/5.26
>     /usr/local/lib/site_perl
>     /usr/lib/x86_64-linux-gnu/perl-base
>
> So it's in Config.pm somewhere IIRC. But your patch doesn't discuss why
> you toggled NO_PERL_MAKEMAKER, is it purely to work around this issue,
> and if we had some aesy way to figure out the target $RELPERLPATH we
> wouldn't need to do that?
>
> Seems a bit of baby & bathwater there :)

So LeonT over at #p5p helped me with this. He believes this'll work
(unless MakeMaker INSTALL_BASE is set, but that should break the Git
install anyway):


    /usr/bin/perl -MConfig -wE 'my ($relsite) = $Config{installsitelib} =~ m[^\Q$Config{siteprefixexp}\E/(.+)]s; say $relsite'
    share/perl/5.26.1

I.e. aside from my spiel below injecting this should work, and would
eliminate the need to toggle NO_PERL_MAKEMAKER (untested):

    BEGIN {
        use lib split /:/,
        (
            $ENV{GITPERLLIB}
            ||
            do {
                require FindBin;
                require File::Spec;
                require Config;
                Config->import;

                my ($relsite) = $Config{installsitelib} =~ m[^\Q$Config{siteprefixexp}\E/(.+)]s
                    or die "PANIC: Ohes noes $Config{siteprefixexp} doesn't match a subset of $Config{installsitelib}";
                File::Spec->catdir($FindBin::Bin, '..', '..', $relsite);
            }
        );
    }


> Aside from that:
>
> 1. The regex match you're doing to munge the dir could be done as a
>    catdir($orig, '..', '..', 'lib'), that doesn't work as discussed
>    above, but *might* be more portable. I say might because I don't know
>    if the path string is always normalized to be unix-like, but if not
>    this won't work e.g on Windows where it'll have \ not /.
>
> 2. You are 'use'-ing FindBin there unconditionally (use is not function
>    local in perl), and implicitly assuming it loads File::Spec.
>
>    Ignoring the NO_PERL_MAKEMAKER caveats above I'd suggest something
>    like this:
>
>        #!/usr/bin/perl
>
>        # BEGIN RUNTIME_PREFIX_PERL=YesPlease generated code.
>        #
>        # This finds our Git::* libraries at relative locations.
>        BEGIN {
>            use lib split /:/,
>            (
>                $ENV{GITPERLLIB}
>                ||
>                do {
>                    require FindBin;
>                    require File::Spec;
>                    File::Spec->catdir($FindBin::Bin, '..', '..', 'lib');
>                }
>            );
>        }
>        # END RUNTIME_PREFIX_PERL=YesPlease generated code.
>
>        # Copyright (C) 2006, Eric Wong <normalperson@yhbt.net>
>        # License: GPL v2 or later
>
>    It's also nice to have some whitespace / comments to note that this
>    is generated code.
>
>  3. I may be squinting at this wrong but it seems to me that between
>     your v1 and v2 reading GITPERLLIB here no longer makes any sense at
>     all. You used to set it in git itself, now it takes priority but
>     nothing sets it, presumably you'd have some external wrapper script
>     that'll set it?
>
>     Now if I compile with RUNTIME_PREFIX=YesPlease I get magic
>     auto-discovery of C program paths, right? But it'll still fallback
>     to the system perl libs (if any) unless
>     RUNTIME_PREFIX_PERL=YesPlease is set. Shouldn't we just make
>     RUNTIME_PREFIX=YesPlease Just Work for everything?

Re(2): [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Dan Jacques]

On Mon, 20 Nov 2017 22:00:10, Ævar Arnfjörð Bjarmason replied:

> [...]

Thanks for responding. I'll readily confess that PERL and the PERL
ecosystem are not areas I'm very familiar with, so I'm really grateful
for your feedback here.

> You need to bust the perl/PM.stamp cache as a function of your
> RUNTIME_PREFIX_PERL variable (or NO_PERL_MAKEMAKER).

Good catch, I'll add that to the next version of the patch.

> Other than that, is this whole NO_PERL_MAKEMAKER workaround just because
> you couldn't figure out what the target RELPERLPATH is in
> $prefix/$RELPERLPATH, which in this case is share/perl/5.26.1 ?

Sort of. I actually had a version set hacked MakeMaker's $INSTALLSITELIB
(same general result) and it worked.

The executable path resolution logic that I built make assumptions about
the layout of the installation, and I ended up favoring NO_PERL_MAKEMAKER
because I could rely on its explicit and straightforward installation
logic.

I am not sure that MakeMaker adds value in a RUNTIME_PREFIX
installation, since RUNTIME_PREFIX's motivation is fundamentally portable
and my impression of MakeMaker is that much of the value that it adds is
handling system-specific PERL installation details. Given that
NO_PERL_MAKEMAKER is supported as a Git installation option, I opted to
take advantage of the explicit installation rather than rely on
MakeMaker as an intermediary.

My other motivation is that if I integrate $RELPERLPATH into the MakeMaker
installation, I'd still have to implement that behavior when
NO_PERL_MAKEMAMER is enabled so that it is compatible with
RUNTIME_PREFIX_PERL.

I'd welcome any insight on whether this is the correct way to proceed.
If we decode to go forward with NO_PERL_MAKEMAKER, I'm happy to add some
better documentation in the Makefile to detail the rationale for
forcefully enabling it.

> 1. The regex match you're doing to munge the dir could be done as a
>    catdir($orig, '..', '..', 'lib'), that doesn't work as discussed
>    above, but *might* be more portable. I say might because I don't know
>    if the path string is always normalized to be unix-like, but if not
>    this won't work e.g on Windows where it'll have \ not /.

The regex-based approach was motivated by a perceived value to
conciseness. Since a larger header code block seems to be acceptable, I
could emit "$(gitexecdir)" as a constant in the header and operate on it
at runtime. This would avoid having to calculate the correct sequence of
"../" to walk up from "$(gitexecdir)" directly in the Makefile.

> 2. You are 'use'-ing FindBin there unconditionally (use is not function
>    local in perl), and implicitly assuming it loads File::Spec.
>
>    Ignoring the NO_PERL_MAKEMAKER caveats above I'd suggest something
>    like this:

Sounds good! I wasn't sure whether I should adhere to the one-line header
that was being used before, but I am all for whitespace if that is
acceptable.

This seems a bit much to emit from a Makefile - what do you think about
adding template files for each header in the "perl/" directory and
preprocessing them via "sed" in the Makefile?

> 3. I may be squinting at this wrong but it seems to me that between
>    your v1 and v2 reading GITPERLLIB here no longer makes any sense at
>    all. You used to set it in git itself, now it takes priority but
>    nothing sets it, presumably you'd have some external wrapper script
>    that'll set it?

$GITPERLLIB is (as far as I can tell) used for testing, so that PERL scripts
invoked by tests can find the Git support libraries. I think it is still
necessary in both RUNTIME_PREFIX_PERL and default worlds because tests run
Git out of the repository root, so relative resolution logic based on
installation paths will not work.

Thanks again!
-Dan

Re: Re(2): [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 20 2017, Dan Jacques jotted:

> On Mon, 20 Nov 2017 22:00:10, Ævar Arnfjörð Bjarmason replied:
>
>> [...]
>
> Thanks for responding. I'll readily confess that PERL and the PERL
> ecosystem are not areas I'm very familiar with, so I'm really grateful
> for your feedback here.
>
>> You need to bust the perl/PM.stamp cache as a function of your
>> RUNTIME_PREFIX_PERL variable (or NO_PERL_MAKEMAKER).
>
> Good catch, I'll add that to the next version of the patch.
>
>> Other than that, is this whole NO_PERL_MAKEMAKER workaround just because
>> you couldn't figure out what the target RELPERLPATH is in
>> $prefix/$RELPERLPATH, which in this case is share/perl/5.26.1 ?
>
> Sort of. I actually had a version set hacked MakeMaker's $INSTALLSITELIB
> (same general result) and it worked.
>
> The executable path resolution logic that I built make assumptions about
> the layout of the installation, and I ended up favoring NO_PERL_MAKEMAKER
> because I could rely on its explicit and straightforward installation
> logic.
>
> I am not sure that MakeMaker adds value in a RUNTIME_PREFIX
> installation, since RUNTIME_PREFIX's motivation is fundamentally portable
> and my impression of MakeMaker is that much of the value that it adds is
> handling system-specific PERL installation details. Given that
> NO_PERL_MAKEMAKER is supported as a Git installation option, I opted to
> take advantage of the explicit installation rather than rely on
> MakeMaker as an intermediary.
>
> My other motivation is that if I integrate $RELPERLPATH into the MakeMaker
> installation, I'd still have to implement that behavior when
> NO_PERL_MAKEMAMER is enabled so that it is compatible with
> RUNTIME_PREFIX_PERL.

Right, it needs some if/else, or we could simply always add lib/ as well
to @INC (via "use lib") and it would work in both modes unconditionally.

> I'd welcome any insight on whether this is the correct way to proceed.
> If we decode to go forward with NO_PERL_MAKEMAKER, I'm happy to add some
> better documentation in the Makefile to detail the rationale for
> forcefully enabling it.

My impression is (see `git log --reverse -p -GNO_PERL_MAKEMAKER`) that
almost nobody uses NO_PERL_MAKEMAKER, so it would be better to make it
work with the more supported mode since it seems easy.

But I've now forgotten all the details of why we're even using MakeMaker
in the first place, if I ever actually knew them.

>> 1. The regex match you're doing to munge the dir could be done as a
>>    catdir($orig, '..', '..', 'lib'), that doesn't work as discussed
>>    above, but *might* be more portable. I say might because I don't know
>>    if the path string is always normalized to be unix-like, but if not
>>    this won't work e.g on Windows where it'll have \ not /.
>
> The regex-based approach was motivated by a perceived value to
> conciseness. Since a larger header code block seems to be acceptable, I
> could emit "$(gitexecdir)" as a constant in the header and operate on it
> at runtime. This would avoid having to calculate the correct sequence of
> "../" to walk up from "$(gitexecdir)" directly in the Makefile.

Yeah I think nobody's going to have any problem with a large header
block.

>> 2. You are 'use'-ing FindBin there unconditionally (use is not function
>>    local in perl), and implicitly assuming it loads File::Spec.
>>
>>    Ignoring the NO_PERL_MAKEMAKER caveats above I'd suggest something
>>    like this:
>
> Sounds good! I wasn't sure whether I should adhere to the one-line header
> that was being used before, but I am all for whitespace if that is
> acceptable.
>
> This seems a bit much to emit from a Makefile - what do you think about
> adding template files for each header in the "perl/" directory and
> preprocessing them via "sed" in the Makefile?

Sure, that sounds great.

>> 3. I may be squinting at this wrong but it seems to me that between
>>    your v1 and v2 reading GITPERLLIB here no longer makes any sense at
>>    all. You used to set it in git itself, now it takes priority but
>>    nothing sets it, presumably you'd have some external wrapper script
>>    that'll set it?
>
> $GITPERLLIB is (as far as I can tell) used for testing, so that PERL scripts
> invoked by tests can find the Git support libraries. I think it is still
> necessary in both RUNTIME_PREFIX_PERL and default worlds because tests run
> Git out of the repository root, so relative resolution logic based on
> installation paths will not work.

Ah, thanks. Makes sense, I missed that.

Re [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Dan Jacques]

Ævar Arnfjörð Bjarmason @ 2017-11-20 21:50 UTC suggested:

> So LeonT over at #p5p helped me with this. He believes this'll work
> (unless MakeMaker INSTALL_BASE is set, but that should break the Git
> install anyway):

I think that the problem with this approach is that it uses the local
"Config" module. The primary purpose of RUNTIME_PREFIX(_PERL) is that one
can build/install Git into a directory, then either move that directory
somewhere else or archive it and put it on a different (binary-compatible)
system altogether.

The latter case concerns me here. If the "Config" module is loading local
system paths, then the relative pathing between "$Config{installsitelib}"
and "$Config{siteprefixexp}" may not be consistent between systems, so an
archive built from one system may not have a compatible relative
directory structure when resolved with the Config module on another
system.

Since we control the installation process and paths, we know that the
directory structure looks someting like:

.../prefix/$GITEXECDIR/git-perl-script
.../prefix/$RELPERLPATH/Git.pm

Our goal is to, given the directory that "git-perl-script" belongs to,
first identify the path for ".../prefix" and then append "$RELPERLPATH" to
it to generate the full library path.

The most straightforward way to do this, to me, is to:

1) Have the Makefile hard-code "$RELPERLPATH" and "$GITEXECDIR" (relative
  paths) into the header code.
2) Assert that "$FindBin::Bin" (the directory containing the script) ends
  with "$GITEXECDIR".
3) Remove "$GITEXECDIR" from the end of "$FindBin::Bin" to obtain
  ".../prefix" ($prefix). Simple string truncation is probably fine for
  this.
4) Add "File::Spec->catdir($prefix, $RELPERLPATH)" to "lib".

I don't think path separators are a problem, since the Makefile uses "/"
indiscriminately. Even Git-for-Windows seems to run its PERL scripts in
a POSIX environment (mingw).

Does this sound like a reasonable way to proceed?

Re: Re [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Ævar Arnfjörð Bjarmason]

On Tue, Nov 21 2017, Dan Jacques jotted:

> Ævar Arnfjörð Bjarmason @ 2017-11-20 21:50 UTC suggested:
>
>> So LeonT over at #p5p helped me with this. He believes this'll work
>> (unless MakeMaker INSTALL_BASE is set, but that should break the Git
>> install anyway):
>
> I think that the problem with this approach is that it uses the local
> "Config" module. The primary purpose of RUNTIME_PREFIX(_PERL) is that one
> can build/install Git into a directory, then either move that directory
> somewhere else or archive it and put it on a different (binary-compatible)
> system altogether.
>
> The latter case concerns me here. If the "Config" module is loading local
> system paths, then the relative pathing between "$Config{installsitelib}"
> and "$Config{siteprefixexp}" may not be consistent between systems, so an
> archive built from one system may not have a compatible relative
> directory structure when resolved with the Config module on another
> system.

I don't see how this is different from any other option we build git
with. When we dynamically link to e.g. PCRE under RUNTIME_PREFIX*=Yes
you can move the installed git from /tmp/git to /tmp/git2, but it'll
still expect the specific version of the *.so libraries to be in
/usr/lib or whatever.

Similarly we under the default MakeMaker path add the perl version to
the directories we have, you can move git from /tmp/git to /tmp/git2 no
problem, since that won't change the perl version, but if you upgrade
the global perl itself from 5.20 to 5.24 you'll need to re-build.

> Since we control the installation process and paths, we know that the
> directory structure looks someting like:
>
> .../prefix/$GITEXECDIR/git-perl-script
> .../prefix/$RELPERLPATH/Git.pm
>

Having said the above, I don't understand why we're using MakeMaker at
all and I think we should just get rid of it.

We're not using the perldoc toolchain to build manpages from *.pod for
these, and we don't have any C bindings, it seems to me that we could
simply replace this whole thing with a removal of all things Make-y from
perl/* and just do the minor work of creating a top-levle git-perl-lib
in our install $PREFIX and make the perl stuff use that.

Looking at the history of the Makefile.PL it originally had XS stuff
(which you'd need a Makefile.PL for), but this was removed in 18b0fc1ce1
before it made it to master.

My comment on your patch series was just that with the method I posted
there's no reason for why RUNTIME_PREFIX*=Yes and MakeMaker need to be
mutually exclusive, so if we're keeping the MakeMaker it seems to me
that we can support both.

But we can probably just get rid of MakeMaker.

> Our goal is to, given the directory that "git-perl-script" belongs to,
> first identify the path for ".../prefix" and then append "$RELPERLPATH" to
> it to generate the full library path.
>
> The most straightforward way to do this, to me, is to:
>
> 1) Have the Makefile hard-code "$RELPERLPATH" and "$GITEXECDIR" (relative
>   paths) into the header code.
> 2) Assert that "$FindBin::Bin" (the directory containing the script) ends
>   with "$GITEXECDIR".
> 3) Remove "$GITEXECDIR" from the end of "$FindBin::Bin" to obtain
>   ".../prefix" ($prefix). Simple string truncation is probably fine for
>   this.
> 4) Add "File::Spec->catdir($prefix, $RELPERLPATH)" to "lib".
>
> I don't think path separators are a problem, since the Makefile uses "/"
> indiscriminately. Even Git-for-Windows seems to run its PERL scripts in
> a POSIX environment (mingw).

Right. I don't know that they are either, it just stuck me as an odd
inconsistency that you're going out of your way to do catdir($p, "lib")
in one place and then hardcoding unix-like paths in another place.

If it's not neede dwe can just do "$p/lib".

> Does this sound like a reasonable way to proceed?

I think the best way to proceed is probalby to just start getting rid of
the perl/* make stuff as discussed above.

Is that something you're interested in / have time for, otherwise I
could see if I could find some time, but I don't have a lot these days.

Which is not to say that I think that should block this patch series or
anything. If it really needs to disable MakeMaker to work we're no worse
off than before.

Re [PATCH 1/1] exec_cmd: RUNTIME_PREFIX on some POSIX systems

[Dan Jacques]

On Wed, 22 Nov 2017, Ævar Arnfjörð Bjarmason wrote:

> > The latter case concerns me here. If the "Config" module is loading local
> > system paths, then the relative pathing between "$Config{installsitelib}"
> > and "$Config{siteprefixexp}" may not be consistent between systems, so an
> > archive built from one system may not have a compatible relative
> > directory structure when resolved with the Config module on another
> > system.
>
> I don't see how this is different from any other option we build git
> with. When we dynamically link to e.g. PCRE under RUNTIME_PREFIX*=Yes
> you can move the installed git from /tmp/git to /tmp/git2, but it'll
> still expect the specific version of the *.so libraries to be in
> /usr/lib or whatever.

PCRE brings up a good point. When I build Git for Chromium, I am actually
statically linking in PCRE and most other runtime dependencies, so this
is the sort of portability that I am targetting. The set of configuration
options for full static linking, however, are too use-case-specific to build
into Git, so this patch set merely seeks to fundamentally enable
relocatability as an option for users to customize as they need.

> Similarly we under the default MakeMaker path add the perl version to
> the directories we have, you can move git from /tmp/git to /tmp/git2 no
> problem, since that won't change the perl version, but if you upgrade
> the global perl itself from 5.20 to 5.24 you'll need to re-build.

As you note below, the Git PERL code below, doesn't include any compiled
binary files. As far as I can tell, it's highly portable, with no real
dependencies outside of PERL 5. To that end, I very much do want to
transplant a Git distribution built on Machine-A w/ PERL 5.20 and dump it
onto Machine-B w/ PERL 5.24 and have things work.

My inclination is to follow your previous advice and export an installation
path to MakeMaker. I found that "INSTALLSITELIB" did what I wanted.

My working version exports a version-neutral installation path,
"$(prefix)/share/perl5", to MakeMaker. I can then hard-code that relative
path into the PERL header and use it for resolution, similar to what my
previous patch is doing. NO_PERL_MAKEMAKER emitted a neutral library path,
"$(prefix)/lib", and that was one of the things that made it an appealing
option in my previous patch, but prescribing the path also works fine.

At the end of the day, we're hard-coding "instlibdir" into the generated
header, so it doesn't *really* matter if we use "$(prefix)/share/perl5/5.2.4"
or "$(prefix)/share/perl5"; however, as a matter of preference, I would like
to avoid naming the build system's PERL version into the deployable bundle
when RUNTIME_PREFIX_PERL is set, so I'd opt for the latter.

> [...]
> If it's not neede dwe can just do "$p/lib".

ATM my working code does (pseudocode and filling in values):

1) $gitexecpath = "libexec/git-core";
2) $perllibpath = "share/perl5"
3) Assert that $FindBin::Bin ends with $gitexecpath (or die).
4) Strip $gitexecpath, result stored as $prefix.
5) Use File->concat($prefix, $perllibpath);

This behaves similarly to how the C code resolves things, and seems adequate
for this purpose.

> Having said the above, I don't understand why we're using MakeMaker at
> all and I think we should just get rid of it.
>
> I think the best way to proceed is probalby to just start getting rid of
> the perl/* make stuff as discussed above.
>
> Is that something you're interested in / have time for, otherwise I
> could see if I could find some time, but I don't have a lot these days.

Once thought on why MakeMaker may have value is that third-party PERL
tooling may want to import Git's PERL libraries. If this is something Git
supports, we probably need something like MakeMaker to know where the system
PERL library installation path is so it can put the Git library in the right
place in standard (non-RUNTIME_PREFIX_PERL) installations.

I agree that it shouldn't block this effort. My general judgement tends to
be that that a patch set should not sway far from a single purpose, else
it becomes hard to review and/or revert if it is problematic. Removing
MakeMaker seems like an independent cleanup task, so I'd inclined to do
that work in a separate patch set.

Having dived this deeply into things, I wouldn't mind working on this if a
block of time presents itself. Is there an issue tracker that the Git
project uses where I could add this as a backburner/cleanup task, so it's
not forgotten? Would it be appropriate to add a "TODO" comment in
"perl/Makefile"?

> [...] I could see if I could find some time, but I don't have a lot these
> days.

Thanks again for taking the time to work this out with me.
-Dan