Hi Jeremy,
On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
> > On Jan 29, 2019, at 2:47 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> >
> > Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx> writes:
> >
> >> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx>
> >> ---
> >> t/test-lib.sh | 3 +++
> >> 1 file changed, 3 insertions(+)
> >
> > This obviously won't be acceptable as-is to my tree. Shouldn't this
> > be something to be dealt with in config.mak.uname or something that
> > is meant to define platform-specific customization?
>
> The issue here is that we're not locating relocatable perl modules
> during testing. This is a general problem with testing RUNTIME_PREFIX
> configurations, and a more general solution to this sledgehammer would
> be appropriate. I don't think config.mak.uname really makes sense since
> it's a general RUNTIME_PREFIX issue and not specifically a darwin issue.
First of all, as others have pointed out, this code is very, very specific
to Darwin (not only xcode-select but also Library/Perl/ are very, very
specific to that platform, I would even argue it is not even
Darwin-specific but instead macOS specific because bare-bones Darwin does
not have Library/Perl/, does it?).
So you *definitely* want to put that code into guards testing for that
platform (I do not think config.mak.uname is the correct place, though, as
it should be accessible to test scripts when run directly, i.e. not
through `make`).
But let's take a huge step back first: why? What is the exact problem this
commit tries to solve? The commit message unfortunately does not really
leave me any wiser.
So I am left with the unfortunate position of having to guess, which is
not really a good use of both of our time. If I allow myself to indulge in
the guessing game, I would guess that whatever `perl` executable is used
in your scenario picks up some unfortunate environment variable that
overrides its internal defaults where to look for Perl modules.
And that simply should not be the case. We are very careful to set
GITPERLLIB in bin-wrappers/, *not* PERL5LIB.
And when we build Git on macOS agents in Travis or Azure Pipelines and
then run the test suite, I fail to see any Perl-related error that looks
like it could be solved by this here patch.
In short: this commit is in dear want of a more substantive commit
message, and most likely in search for a different solution.
Ciao,
Johannes
>
> >
> >>
> >> diff --git a/t/test-lib.sh b/t/test-lib.sh
> >> index 0f1faa24b2..4060a53f56 100644
> >> --- a/t/test-lib.sh
> >> +++ b/t/test-lib.sh
> >> @@ -1017,6 +1017,9 @@ fi
> >>
> >> GITPERLLIB="$GIT_BUILD_DIR"/perl/build/lib
> >> export GITPERLLIB
> >> +PERL_VERSION=$(grep DEFAULT /usr/local/versioner/perl/versions | sed 's:^.*= *\([^ ]*\)$:\1:')
> >> +PERL5LIB="$GIT_BUILD_DIR"/perl:"$(xcode-select -p)"/Library/Perl/$PERL_VERSION
> >> +export PERL5LIB
> >> test -d "$GIT_BUILD_DIR"/templates/blt || {
> >> error "You haven't built things yet, have you?"
> >> }
>
>
