Re: [PATCH v4 3/4] Makefile: add Perl runtime prefix support

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 29, 2017, 21:00 UTC
Message-ID: <87k1y8vlzs.fsf@evledraar.booking.com>
In-Reply-To: <20171129155637.89075-4-dnj@google.com>

On Wed, Nov 29 2017, Dan Jacques jotted:

Show 35 quoted lines

> Add a new Makefile flag, RUNTIME_PREFIX_PERL, which, when enabled,
> configures Perl scripts to locate the Git installation's Perl support
> libraries by resolving against the script's path, rather than
> hard-coding that path at build-time.
> [...]
> diff --git a/perl/header_runtime_prefix.pl.template b/perl/header_runtime_prefix.pl.template
> new file mode 100644
> index 000000000..fb9a9924d
> --- /dev/null
> +++ b/perl/header_runtime_prefix.pl.template
> @@ -0,0 +1,24 @@
> +# BEGIN RUNTIME_PREFIX_PERL generated code.
> +#
> +# This finds our Git::* libraries relative to the script's runtime path.
> +BEGIN {
> +	use lib split /@@PATHSEP@@/,
> +	(
> +		$ENV{GITPERLLIB}
> +		||
> +		do {
> +			require FindBin;
> +			require File::Spec;
> +			my $gitexecdir_relative = '@@GITEXECDIR@@';
> +			my $perllibdir_relative = '@@PERLLIBDIR@@';
> +
> +			($FindBin::Bin =~ m=${gitexecdir_relative}$=) ||
> +			    die('Unrecognized runtime path.');
> +			my $prefix = substr($FindBin::Bin, 0, -length($gitexecdir_relative));
> +			my $perllibdir = File::Spec->catdir($prefix, $perllibdir_relative);
> +			(-e $perllibdir) || die("Invalid library path: $perllibdir");
> +			$perllibdir;
> +		}
> +	);
> +}
> +# END RUNTIME_PREFIX_PERL generated code.

Ah, I see. To answer my own question in <87lgiovokg.fsf@evledraar.booking.com> you're making this stuff a relative path so you can use it here later on. I.e. we $FindBin::Bin, and then go from there. Makes sense.

We could use $ENV{GIT_EXEC_PATH} instead of FindBin here though, I missed that the first time. But that's just a nano-optimization. I just wondered whether git wasn't already passing us this info.

There is one remaining bug here. Git::I18N isn't doing the right thing, I installed in /tmp/git and moved to /tmp/git2, and it has:

    our $TEXTDOMAINDIR = $ENV{GIT_TEXTDOMAINDIR} || '/tmp/git/share/locale';

And GIT_TEXTDOMAINDIR is not passed by git (it's only used for the tests IIRC). Would need a similar treatment as this. Easiest to just set the path we find here in $Git::Whatever and pick it up in $Git::I18N later, it's not like anyone uses it outside of git.git.

But that does raise a more general concern for me. Isn't there some way we can run the test suite against an installed git (don't remember), then build, install, move the dir, and run the tests from the moved dir.

That would have caught this bug, and anything else that may be lurking still.