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

fetchFromGitHub inconsistent hash #39308

Open
matthewbauer opened this issue Apr 21, 2018 · 13 comments
Open

fetchFromGitHub inconsistent hash #39308

matthewbauer opened this issue Apr 21, 2018 · 13 comments
Labels
6.topic: darwin Running or building packages on Darwin

Comments

@matthewbauer
Copy link
Member

matthewbauer commented Apr 21, 2018

Problem

I've been struggling to get fetchFromGitHub to work correctly. It's from darlinghq/darling. Here is the expression:

  src = fetchFromGitHub {
    repo = "darling";
    owner = "darlinghq";
    rev = "d2cc5fa748003aaa70ad4180fff0a9a85dc65e9b";
    sha256 = "1sdl0ysa3yfdvkq0x7vkdl64g7mcfy3qx70saf1d8rnhycbxjgjg";
  };

But, Hydra has reported it's incorrect (on a Darwin machine):

fixed-output derivation produced path '/nix/store/z9zpz2yqx1ixn4xl1lsrk0f83rvp7srb-source' with sha256 hash '1mkcnzy1cfpwghgvb9pszhy9jy6534y8krw8inwl9fqfd0w019wz' instead of the expected hash '1sdl0ysa3yfdvkq0x7vkdl64g7mcfy3qx70saf1d8rnhycbxjgjg'

but then also reports this (on a Linux machine):

output path '/nix/store/z9zpz2yqx1ixn4xl1lsrk0f83rvp7srb-source' has r:sha256 hash '1sdl0ysa3yfdvkq0x7vkdl64g7mcfy3qx70saf1d8rnhycbxjgjg' when '1mkcnzy1cfpwghgvb9pszhy9jy6534y8krw8inwl9fqfd0w019wz' was expected

Jobs:

Attempted fixes

I've gone back and forth between the two with neither working 100% (probably due to cache sharing between Linux/Darwin machines).

85cadf9
9330ef4
6bb3ec8

I think it has something to do with symlinks in the GitHub repo. Maybe there is some impurity introduced by the filesystem?

@dezgeg
Copy link
Contributor

dezgeg commented Apr 22, 2018

Maybe diff those paths? I would guess it's to do something with case-insensitivity or Unicode normalization on MacOS.

@LnL7
Copy link
Member

LnL7 commented Apr 23, 2018

Yeah, we've seen problems like this before because of filename normalisation.

@LnL7
Copy link
Member

LnL7 commented Apr 23, 2018

eg. #26280 (comment)

@LnL7 LnL7 added the 6.topic: darwin Running or building packages on Darwin label Apr 23, 2018
@copumpkin
Copy link
Member

Yeah, Darling has a case overlap:

https://github.com/darlinghq/darling/blob/master/src/libm/Source/Intel/xmmLibm_Prefix.h

vs.

https://github.com/darlinghq/darling/blob/master/src/libm/Source/Intel/xmmLibm_prefix.h

The uppercase one is a symlink to the lowercase one. Just clone the repo on a Mac (or case-insensitive filesystem) and Git will instantly tell you there's an unstaged change, because while cloning it overwrites one file with the other.

@matthewbauer
Copy link
Member Author

Very weird! It looks like Apple's libm has some typos causing that symlink to be necessary.

@copumpkin
Copy link
Member

copumpkin commented Apr 24, 2018

Seems like a hypothetical darling Nix build could inject that symlink at build time on Linux rather than at fetch time on all platforms. So the fetcher removes it unconditionally, and then you have an postUnpack = lib.optionalString isLinux "ln -s .."; in your build to make case line up.

Edit: the fetcher needs to be careful too, because when unpacking the file, it's not guaranteed (I think) which case gets unpacked first, so in some cases the symlink can overwrite the file and in other cases the file can overwrite the symlink. And of course, you can't just blindly remove the overlapping file in postFetch 😄

Someday we can assume APFS across the board and then just stick the Nix store and build staging areas on a case-sensitive mount.

