goDeps: support fetchFromGitHub to fetch Go libs

[kamilchm]

Motivation for this change

It's more like a proposal than a real change.
I can convert all github goDeps from fetchgit to fetchFromGitHub so they will look like:

{
  goPackagePath = "github.com/Masterminds/vcs";
  fetch = {
    type = "FromGitHub"; # <- this is the new thing
    owner = "Masterminds";
    repo = "vcs";
    rev = "fbe9fb6ad5b5f35b3e82a7c21123cfc526cbf895";
    sha256 = "09rdq8k2smwgy6wdslddcq13wg3lzd4amzpy0f8kd0hlxxvas89c";
  };
}

So let me know is it ok and after we merge fetchFromGitHub support I'll convert all goDeps that can use it and add fetchFromGitHub to go2nix.

Things done

• Tested using sandboxing
  (nix.useSandbox on NixOS,
  or option build-use-sandbox in nix.conf
  on non-NixOS)
• Built on platform(s)
  • NixOS
  • OS X
  • Linux
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

CC: @copumpkin, @edolstra, @rushmorem, @zimbatm, @cstrahan

#16017 (comment)
#17254 (comment)

[mention-bot]

@kamilchm, thanks for your PR! By analyzing the history of the files in this pull request, we identified @wkennington, @ehmry and @lethalman to be potential reviewers.

[zimbatm]

Yeah I think that would be useful as a general idea.

There should be a way to say ./fetchSources ./deps.nix where deps.nix is in the NIXON or JSON format. Then it's easy to build tools that look for upstream releases and update just those files.

I am currently thinking in the same lines related to multi-arch binary releases. There should be a way to say nix-prefetch-url -A mypackage.src.all and get the sha of all the archs. Ideally nix-prefetch-url also generates the deps.nix format and then we're gold :)

[kamilchm]

So should we merge it and move all Go packages to use fetchFromGitHub where it can?

[zimbatm]

@kamilchm what would you think of building on top of #19540 ?

[kamilchm]

Sure! I'd be glad if we can benefit from unified NIXON format for fetchers.
I've read your examples but I couldn't get it how exactly could goDeps definition look using it. Something like?

goDeps = [ 
  {
    goPackagePath = "github.com/Masterminds/vcs";
    fetchdata = {
      type = "FromGitHub";
      owner = "Masterminds";
      repo = "vcs";
      rev = "fbe9fb6ad5b5f35b3e82a7c21123cfc526cbf895";
      sha256 = "09rdq8k2smwgy6wdslddcq13wg3lzd4amzpy0f8kd0hlxxvas89c";
    };
  }
];

[zimbatm]

One possible implementation would be to do:

[ 
  {
    goPackagePath = "github.com/Masterminds/vcs";
    fetch = {
      fetcher = "fetchFromGitHub";
      owner = "Masterminds";
      repo = "vcs";
      rev = "fbe9fb6ad5b5f35b3e82a7c21123cfc526cbf895";
      sha256 = "09rdq8k2smwgy6wdslddcq13wg3lzd4amzpy0f8kd0hlxxvas89c";
    };
  }
]

And then in your code you could have a condition:

if goDep.fetch ? fetcher then fetchdata goDep.fetch else ...

and you can retain the other two branches for backward compatibility.

I know the fetcher attribute is longer than the type one but it allows for more flexibility without having to maintain a map of types to fetchers.

[disassembler]

@kamilchm I rebased this on master. The other issue #19540 died, so I think this can go through as-is. @grahamc can you assimilate this issue so we can see how many rebuilds it will cause?

[orivej]

@disassembler This rebuilds nothing because it does not change any derivation. You should be able to see for yourself with nox-review or maintainers/scripts/rebuild-amount.sh. Besides, rebuilding all go packages is not a mass rebuild due to the compilation speed.

[disassembler]

Thanks for the tips! @orivej you think this is good to merge? With me fixing those changes, I don't feel like being the one to hit the button without another review!

[orivej]

Let's wait another day for others to review, then I'd merge this.

[kamilchm]

I'm not sure is it a desired direction after @edolstra comment:

fetchFromGitHub should not have been introduced in the first place since it doesn't really provide any value over calling fetchzip directly
...
So I no longer feel we should promote the use of fetchFromGitHub.

[orivej]

Unlike fetchzip, fetchFromGitHub used to provide meaningful names in the nix store. (fetchzip used to save /nix/store/{hash}-{version} while fetchFromGitHub used to save {hash}-{name}-{version}-src.) This feature was deleted in c3255fe#commitcomment-25286020 and now, indeed, fetchzip is a real alternative to fetchFromGitHub. We should add fetch.type = "zip" as an option [1]. Yet the complete deletion of source names seems excessive and may be restricted in the future, so I don't think that we should prefer fetchzip to fetchFromGitHub in go2nix.

[1] deps.nix could be made more along these lines, simple and concise if we generate e.g.

{ fetch }: [
  {
    goPackagePath = "github.com/Masterminds/vcs";
    src = fetch.github "Masterminds" "vcs" "v1.12.0" "09rdq8k2smwgy6wdslddcq13wg3lzd4amzpy0f8kd0hlxxvas89c" { };
  }
  {
    goPackagePath = "github.com/jawher/mow.cli";
    src = fetch.zip "https://github.com/jawher/mow.cli/archive/v1.0.3.zip" "1a8hnh2k3vc3prjhnz4rjbiwhqq6r3mi18h9cdb6fc6s6yzjc19j" { };
  }
]

because go2nix is generating fetchers with a fixed number of arguments, or even

{ fetch }: {
  "github.com/Masterminds/vcs" = fetch.github "Masterminds" "vcs" "v1.12.0" "09rdq8k2smwgy6wdslddcq13wg3lzd4amzpy0f8kd0hlxxvas89c" { };
  "github.com/jawher/mow.cli" = fetch.zip "https://github.com/jawher/mow.cli/archive/v1.0.3.zip" "1a8hnh2k3vc3prjhnz4rjbiwhqq6r3mi18h9cdb6fc6s6yzjc19j" { };
}

if you don't expect the need for additional attributes.

[zimbatm]

Eelco is against adding a generic fetcher to nixpkgs as far as I remember so we should avoid doing that. But we are still free to do what we want with the goDeps I believe.

One important thing is that the generated code should be able to evolve independently from nixpkgs when people are using it outside of the repo. For that I think that we should at least introduce a top-level schemaVersion attribute so that we can keep backward-compatibility when the schema is changing. That would be a thing to do now.

Regarding the exact schema, it will be easier to maintain it if we only have to deal with pure data and no functions as function have a tendency to change. Then in the goDeps loader we can implement and update the heuristic as needed when nixpkgs evolves.

For the exact schema shape I would like to suggest looking at the generated lock files generated by tools like govendor and see if we can re-use any of it. Ideally we can read the govendor (or other) tool's lock file directly, that would be one less tool to maintain.

[kamilchm]

OK, so It looks reasonable to use fetchFromGitHub internally to get Go dependencies.
We can merge this, as it's backward compatible.

@zimbatm by adding top-level schemaVersion, you mean that deps.nix should be converted to an attribute set wit schemaVersion in it?

@orivej I think that your proposals needs a better design or a separate PR to see better what changes needs to be done in existing Go derivations.

[zimbatm]

@kamilchm yes, the schemaVersion would be on the top of deps.nix and be bumped when backward-incompatible changes are introduced.

[orivej]

OK, let's postpone schema changes and use what this PR makes available.