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

lib.fileset: Write down the rationale for not supporting store paths #264537

Closed
infinisil opened this issue Oct 31, 2023 · 0 comments · Fixed by #264892
Closed

lib.fileset: Write down the rationale for not supporting store paths #264537

infinisil opened this issue Oct 31, 2023 · 0 comments · Fixed by #264892
Assignees

Comments

@infinisil
Copy link
Member

infinisil commented Oct 31, 2023

lib.fileset does not support filtering of store paths. Rationale for that should be added to the file set library design decisions so I can link to it in the future.

Note that if you need to filter a lib.sources value, that will be supported soon with #261732.

Reasons for lib.fileset not supporting store paths:

  • lib.fileset guarantees that it prevents files that aren't selected from being added into the store, such that changing them doesn't cause a rebuild. But if you filter a store path, all those files are already in the store and will always cause a rebuild if changed, no matter if you filter them out afterwards, so it couldn't guarantee the same. The use case is different.
  • Store paths are only known at build time, but lib.fileset (and lib.sources) rely on builtins.path to do the filtering, which runs at evaluation time, so using it on store paths would require IFD, when it wouldn't be necessary
  • Instead, functions for filtering store paths could be much more powerful by using a build-time derivation to do the filtering. And such an interface wouldn't have to be limited to just filtering, it could also rename, add and move paths. In fact you can do this already today, though it's not very pretty:
    runCommand "lib-filtered" {} ''
      cp -r ${pkgs.hello} $out
      chmod +w -R $out
      
      # Remove ./share and rename ./bin/hello
      rm -rf $out/share
      mv $out/bin/{hello,hallo}
    ''
    Maybe there should just be a simple pkgs.transformStorePath to take care of the boilerplate, this is tracked in Function for transforming store path contents #264541
  • lib.fileset is made very convenient and safe because it supports Path expression syntax to specify paths to include/exclude, like unions [ ./foo ./bar ]. There's no equivalent you could use for store paths with the same properties (using strings "./foo", "./bar" would have to delay checking of file existence and computations on trees to build time to avoid IFD).

Originally posted by @infinisil in #188301 (comment)

This work is sponsored by Antithesis

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 a pull request may close this issue.

1 participant