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