hercules-ci / gitignore.nix

Nix functions for filtering local git sources
The Unlicense
243 stars 20 forks source link

gitignoreSource: ensure Nix store path determinism #11

Closed lukateras closed 5 years ago

lukateras commented 5 years ago

This ensures that, when used in situations like gitignoreSource ./., both Hydra and local checkout will have the exact same Nix path, which is important for reproducibility and being able to leverage binary cache.

lukateras commented 5 years ago

Wow, thank you for the fast merge! :)

yorickvP commented 5 years ago

I support having this in here. Actually, I think cleanSourceWith should do this. However,

  1. having 'source' as the path name leads to very unclear error messages.
  2. This PR breaks composability with other cleanSourceWith's.

My old workaround for this problem, which is composable with other cleanSource's:

let
  toConstantSource = name: src: if src ? _isLibCleanSourceWith then builtins.path {
    inherit name;
    inherit (src) filter;
    path = src.origSrc;
  } else builtins.path { inherit name; path = src; };
  constGitIgnore = name: path: toConstantSource name (gitignoreSource path);
lukateras commented 5 years ago
  1. having 'source' as the path name leads to very unclear error messages.

For the record, source is pretty idiomatic. For example, see fetchzip and by extension fetchers built on top of it (like fetchFromGitHub): https://github.com/NixOS/nixpkgs/blob/f3282c8d1e0ce6ba5d9f6aeddcfad51d879c7a4a/pkgs/build-support/fetchzip/default.nix#L14

Which implies that majority of Nixpkgs stdenv derivations have src that uses name = "source". It seems that your concern is similar to that in https://github.com/NixOS/nixpkgs/pull/49862.

  1. This PR breaks composability with other cleanSourceWith's.

True. And that was listed as a feature: https://github.com/hercules-ci/gitignore/blob/ec5dd0536a5e4c3a99c797b86180f7261197c124/README.md#features

However, it's still possible to compose with gitignoreFilter.

lukateras commented 5 years ago

@yorickvP: Also, your workaround doesn't seem to work for me as of Nix 2.2.2. When I try to build a derivation that uses src = lib.cleanSource (constGitIgnore "source" ./.), I get:

string '/nix/store/rwlmxrxs27x7lplhgji74rbfjdxl3nla-source' cannot refer to other paths, at /nix/store/jc62lsfijc5qab87yrbi6aag67jd0lr8-source/lib/sources.nix:49:17

Seems to me that's because constGitIgnore in your example returns builtins.path without all of attributes that cleanSourceWith returns. I'd guess the idea is that it only composes to the right, with constGitIgnore always being the final filter.

yorickvP commented 5 years ago

@yegortimoshenko yeah, it terminates composability, but can still be used as the end to a composed set of cleanSource's.

Regarding (1), having package names in the nix store path may be a mistake altogether.