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@xxxxxxxx>
# 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?
