From 2163fc7f0a3b8b8215452fc6631642b811116000 Mon Sep 17 00:00:00 2001
From: Ruud van Asseldonk <dev@veniogames.com>
Date: Mon, 25 Nov 2019 23:14:28 +0100
Subject: [PATCH] git: fix the "perlSupport = false" configuration

When perlSupport = false, we will set NO_PERL=1, and build Git without
Perl support. This is a build option that Git supports. However, Git's
test suite still requires a Perl to be available to run the tests, and
we did not provide one. The tests respect PERL_PATH, and if it is not
set, they default to /usr/bin/perl.

Before this commit, if we set "perlSupport = false", then no Perl would
be available to the package, and so the tests would default to
/usr/bin/perl. When building without a sandbox, that could still work,
even though there is no "perl" on the path, because the tests defaulted
to an absolute path.

You can reproduce this issue as follows:

    nix-build -E 'let pkgs = (import ./default.nix) {}; in pkgs.git.override { perlSupport = false; }'

I just ran into this when trying to build pkgs.git from an old version
of Nixpkgs that I was able to build just fine in the past, and today it
would not build any more, complaining when running the tests:

    make -C t/ all
    make[1]: Entering directory '/build/git-2.18.0/t'
    rm -f -r 'test-results'
    /nix/store/czx8vkrb9jdgjyz8qfksh10vrnqa723l-bash-4.4-p23/bin/bash: /usr/bin/perl: No such file or directory

In the past the sandbox was not enabled by default, so then it worked
for me. But now that it is enabled, my host's (not NixOS) /usr/bin/perl
is no longer accessible, and the build fails.

The solution is to explicitly set PERL_PATH when running the tests. This
*almost* works, except that there appears to be a bug in the test for
"git request-pull". That command is a Bash script that calls Perl at
some point, so it requires Perl, and therefore it cannot be supported
when NO_PERL=1. But that particular test does not check whether Git was
compiled with Perl support (other tests do include that check), and that
makes the test fail:

    t5150-request-pull.sh ..............................
    not ok 4 - pull request after push
    not ok 5 - request asks HEAD to be pulled
    not ok 6 - pull request format
    not ok 7 - request-pull ignores OPTIONS_KEEPDASHDASH poison
    not ok 9 - pull request with mismatched object
    not ok 10 - pull request with stale object
    Dubious, test returned 1 (wstat 256, 0x100)
    Failed 6/10 subtests

This output makes sense if you look at t5150-request-pull.sh. Test 1 and
2 are setup steps. Test 3 does call request-pull, but it expects the
command to fail, and it cannot distinguish between the command exiting
with a nonzero exit code, or failing to start it at all. So test 3
passes for the wrong reasons. Test 4 through 10 all call request-pull,
so they fail.

The quick workaround here is to disable the test. I will look into
upstreaming a patch that makes the test skip itself when Perl is
disabled.
---
 .../version-management/git-and-tools/git/default.nix | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/pkgs/applications/version-management/git-and-tools/git/default.nix b/pkgs/applications/version-management/git-and-tools/git/default.nix
index a8e284c6fec7..7c6f2952acce 100644
--- a/pkgs/applications/version-management/git-and-tools/git/default.nix
+++ b/pkgs/applications/version-management/git-and-tools/git/default.nix
@@ -253,7 +253,10 @@ stdenv.mkDerivation {
   installCheckTarget = "test";
 
   # see also installCheckFlagsArray
-  installCheckFlags = "DEFAULT_TEST_TARGET=prove";
+  installCheckFlags = [
+    "DEFAULT_TEST_TARGET=prove"
+    "PERL_PATH=${buildPackages.perl}/bin/perl"
+  ];
 
   preInstallCheck = ''
     installCheckFlagsArray+=(
@@ -296,6 +299,13 @@ stdenv.mkDerivation {
     # Tested to fail: 2.18.0
     disable_test t9902-completion "sourcing the completion script clears cached --options"
 
+    ${stdenv.lib.optionalString (!perlSupport) ''
+      # request-pull is a Bash script that invokes Perl, so it is not available
+      # when NO_PERL=1, and the test should be skipped, but the test suite does
+      # not check for the Perl prerequisite.
+      disable_test t5150-request-pull
+    ''}
+
     # As of 2.19.0, t5562 refers to #!/usr/bin/perl
     patchShebangs t/t5562/invoke-with-content-length.pl
   '' + stdenv.lib.optionalString stdenv.isDarwin ''