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

Error on more hash mismatch cases #3653

Closed

Conversation

matthewbauer
Copy link
Member

instead of asserting, show the user what was expected vs. what was
handled.

This condition is very close to the one right after it, but this one
happens when the tree narHash is not equal to input narHash.

instead of asserting, show the user what was expected vs. what was
handled.

This condition is very close to the one right after it, but this one
happens when the tree narHash is not equal to input narHash.
assert(input->narHash == tree.info.narHash);
if (input->narHash && input->narHash != tree.info.narHash)
throw Error("NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'",
to_string(), tree.actualPath, input->narHash->to_string(SRI), tree.info.narHash.to_string(SRI));
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should go in fetchTreeInternal? Nevermind that is the virtual method.

Copy link
Member Author

Choose a reason for hiding this comment

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

narHash is in a valid attr for all inputs, so I think all of them need to handle this case. it's replacing an assert that was there previously.

@matthewbauer
Copy link
Member Author

matthewbauer commented Jun 3, 2020

Example of what happens without this change:

builtins.fetchTree {
  type = "git";
  url = "https://github.com/nixos/nix.git";
  rev = "bfa1acd85c4d15c5ea95337138f47672659e2a9e";
  narHash = "sha256-0000000000000000000000000000000000000000000=";
}
Assertion failed: (input->narHash == tree.info.narHash), function fetchTree, file src/libfetchers/fetchers.cc, line 66.
Abort trap: 6

@Ericson2314
Copy link
Member

Ericson2314 commented Jun 3, 2020

@edolstra I am not sure what to make of PR this, as I am stuck trying to understand why fetchTree{,Internal} returns another input. That seems dubious to me, and changing that would avoid the issue @matthewbauer found.

@Ericson2314
Copy link
Member

@edolstra nevermind, I think this is just a good change, and my "why two inputs" questions are orthogonal.

@matthewbauer
Copy link
Member Author

This is fixed on flakes branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants