Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix error handling and return code #35017

Closed

Conversation

qknight
Copy link
Member

@qknight qknight commented Feb 15, 2018

Motivation for this change

there is a bug in nix-prefetch-git which makes our implementation of dep2nix impossible, since the error handling is not working at all.

the cause for the problem is that https://golang.org/x/net is not a git repository at all and therefore nix-prefetch-git should fail with an exit code != 0 which is not the case.

the output before the change A
nix-prefetch-git https://golang.org/x/net --rev f5dfe339be1d06f81b22525fe34671ee7d2c8904
Leeres Git-Repository in /tmp/git-checkout-tmp-77RGm6hA/net-f5dfe33/.git/ initialisiert
fatal: https://golang.org/x/net/info/refs not valid: is this a git repository?
fatal: https://golang.org/x/net/info/refs not valid: is this a git repository?
Unable to checkout f5dfe339be1d06f81b22525fe34671ee7d2c8904 from https://golang.org/x/net.
joachim@lenovo-t530 /tmp> echo $status
1
the output before the change A (with --quiet)
nix-prefetch-git https://golang.org/x/net --rev f5dfe339be1d06f81b22525fe34671ee7d2c8904 --quiet
{
  "url": "https://golang.org/x/net",
  "rev": "f5dfe339be1d06f81b22525fe34671ee7d2c8904",
  "date": "",
  "sha256": "0sjjj9z1dhilhpc8pq4154czrb79z9cm044jvn75kxcjv6v5l2m5",
  "fetchSubmodules": true
}
joachim@lenovo-t530 /tmp> echo $status
0

WARNING: exit code should still be 1 and not 0!

the output after the change B

[nix-shell:~/Desktop/projects/nixcloud/dep2nix/src/github.com/nixcloud/dep2nix]$ ./nix-prefetch-git https://golang.org/x/net --rev f5dfe339be1d06f81b22525fe34671ee7d2c8904 
Leeres Git-Repository in /run/user/1000/git-checkout-tmp-9SXKa8i0/net-f5dfe33/.git/ initialisiert
fatal: https://golang.org/x/net/info/refs not valid: is this a git repository?
fatal: https://golang.org/x/net/info/refs not valid: is this a git repository?

[nix-shell:~/Desktop/projects/nixcloud/dep2nix/src/github.com/nixcloud/dep2nix]$ echo $?
1
the output after the change B (with --quiet)
[nix-shell:~/Desktop/projects/nixcloud/dep2nix/src/github.com/nixcloud/dep2nix]$ ./nix-prefetch-git https://golang.org/x/net --rev f5dfe339be1d06f81b22525fe34671ee7d2c8904 --quiet

[nix-shell:~/Desktop/projects/nixcloud/dep2nix/src/github.com/nixcloud/dep2nix]$ echo $?
1

Things done

@aszlig
Copy link
Member

aszlig commented Feb 16, 2018

@qknight: I'm unable to reproduce this, nix-prefetch-git on https://golang.org/x/net results in the following output with exit code 1:

$ nix-prefetch-git https://golang.org/x/net --rev 9831f2c3ac1068a78f50
Initialized empty Git repository in /tmp/git-checkout-tmp-AREolhSA/net-9831f2c/.git/
fatal: https://golang.org/x/net/info/refs not valid: is this a git repository?
fatal: https://golang.org/x/net/info/refs not valid: is this a git repository?
Unable to checkout 9831f2c3ac1068a78f50 from https://golang.org/x/net.
$ echo $?
1
$ 

You mention https://golang.org/x/net at first but in the example you use https://github.com/ugorji/go. The latter is actually a Git repository.

Do you have the output without these changes applied, but using https://golang.org/x/net?

Copy link
Member Author

@qknight qknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aszlig i've fixed the paste above in the OP. sorry that i pasted the wrong output.

echo 1>&2 "Unable to checkout $hash$ref from $url."
exit 1
)
exit_status=$(checkout_ref "$hash" "$ref" || checkout_hash "$hash" "$ref")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure if this is the proper solution but it seems to work. can someone confirm this is correct code?

@qknight
Copy link
Member Author

qknight commented Feb 19, 2018

@aszlig i've update the example and you should now be able to see that it is broken and that this PR fixes it.

@mmahut
Copy link
Member

mmahut commented Aug 3, 2019

What is the status of this pull request?

@mmahut
Copy link
Member

mmahut commented Oct 5, 2019

Closing due to lack of activity, feel free re-open this if needed.

@mmahut mmahut closed this Oct 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants