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

fetchGit: add binary substitution support #8246

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sheldonneuberger-sc
Copy link

@sheldonneuberger-sc sheldonneuberger-sc commented Apr 21, 2023

Motivation

The fetchGit builtin should try to substitute before falling back to git clone. This is useful in a couple of situations:

  • You have users that won't have authorization to clone from the git repo, but that will have have auth to a binary substituter.
  • Your binary substituter has better speed/reliability than your git server.

Related issues:

Context

Tasks:

  • Tested cache hit by observing log output of copying path '/nix/store/3d5nn0bw1ja7wad3pir3a8wp2xp49zsx-source' from 'https://my-nix-cache.internal.com'..., and observing that the git clone log output is no longer occurring.
  • Tested cache miss by observing git clone log output.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@github-actions github-actions bot added the fetching Networking with the outside (non-Nix) world, input locking label Apr 21, 2023
Attrs attrs({
// TODO: do these need real values?
{"lastModified", 0ull},
{"revCount", 0ull},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely cannot provide fake values depending on whether the path was substituted. That would make evaluation non-deterministic.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lastModified and revCount are both calculated from the git repo, which we don't have access to in the case of substitution (just the tree checkout, not the .git dir). Are those values written into a .drv file in the binary cache (I'm new to nix)?

If not, then I think we could require that the user specify narHash+lastModified+revCount if they want substitution to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could make these values lazy, and retrieve them as usual, but only when needed. Though laziness isn't supported by libfetchers yet(?). nix::fetchers::Attrs currently only allows data; no callbacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fetching Networking with the outside (non-Nix) world, input locking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants