Github dependencies fail with nix 2.4

[Sohalt]

Describe the bug
Using nix 2.4 the github-dependency test case from the examples-projects fails with fetching Git repository 'https://github.com/tmcw/leftpad'fatal: couldn't find remote ref refs/heads/db1442a0556c2b133627ffebf455a78a1ced64b9 .

To Reproduce
package.json

{
  "name": "github-dependency",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "leftpad": "github:tmcw/leftpad#db1442a0556c2b133627ffebf455a78a1ced64b9"
  }
}

package-lock.json

{
  "name": "github-dependency",
  "version": "1.0.0",
  "lockfileVersion": 1,
  "requires": true,
  "dependencies": {
    "leftpad": {
      "version": "github:tmcw/leftpad#db1442a0556c2b133627ffebf455a78a1ced64b9",
      "from": "github:tmcw/leftpad#db1442a0556c2b133627ffebf455a78a1ced64b9"
    }
  }
}

flake.nix

{
  description = "Content management system for open local knowledge";

  # Nixpkgs / NixOS version to use.
  inputs.nixpkgs.url = "nixpkgs/nixos-21.05";

  inputs.npmlock2nix = {
    url = "github:tweag/npmlock2nix";
    flake = false;
  };

  outputs = { self, nixpkgs, npmlock2nix }:
    let

      # Generate a user-friendly version numer.
      version = "1.0.0";

      # System types to support.
      supportedSystems = [ "x86_64-linux" "x86_64-darwin" ];

      # Helper function to generate an attrset '{ x86_64-linux = f "x86_64-linux"; ... }'.
      forAllSystems = f: nixpkgs.lib.genAttrs supportedSystems (system: f system);

      # Nixpkgs instantiated for supported system types.
      nixpkgsFor = forAllSystems (system: import nixpkgs { inherit system; overlays = [ self.overlay ]; });

    in

    {

      # A Nixpkgs overlay.
      overlay = final: prev: {

        npmtest = with final; (import npmlock2nix {pkgs = final;}).node_modules rec {
          name = "npmtest-${version}";

          src = ./.;
        };

      };

      # Provide some binary packages for selected system types.
      packages = forAllSystems (system:
        {
          inherit (nixpkgsFor.${system}) npmtest;
        });

      # The default package for 'nix build'. This makes sense if the
      # flake provides only one package or there is a clear "main"
      # package.
      defaultPackage = forAllSystems (system: self.packages.${system}.npmtest);

    };
}

Then run nix build

Expected behavior
The build succeeds.

Environment

• NixOS 21.05 + Nix 2.4
• Version of the code: c63a75e

[andir]

Does that also happen if you don't run the build through a flake?

[Sohalt]

Does that also happen if you don't run the build through a flake?

Yes

[andir]

This looks like a Nix regression then:

# reproducer.nix
builtins.fetchGit {
  ref = "db1442a0556c2b133627ffebf455a78a1ced64b9";
  rev = "db1442a0556c2b133627ffebf455a78a1ced64b9";
  url = "https://github.com/tmcw/leftpad";
}

On stable (2.3) nix:

warning: refname 'db1442a0556c2b133627ffebf455a78a1ced64b9' is ambiguous.
Git normally never creates a ref that ends with 40 hex characters
because it will be ignored when you just specify 40-hex. These refs
may be created by mistake. For example,

  git switch -c $br $(git rev-parse ...)

where "$br" is somehow empty and a 40-hex ref is created. Please
examine these refs and maybe delete them. Turn this message off by
running "git config advice.objectNameWarning false"
{ outPath = "/nix/store/8frq54wwgi63wqgkc7p6yrcljlx4zwzh-source"; rev = "db1442a0556c2b133627ffebf455a78a1ced64b9"; revCount = 5; shortRev = "db1442a"; }

On the unstable (2.4) version of NIx:

fetching Git repository 'https://github.com/tmcw/leftpad'fatal: couldn't find remote ref refs/heads/db1442a0556c2b133627ffebf455a78a1ced64b9
error: program 'git' failed with exit code 128

