poetry2nix icon indicating copy to clipboard operation
poetry2nix copied to clipboard

mkPoetryDeps treats git branch as hash

Open lukebfox opened this issue 5 years ago • 9 comments
trafficstars

Presumably due to recent lockfile format changes, attempting to re-enter nix-shell after relocking a project with git dependencies fails. Steps to reproduce:

$ git clone https://github.com/NixOS/nixops-aws
$ cd nixops-aws
$ nix-shell 
$ poetry lock
$ exit
$ nix-shell

For this repository the pyproject entry causing the issue is nixops,

[tool.poetry.dependencies]
nixops = {git="https://github.com/nixos/nixops" rev="master"}

and attempting to enter nix-shell after relocking results in the following error:

error: --- BadHash ------------------------------------------------------------------ nix-shell
hash 'master' has wrong length for hash type 'sha1'
(use '--show-trace' to show detailed location information)

Similarly when I try another variant,

nixops = {git="https://github.com/lukebfox/nixops" rev="statedict-store-dict"}

I get another error:

error: --- UsageError --------------------------------------------------------------- nix-shell
unknown hash algorithm 'statedict'
Try 'nix-shell --help' for more information.

The top of the stack trace in both cases:

trace: while evaluating the attribute 'src.name'
at: (155:7) in file: /nix/store/vjj6vlbfq661l2zllgvyspdsdyvz8hl9-source/pkgs/development/tools/poetry2nix/poetry2nix/mk-poetry-dep.nix

   154|       # Here we can then choose a file based on that info.
   155|       src =
      |       ^
   156|         if isGit then

trace: while evaluating 'hasSuffix'
at: (212:5) in file: /nix/store/vjj6vlbfq661l2zllgvyspdsdyvz8hl9-source/lib/strings.nix

   211|     # Input string
   212|     content:
      |     ^
   213|     let

