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

add a filter function argument to builtins.fetchGit #3171

Closed
wants to merge 1 commit into from

Conversation

basvandijk
Copy link
Member

Fixes: #2944.

When working with monorepos containing projects in sub directories
each sub project typically has a derivation where the src attribute
is set to the path of the sub directory.

That setup is not ideal since all files below that sub directory,
including build artefacts, temporary files or even secrets, will be
copied to the store. Ideally only files added to the git repository
should be copied to the store to ensure local nix builds are
reproducible on CI.

Note that builtins.fetchGit can be used to copy a local git
repository to the store. This commit extends builtins.fetchGit with
a filter function argument which can be used to only include a sub
directory.

The filter function has the same type as expected by
builtins.filterSource and has the same semantics except that the
path is relative to the git checkout.

@edolstra at NixCon you mentioned we should possibly think about a more compositional API since most builtin fetch functions now take a filter argument. However this addition provides in a common need and prevents users from IFD. Let me know what you think.

When working with monorepos containing projects in sub directories
each sub project typically has a derivation where the `src` attribute
is set to the path of the sub directory.

That setup is not ideal since all files below that sub directory,
including build artefacts, temporary files or even secrets, will be
copied to the store. Ideally only files added to the git repository
should be copied to the store to ensure local nix builds are
reproducible on CI.

Note that `builtins.fetchGit` can be used to copy a local git
repository to the store. This commit extends `builtins.fetchGit` with
a filter function argument which can be used to only include a sub
directory.

The filter function has the same type as expected by
`builtins.filterSource` and has the same semantics except that the
path is relative to the git checkout.
@stale
Copy link

stale bot commented Feb 13, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Feb 13, 2021
@stale
Copy link

stale bot commented Apr 16, 2022

I closed this issue due to inactivity. → More info

@stale stale bot closed this Apr 16, 2022
@dminuoso dminuoso mentioned this pull request Jan 16, 2023
@Ericson2314 Ericson2314 reopened this Jan 19, 2023
@stale stale bot removed the stale label Jan 19, 2023
@roberth
Copy link
Member

roberth commented Jan 20, 2023

@Ericson2314 I see you've reopened this PR, but we should consider the other implementation strategy, which is to compose a filtering lazy tree onto the git lazy tree. This solution has better composability and separation of concerns, with the only "downside" being that lazy trees are still in development. However, I am confident that lazy trees are a good idea, so considering the scarcity of maintainer time, I think we should work towards a better lazy trees implementation.

@fricklerhandwerk fricklerhandwerk added feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc labels Mar 3, 2023
@roberth roberth added the fetching Networking with the outside (non-Nix) world label May 12, 2023
@thufschmitt
Copy link
Member

Notes from the maintainers team meeting

  • For everything but secrets, adding a builtins.filterSource on top should do most of what's needed (modulo the fact that it's doing more IO)
  • For secrets, could something like one of the nix-gitignore implementations do the trick? It would do a git-like filtering, plus allowing to exclude arbitrary other files

Otherwise, we'd rather go the lazy-trees (#6530) route as it would provide a more generic solution to this problem.

@infinisil
Copy link
Member

infinisil commented Jun 19, 2023

@edolstra at NixCon you mentioned we should possibly think about a more compositional API since most builtin fetch functions now take a filter argument.

I recently worked on a very compositional API that's easy and safe to use for this purpose! It's implemented as a draft PR to Nixpkgs and the interface needs some feedback to figure out whether it satisfies all necessary use cases. Please take a look, try it out and give feedback! Here's a Discourse post giving a brief tour 😃 https://discourse.nixos.org/t/easy-source-filtering-with-file-sets/29117

@roberth
Copy link
Member

roberth commented Jun 19, 2023

easy and safe to use for this purpose!

Not if you're looking for a fetchGit that doesn't but certain files in the store ever. With the Nix language, you can currently only achieve this by either manually parsing the files of a local repository, or by fetching through a derivation (IFD / Read From Realisation (is that what we're going for now?)).

Anyway, most repositories don't have a performance or confidentiality problem, so do check out the link to learn how to filter better and help validate the design! Let's go

@fricklerhandwerk
Copy link
Contributor

This PR is very much out of date, and the file set library already solves part of the issue, therefore closing it.

Current state of discussion on the other part, lazy fetching: #2944 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request or proposal fetching Networking with the outside (non-Nix) world language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

builtins.gitLsFiles
6 participants