@matthewbauer
Copy link
Member Author

matthewbauer commented Apr 29, 2018

I've opened darlinghq/darling#379 for this. Although it would be nice if we could figure out how to normalize it within Nix.

@copumpkin I wonder if it would be useful to have Nix just remove the case conflicts automatically. This could break some things but even Linux users will occasionally have to work on case insensitive file systems. From what I see case sensitive files are fairly rare in Nix anyway. The current "hash" mismatch is at the very least a confusing error message.

@dezgeg
Copy link
Contributor

dezgeg commented Apr 30, 2018

IIRC case conflicts are actually somewhat common, there are some in ncurses and both in Linux kernel sources and compiled modules. Besides, by the time Nix is inspecting the output it's too late to detect the problem (on a case-insensitive filesystem) as any colliding files have already overwritten each other.

@copumpkin
Copy link
Member

copumpkin commented Apr 30, 2018

@matthewbauer the issue is that Nix never even sees the case conflict, unless I'm misunderstanding you.

If you run git clone on linux, in pseudocode it just does something like the following:

write("fileWithCASEA.txt", "content-for-all-caps")
write("fileWithCaseB.txt", "content-for-not-all-caps")

On Linux, that results in two files. On a case-insensitive (but preserving) filesystem, that produces a single file whose name is fileWithCASEA.txt but whose content is "content-for-not-all-caps". The first line creates the file and the second line overwrites its content. This is why even on a fresh untouched clone of a git repo with conflicts like this (like the Linux one, or the darling one), a git status shows changes: because in the very process of cloning, git is inadvertently making unwanted changes to the files it manages.

By the time Nix sees any of these directory trees to hash the fixed-output derivations, they just contain nonsense with no indication as to how they got that way, and a single hash isn't big enough (or structured enough) to preserve information on individual filenames or such to indicate that a case screwup occurred. The only way I can think of to catch it would be to monitor file creation and write events in child processes, which we could conceivably do inside Nix on macOS with dtrace. Unfortunately, because we use /bin/sh so widely and SIP blocks dtrace on "system binaries" by default, we wouldn't be able to catch bad case behavior in shell scripts interpreted by /bin/sh. But even with all this dtrace wizardry, we'd only be able to emit a warning that someone used inconsistent casing inside a child process. A fair amount of work for a better error message! I'm happy to provide guidance if you want to explore it, but there's also the lingering question of how to monitor children from the Nix side: either we depend on the dtrace binary which isn't currently built by Nix (so you'd need developer tools), or we depend on the library, which also isn't built by Nix.

Somewhat relatedly, @shlevy has done some work on this for Hackage, basically (as I remember it) figuring out how to optimally partition and extract a big tarball with case overlaps to multiple directories so they don't overlap on the target filesystem. This only works (without significant other patch-up work) if the end result doesn't need to coexist though, which isn't the case with darling and Linux because they have a bunch of #include directives that assume the headers are distinguishable. Even if we put the conflicted headers into separate -I paths, it's not as if the header search logic would know how to prioritize the perfect case match over the imperfect one.

@matthewbauer
Copy link
Member Author

So I was thinking of doing something like this for every fetchzip (which is what fetchFromGitHub uses):

find . -type f | tr '[:upper:]' '[:lower:]' | sort | uniq -d | xargs rm -f

Basically just get rid of any files that are case conflicts (realizing now that the above won't actually work, but something like it could). It's probably not worth doing because of how rare case conflicts actually are.

@copumpkin
Copy link
Member

@matthewbauer so you'd kill case conflicts on the Linux side? The kernel source tree uses nontrivial case overlaps extensively.

@matthewbauer
Copy link
Member Author

Ah I did not realize that! It surprises me that that's the case. Anyway, probably best to deal with it in a case by case way.

@domenkozar
Copy link
Member

We should have a check that warns about this, I've just lost an hour or two on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin
Projects
None yet
Development

No branches or pull requests

5 participants