So we went from a warning into a failure (instead of an explicit error from the Nix side).

[doronbehar]

I encountered this issue as well with unstable Nix, and I tried using stable Nix with:

$ nix shell nixpkgs#nixStable -c nix-build
warning: refname '4a54f56a25b58950f9e4277c56db2912d62242e7' is ambiguous.
Git normally never creates a ref that ends with 40 hex characters
because it will be ignored when you just specify 40-hex. These refs
may be created by mistake. For example,

  git switch -c $br $(git rev-parse ...)

where "$br" is somehow empty and a 40-hex ref is created. Please
examine these refs and maybe delete them. Turn this message off by
running "git config advice.objectNameWarning false"
error: [npmlock2nix] failed to parse GitHub reference `git+https://github.com/resin-io/bonjour.git#e018851dc823b4b3f670f658f71d0c1c7f3e637c`. Expected a string of format `github:org/repo#revision`

But I don't understand why it fails.. The lock file is at:

https://github.com/balena-io/balena-cli/blob/master/npm-shrinkwrap.json

[milahu]

error: [npmlock2nix] failed to parse GitHub reference `git+https://github.com/resin-io/bonjour.git#e018851dc823b4b3f670f658f71d0c1c7f3e637c`. Expected a string of format `github:org/repo#revision`

this format is used in npm7 lockfiles

current npmlock2nix only understands github:org/repo#rev

function parseGitHubRef

  # Description: Takes a string of the format "github:org/repo#revision" and returns
  # an attribute set { org, repo, rev }
  # Type: String -> Set
  parseGitHubRef = str:
    if lib.hasPrefix "github:" str
    then
      let
        parts = builtins.split "[:#/]" str;
      in
      assert !(builtins.length parts == 7) ->
        throw "failed to parse GitHub reference `${str}`. Expected a string of format `github:org/repo#revision`";
      rec {
        inherit parts;
        org = builtins.elemAt parts 2;
        repo = builtins.elemAt parts 4;
        rev = builtins.elemAt parts 6;
      }
    else
    if lib.hasPrefix "git+ssh://git@github.com/" str # produced by npm v7 (dependency.version)
    then
    # git+ssh://git@github.com/milahu/postcss-nodegui-autoprefixer.git#263b0245164ae0ab10cd5abf31f2ad302c0ecec3
      let
        parts = builtins.split "[:#/]|(.git#)" str;
      in
      assert !(builtins.length parts == 13) ->
        throw "failed to parse GitHub reference `${str}`. Expected a string of format `git+ssh://git@github.com/repo.git#revision`";
      rec {
        inherit parts;
        org = builtins.elemAt parts 8;
        repo = builtins.elemAt parts 10;
        rev = builtins.elemAt parts 12;
      }
    else
    if (builtins.match ".+@github:.+" str) != null # produced by npm v7 (dependency.from)
    # postcss-nodegui-autoprefixer@github:milahu/postcss-nodegui-autoprefixer#263b0245164ae0ab10cd5abf31f2ad302c0ecec3
    then
      let
        parts = builtins.split "[:#/]|(.git#)" (builtins.elemAt (builtins.split "@github:" str) 2);
      in
      assert !(builtins.length parts == 5) ->
        throw "failed to parse GitHub reference `${str}`. Expected a string of format `name@github:org/repo#revision`. str2 = ${builtins.elemAt (builtins.split "@github:" str) 2}. num parts = ${toString (builtins.length parts)}";
      rec {
        inherit parts;
        org = builtins.elemAt parts 0;
        repo = builtins.elemAt parts 2;
        rev = builtins.elemAt parts 4;
      }
    else
    builtins.throw "failed to parse version: ${str}"
  ;

[doronbehar]

this format is used in npm7 lockfiles

current npmlock2nix only understands github:org/repo#rev

Thanks for the clarification, it seems as if #94 helps this, but nixStable is still needed.