Skip to content

fix error handling and return code#35017

Closed
qknight wants to merge 1 commit intoNixOS:masterfrom
nixcloud:nix-prefetch-git_error_handling_fix
Closed

fix error handling and return code#35017
qknight wants to merge 1 commit intoNixOS:masterfrom
nixcloud:nix-prefetch-git_error_handling_fix

Conversation

@qknight
Copy link
Copy Markdown
Member

@qknight qknight commented Feb 15, 2018 •
edited
Loading

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

@GrahamcOfBorg GrahamcOfBorg added 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 15, 2018
@aszlig
Copy link
Copy Markdown
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?

qknight
qknight commented Feb 19, 2018
View reviewed changes
Copy link
Copy Markdown
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.

pkgs/build-support/fetchgit/nix-prefetch-git
echo 1>&2 "Unable to checkout $hash$ref from $url."
exit 1
)
exit_status=$(checkout_ref "$hash" "$ref" || checkout_hash "$hash" "$ref")
Copy link
Copy Markdown
Member Author

@qknight qknight Feb 15, 2018

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
Copy Markdown
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.

@thefloweringash thefloweringash mentioned this pull request Jan 24, 2019
Merged
10 tasks
@mmahut
Copy link
Copy Markdown
Member

mmahut commented Aug 3, 2019

What is the status of this pull request?

@mmahut
Copy link
Copy Markdown
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

Reviewers

No reviews

Assignees

No one assigned

Labels

6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@qknight @aszlig @mmahut @GrahamcOfBorg