nix-community / poetry2nix

Convert poetry projects to nix automagically [maintainer=@adisbladis,@cpcloud]
MIT License
775 stars 402 forks source link

mkPoetryDeps treats git branch as hash #197

Open lukebfox opened 3 years ago

lukebfox commented 3 years ago

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.

teto commented 3 years ago

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 commented 3 years ago

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.

olmokramer commented 3 years ago

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
typetetris commented 3 years ago

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.

teto commented 3 years ago

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/

typetetris commented 3 years ago

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.

teto commented 3 years ago

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.

typetetris commented 3 years ago

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.

lukebfox commented 3 years ago

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.