trace: from call site
at: (122:25) in file: /nix/store/vjj6vlbfq661l2zllgvyspdsdyvz8hl9-source/pkgs/development/interpreters/python/mk-python-derivation.nix

   121|       pythonRemoveBinBytecodeHook
   122|     ] ++ lib.optionals (lib.hasSuffix "zip" (attrs.src.name or "")) [
      |                         ^
   123|       unzip

(it goes on a fair bit)

Not sure if this is intended behaviour and the pyproject.toml needs updating to something new, or this is an issue.

lukebfox avatar Oct 26 '20 13:10 lukebfox

I've just noticed the same issue with tag instead of rev: it's used as a hash:

nix develop --show-trace                                                                                                                                                          ~/nova/jcli
warning: Git tree '/home/teto/nova/jcli' is dirty
error: --- BadHash ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- nix
hash 'v6.3.1' has wrong length for hash type 'sha1'
------------------------------------------------------------------------------------------ show-trace ------------------------------------------------------------------------------------------
trace: while evaluating the attribute 'src.name'
at: (155:7) in file: /nix/store/zbf46b09fzd5lm7kqgwzsadsxax44mvp-source/pkgs/development/tools/poetry2nix/poetry2nix/mk-poetry-dep.nix

   154|       # Here we can then choose a file based on that info.
   155|       src =
      |       ^
   156|         if isGit then

trace: while evaluating 'hasSuffix'
at: (234:5) in file: /nix/store/zbf46b09fzd5lm7kqgwzsadsxax44mvp-source/lib/strings.nix

   233|     # Input string
   234|     content:
      |     ^
   235|     let

teto avatar Oct 26 '20 23:10 teto

So I think I know why, it depends on the nix version https://nixos.org/manual/nix/unstable/release-notes/rl-2.3.html, which is why I saw the bug disappear in https://github.com/nix-community/poetry2nix/pull/167

builtins.fetchGit's ref argument now allows specifying an absolute remote ref. Nix will automatically prefix ref with refs/heads only if ref doesn't already begin with refs/.

I've added a debug statement in mk-poetry-deps

            builtins.fetchGit (lib.debug.traceValSeq {
              inherit (source) url;
              rev = source.resolved_reference or source.reference;
              ref = sourceSpec.branch or sourceSpec.tag or "HEAD";
            })

and poetry generates:

trace: { ref = "refs/tags/rel_1_3_1"; rev = "8d6bb007a4de046c4d338f4b79b40c9fcbf73ab7"; url = "https://github.com/sqlalchemy/alembic.git"; }
trace: { ref = "4321bbfda9aa190acdad05eb901d3b59439f0ec9"; rev = "4321bbfda9aa190acdad05eb901d3b59439f0ec9"; url = "https://github.com/tartley/colorama.git"; }
fatal: impossible de trouver la référence distante refs/heads/4321bbfda9aa190acdad05eb901d3b59439f0ec9

which is obviously incorrect. so we should correctly prefix tags/ignore ref when not available.

teto avatar Oct 28 '20 15:10 teto

I have the opposite of this issue, where for me it seems to use the rev as a branch while it is a hash. I have this entry in my pyproject.toml:

oauth2 = {git = "https://github.com/CodeGra-de/python3-oauth2.git", rev = "47fcd95ad2d29ae85c0a03cc39ddee7ffe921f3d"}

But that gives the error

fetching Git repostiory 'https://github.com/CodeGra-de/python3-oauth2.git'fatal: couldn't find remote ref refs/heads/47fcd95ad2d29ae85c0a03cc39ddee7ffe921f3d

I've also tried your fix @teto, but that gives another error

fetching Git repostiory 'https://github.com/CodeGra-de/python3-oauth2.git'fatal: couldn't find remote ref refs/heads/HEAD

olmokramer avatar Nov 05 '20 07:11 olmokramer

According to this

https://github.com/NixOS/nix/blob/4dcb183af31d5cb33b6ef8e581e77d1c892a58b9/src/libfetchers/git.cc#L282

builtins.fetchGit can't be used to fetch HEAD of remote repositories. (The default being master in spite of the documentation and it is always prefixed with refs/heads if it doesn't start with refs/ which doesn't allow for HEAD.)

And if the rev is already resolved to a commit hash, it is unnecessary to provide a ref. So in this case, why not just skip it?

Are there cases, where rev isn't resolved to an commit sha in

https://github.com/nix-community/poetry2nix/blob/1894b501cf4431fb218c4939a9acdbf397ac1803/mk-poetry-dep.nix#L160

If that is the case, we would probably need a check on the rev, whether is a commit sha, for deciding if we skip ref.

typetetris avatar Nov 21 '20 07:11 typetetris

And if the rev is already resolved to a commit hash, it is unnecessary to provide a ref.

I dont think so, if you don't specify the correct branch, builtins.fetchGit won't find the revision. It is also advised to keep both to print better error messages see fetchGit doc https://nixos.org/manual/nix/stable/

teto avatar Nov 25 '20 18:11 teto

And if the rev is already resolved to a commit hash, it is unnecessary to provide a ref.

I dont think so, if you don't specify the correct branch, builtins.fetchGit won't find the revision. It is also advised to keep both to print better error messages see fetchGit doc https://nixos.org/manual/nix/stable/

I thought commit hashes were unique across the whole repository, so the ref is irrelevant in that case.

But I think the more important information is, that you can't fetch HEAD of a remote repository with builtins.fetchGit no matter what the documentation says (it is wrong).

See https://github.com/NixOS/nix/blob/4dcb183af31d5cb33b6ef8e581e77d1c892a58b9/src/libfetchers/git.cc#L282

Maybe a different fetcher for git has to be used. Didn't look into the ones from nixpkgs, what they do.

typetetris avatar Nov 25 '20 18:11 typetetris

I thought commit hashes were unique across the whole repository, so the ref is irrelevant in that case.

they may be unique but similar to how a shallow clone doesn't download the whole history, fetchGit (I suppose) downloads only the revision tree for the current ref (with --single-branch ?) and thus may not know your rev.

where is HEAD stored ? anyhow that's an annoying bug, hope it can get fixed.

teto avatar Nov 25 '20 19:11 teto

I thought commit hashes were unique across the whole repository, so the ref is irrelevant in that case.

they may be unique but similar to how a shallow clone doesn't download the whole history, fetchGit (I suppose) downloads only the revision tree for the current ref (with --single-branch ?) and thus may not know your rev.

Ah my bad, that is true. Didn't think of that. Sorry for the noise.

where is HEAD stored ? anyhow that's an annoying bug, hope it can get fixed.

It is .git/HEAD usually, isn't it? So the forced prefix of refs/ prevents builtins.fetchGit from being able to get it.

typetetris avatar Nov 25 '20 20:11 typetetris

For a moment I thought this was resolved by #199 but I tested this on two repositories and for one of them I still get the error described by @olmokramer, don't mind me.

lukebfox avatar Feb 14 '21 21:02 lukebfox