Git not found when trying to upgrade pubs

[guillaumecherel]

(I'm not sure this is the appropriate for this report. Sorry for the inconvenience if not and please let me know.)

Describe the bug
Trying to update the package pubs to the latest version (0.8.3), the build fails when running the tests because the command git cannot be found. I'm new to nix and I'm not sure if this can be fixed on the nix side or not.

To Reproduce
Steps to reproduce the behavior:

1. Override the pubs package like so in a file default.nix:

  pubs083 = pubs.overridePythonAttrs (old: rec {
    version = "0.8.3";
    src = fetchFromGitHub {
      owner = "pubs";
      repo = "pubs";
      rev = "v{version}";
      sha256 = "0npgsyxj7kby5laznk5ilkrychs3i68y57gphwk48w8k9fvnl3zc";
    };

    patches = [ ];
  });

(I removed the patches because I believe they were applied upstream.)

2. Install with nix-env -f default.nix -iA pubs083

3. The following error appears during the test phase:

...
======================================================================
ERROR: test_git (tests.test_git.TestGitPlugin)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/build/source/pubs/plugs/git/git.py", line 44, in _gitinit
    self.shell('init')
  File "/build/source/pubs/plugs/git/git.py", line 81, in shell
    raise RuntimeError('The git plugin encountered an error when running the git command:\n' +
RuntimeError: The git plugin encountered an error when running the git command:
git -C /build/tmppalwuu58/pubs -c color.ui=always init

Returned output:
/bin/sh: git: not found
...

Notify maintainers
@gebner

Metadata
Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

 - system: `"x86_64-linux"`
 - host os: `Linux 5.4.54, NixOS, 20.09pre236721.840c782d507 (Nightingale)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.3.7`
 - channels(guillaume): `""`
 - channels(root): `"nixos-20.09pre236721.840c782d507, nixos-hardware"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

Maintainer information:

# a list of nixpkgs attributes affected by the problem
attribute:
# a list of nixos modules affected by the problem
module: pubs

[Thra11]

You can add git to checkInputs with a line like this in your overrideAttrs:

    checkInputs = old.checkInputs ++ [ git ];

(Depending on the context, you might need pkgs.git or self.git or something else, depending what's in scope)
However, that doesn't actually get you much further:

======================================================================
ERROR: test_git (tests.test_git.TestGitPlugin)
-------------------------full---------------------------------------------
Traceback (most recent call last):
  File "/build/source/tests/test_git.py", line 26, in test_git
    hash_a = git_hash(self.default_pubs_dir)
  File "/build/source/tests/test_git.py", line 12, in git_hash
    return subprocess.check_output(hash_cmd)
  File "/nix/store/1l23hylw0nxrfdd12cyp9lsasgdm5fwd-python3-3.7.7/lib/python3.7/subprocess.py", line 411, in check_output
    **kwargs).stdout
  File "/nix/store/1l23hylw0nxrfdd12cyp9lsasgdm5fwd-python3-3.7.7/lib/python3.7/subprocess.py", line 512, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '('git', '-C', '/build/tmp4167cgcg/pubs', 'rev-parse', 'HEAD')' returned non-zero exit status 128.

======================================================================
ERROR: test_manual (tests.test_git.TestGitPlugin)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/build/source/pubs/pubs_cmd.py", line 104, in execute
    args.func(conf, args)
  File "/build/source/pubs/plugs/git/git.py", line 64, in command
    self.shell(' '.join([shell_quote(a) for a in args.arguments]), command=True)
  File "/build/source/pubs/plugs/git/git.py", line 85, in shell
    'https://github.com/pubs/pubs/issues')
RuntimeError: The git plugin encountered an error when running the git command:
git -C /build/tmp5e8h1b_u/pubs -c color.ui=always commit -m '"initial_commit"'

Returned output:

*** Please tell me who you are.

Run

  git config --global user.email "you@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'nixbld@localhost.(none)')

If needed, you may fix the state of the /build/tmp5e8h1b_u/pubs git repository manually.
If relevant, you may submit a bug report at https://github.com/pubs/pubs/issues

Looks like the second failure, test_manual is because the test expects git to be pre-configured with the committer's details, which in a nix build sandbox it isn't. I believe they could probably fix this test by setting some environment variables. e.g. I believe something like this won't try to read the git config because all the info it needs is already in the environment:

GIT_AUTHOR_NAME="AuthorName" GIT_AUTHOR_EMAIL="author@test.email" GIT_COMMITTER_NAME="CommitterName" GIT_COMMITTER_EMAIL="committer@test.email" git -C /build/tmp5e8h1b_u/pubs -c color.ui=always commit -m '"initial_commit"'

As for the first failure, test_git... I'm not sure if it's trying to use the sources as a git repo in the test (and failing because fetchFromGitHub just downloads an archive tarball, without the .git directory, not an actual git repo), or whether it's just getting its paths muddled in some way.

If you just want to get it built and aren't too worried about the git tests, you can simply remove the offending test file by adding something like this to your overrideAttrs:

    preCheck = ''rm tests/test_git.py'';

This will delete the test_git.py before running the check phase. setup.py uses a test_*.pyglob to find test files, so it doesn't even know it ever existed. This resulted in a successful build for me.

[Thra11]

If you do remove the git tests as I described, there's probably no longer any need to add git to checkInputs.

[guillaumecherel]

Thanks for your help. I'm going to take the path of disabling git tests as you described.

I would have liked to update the nixpkgs repo with the new version, but this workaround is probably insufficient to make it into the repo, isn't it? I tried diving in the build process with nix-shell to debug but I can only trigger the unpackPhase, I can't figure out how the rest of the build is done. The subsequent phases (configurePhase, buildPhase, …) don't get me anywhere, I guess there is something special about the buildPythonApplication function used to make the derivation and I don't know enough about it. Unfortunately I lack time to dive further.

I'm going to raise this issue to the pubs repo in case the developer can do anything about it.

[Thra11]

Removing failing tests when packaging things in nixpkgs is generally ok[1], as long as we have good reason to think that the tests are invalid (i.e. that they are failing because they are making incorrect assumptions, rather than because the package we're building is broken.). Just add a comment to the package next to your fix with a link to your upstream issue, so others know why you did it and when it is no longer required.

[1] Some packages in nixpkgs just skip their entire test suite with dontCheck = true;.

[guillaumecherel]

Ok, I made the PR.

[gebner]

Fixed by #95899.