mkDummySrc doesn't behave well with manually filtered source

[winterqt]

Using the following source filter as the argument to crane.buildPackage:

lib.cleanSourceWith {
  inherit src;
  filter = path: type: lib.hasPrefix "Cargo" (builtins.baseNameOf path) || type == "directory";
}

results in the generated dummy source only containing Cargo.toml.

I don't see anything in mkDummySrc that would be causing this behavior, and my filter is sane (I manually tested to make sure it contains both Cargo.toml and Cargo.lock). My Crane input is at 223aaed. I've also confirmed that the given source works when not filtering.

[ipetkov]

@winterqt totally random question, is the Cargo.lock file at the root of the filtered source?

Right now mkDummySrc assumes that is the case, but perhaps it should keep any Cargo.lock file it finds in the input source and let cargo sort things out afterwards

[winterqt]

@winterqt totally random question, is the Cargo.lock file at the root of the filtered source?

Yup, it is, as is the Cargo.toml.

[ipetkov]

Alright I've spotted the issue:

• mkDummySrc tries to be clever to make its output minimal (i.e. avoid having keeping empty directories and being invalidated if a new directory is added to the source) by first looking for any "interesting" files (namely .cargo/config.toml and Cargo.lock)
• If any interesting files are found, we keep track of their path relative to the source root (e.g. the filter would see /nix/store/hash/foo/bar/.cargo/config and we would keep track of foo/bar/.cargo/config)
• With the data in the previous step we call the actual lib.cleanSourceWith with the assumption that it will have the same root prefix as when we crawl for files
• This assumption appears to be wrong when the input src is itself filtered. My guess is that the file crawling sees a source cleaned with the original filter, while when we do our own lib.cleanSourceWith nix collapses the two filters together, but uses the original source as the input, meaning the hash of the resulting file changes and our cleverness is no more

[winterqt]

meaning the hash of the resulting file changes and our cleverness is no more

I assume you mean the hash in the store path for the final filtered source, right?

Why is the root path taken into account anyways? Is that so the paths can be compared? As in, the prefix is stored, then stripped between two paths to compare, and if they don't match (properly/stripped != /nix/store/.../properly/stripped), it doesn't copy it? Do I understand that correctly?

[ipetkov]

Why is the root path taken into account anyways? Is that so the paths can be compared? As in, the prefix is stored, then stripped between two paths to compare, and if they don't match (properly/stripped != /nix/store/.../properly/stripped), it doesn't copy it? Do I understand that correctly?

Yeah this is a very round-about way of avoiding lib.cleanSourceWith from leaving empty directories lying around.

I think I've found a potential solution, whenever we look for interesting files we should check if the input is a filtered source (and if so use src.origSrc). Otherwise the filtered source gets coerced into a string (and put in the Nix store as its own path) which can lead to mismatched prefixes

[winterqt]

Yeah this is a very round-about way of avoiding lib.cleanSourceWith from leaving empty directories lying around.

When would it leave empty directories around? (Maybe I should read the explanation in mkDummySrc 😅, I'm guessing it's all explained there)

[ipetkov]

If you haven't read it already, hopefully this explains Nix's source filtering behavior (though the rest of the code block gives some extra context around what mkDummySrc is trying to do :)

[winterqt]

Just to make sure I'm understanding this correctly: the directory walking is done at eval time to prevent directories that we don't care about from being included during filtering (due to the top down lazy nature), right?

[ipetkov]

Yes exactly! That prevents something like adding a new directory or changing an irrelevant file from invalidating the cache and rebuilding everything from scratch

[winterqt]

My guess is that the file crawling sees a source cleaned with the original filter, while when we do our own lib.cleanSourceWith nix collapses the two filters together, but uses the original source as the input, meaning the hash of the resulting file changes and our cleverness is no more

By the way, you're exactly right: see lib.cleanSourceWith's usage of toSourceAttributes, which is basically implementing the exact same workaround you describe. (Although I'd recommend doing what they do in which you use the same filter in addition to yours maybe?)

I'd be happy to write and test an implementation of this fix, and PR it if all goes well.

[ipetkov]

@winterqt

(Although I'd recommend doing what they do in which you use the same filter in addition to yours maybe?)

Yep the original filter is honored and the dummy filter is layered on top!

I'd be happy to write and test an implementation of this fix, and PR it if all goes well.

Opened #47 with the fix which seems to work with my testing. Would you try it out and let me know if it works out? If it doesn't I would very much appreciate more tests if you have some to contribute ❤️