-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 an optional hash
parameter to builtins.fetchGit
#3216
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also started to have my doubts about refs like in #2582. For example
builtins.fetchGit {
url = https://github.com/NixOS/nixpkgs.git;
ref = "refs/pull/1024/head;
}
While I want to be able to fetch them, they are mutable. Maybe we should discourage the use of them?
The |
I would consider fetchGit with "rev" to be fixed-output and fetchGit with "ref" to be not fixed-output. In Git, branches and tags are considered references, and are not content-addressable (although tags require force push to change). Perhaps another arg could be added like |
@globin and I actually discussed about this last weekend :) The reason why
That's a good idea actually! Will implement this probably next weekend :) |
Could you replace |
To be clear, neither is fixed-output because they're not derivations. But they both produce content-addressable store paths. The difference is that with Morally speaking, fetching using a |
Won't fetching using a |
Yeah but we often want to pin on a tag and guard that with the hash to notice if people force push a new version and still have a pure evaluation. |
That would be tricky because the BTW with flakes it's fine to use just a tag/branch because it will get locked to a specific |
0872bb5
to
1b1b6ab
Compare
sha256
parameter to builtins.fetchGit
hash
parameter to builtins.fetchGit
@edolstra may I ask if there's anything else to add/fix? :) |
@edolstra Matt and I were also talking about using tree hashes, which Nix could verify without storing history. |
This is particularly useful to ensure the purity e.g. of a repo which is pinned to a git tag (tags can be force-pushed which makes builds silently irreproducible) or when trying to evaluate a nix expression without internet connectivity (if the cache-entry is expired, the eval will break as the git repo can't be re-fetched). This change adds an optional `hash` parameter (with an SRI-hash[1]) to the builtin `fetchGit` and checks if there's a content-addressable path which matches the hash (the hash can be determined using `nix to-sri --type sha256 $(nix-prefetch-git ...)`). If no such path exists, it will be attempted to fetch the Git repository in order to compare the hashes. Please note that caching is still used here, so if the repo is already fetched and the only hash in the expression changes, the evaluation will fail pretty fast. [1] https://www.w3.org/TR/SRI/
1b1b6ab
to
24e14cc
Compare
Just updated the patch to work with the refactorings on the store API on master :) |
@edolstra @Ericson2314 is there anything I can address here atm? :) |
The new fetcher infrastructure adds a generic
I think that's better than adding ad hoc support to specific fetchers. Also, the name |
Related: NixOS/nixpkgs#79987 |
@jtojnar did you link this just for documentation purposes? This patch only supports SRI hashes as well :) I'll take a closer look at the new fetcher code later to confirm that this can be closed now. |
@edolstra so I took a closer look at the new fetcher-infrastructure today. I currently have two questions about this:
|
Closing now. The fetcher-API has been backported to nix-master quite recently. I already have a patch locally which adds support for the |
Replacement for NixOS#3216 which became obsolete after the fetchers-API was introduced which provides a `narHash`-argument for each fetcher which is a SRI-hash of a content-addressable path. However, the `fetchGit`-primop required some changes to work with it. To summarize, the following things changed: * The hash-mismatch error in `libfetchers` now returns `102` as exit code which is the default for those errors. * The `git`-fetcher computes an SRI-hash of the result for the hash-validation. * It's possible to specify an SRI-hash using the `narHash`-attribute in `builtins.fetchGit`. This is only allowed if a revision is specified.
The original idea was to implement a git-fetcher in Nix's core that supports content hashes[1]. In NixOS#3549[2] it has been suggested to actually use `fetchTree` for this since it's a fairly generic wrapper over the new fetcher-API[3] and already supports content-hashes. This patch implements a new git-fetcher based on `fetchTree` by incorporating the following changes: * Removed the original `fetchGit`-implementation and replaced it with an alias on the `fetchTree` implementation. * Ensured that the `git`-fetcher from `libfetchers` always computes a content-hash and returns an "empty" revision on dirty trees (the latter one is needed to retain backwards-compatibility). * The hash-mismatch error in the fetcher-API exits with code 102 as it usually happens whenever a hash-mismatch is detected by Nix. * Removed the `flakes`-feature-flag: I didn't see a reason why this API is so tightly coupled to the flakes-API and at least `fetchGit` should remain usable without any feature-flags. * It's only possible to specify a `narHash` for a `git`-tree if either a `ref` or a `rev` is given[4]. * It's now possible to specify an URL without a protocol. If it's missing, `file://` is automatically added as it was the case in the original `fetchGit`-implementation. [1] NixOS#3216 [2] NixOS#3549 (comment) [3] NixOS#3459 [4] NixOS#3216 (comment)
The original idea was to implement a git-fetcher in Nix's core that supports content hashes[1]. In NixOS#3549[2] it has been suggested to actually use `fetchTree` for this since it's a fairly generic wrapper over the new fetcher-API[3] and already supports content-hashes. This patch implements a new git-fetcher based on `fetchTree` by incorporating the following changes: * Removed the original `fetchGit`-implementation and replaced it with an alias on the `fetchTree` implementation. * Ensured that the `git`-fetcher from `libfetchers` always computes a content-hash and returns an "empty" revision on dirty trees (the latter one is needed to retain backwards-compatibility). * The hash-mismatch error in the fetcher-API exits with code 102 as it usually happens whenever a hash-mismatch is detected by Nix. * Removed the `flakes`-feature-flag: I didn't see a reason why this API is so tightly coupled to the flakes-API and at least `fetchGit` should remain usable without any feature-flags. * It's only possible to specify a `narHash` for a `git`-tree if either a `ref` or a `rev` is given[4]. * It's now possible to specify an URL without a protocol. If it's missing, `file://` is automatically added as it was the case in the original `fetchGit`-implementation. [1] NixOS#3216 [2] NixOS#3549 (comment) [3] NixOS#3459 [4] NixOS#3216 (comment)
The original idea was to implement a git-fetcher in Nix's core that supports content hashes[1]. In NixOS#3549[2] it has been suggested to actually use `fetchTree` for this since it's a fairly generic wrapper over the new fetcher-API[3] and already supports content-hashes. This patch implements a new git-fetcher based on `fetchTree` by incorporating the following changes: * Removed the original `fetchGit`-implementation and replaced it with an alias on the `fetchTree` implementation. * Ensured that the `git`-fetcher from `libfetchers` always computes a content-hash and returns an "empty" revision on dirty trees (the latter one is needed to retain backwards-compatibility). * The hash-mismatch error in the fetcher-API exits with code 102 as it usually happens whenever a hash-mismatch is detected by Nix. * Removed the `flakes`-feature-flag: I didn't see a reason why this API is so tightly coupled to the flakes-API and at least `fetchGit` should remain usable without any feature-flags. * It's only possible to specify a `narHash` for a `git`-tree if either a `ref` or a `rev` is given[4]. * It's now possible to specify an URL without a protocol. If it's missing, `file://` is automatically added as it was the case in the original `fetchGit`-implementation. [1] NixOS#3216 [2] NixOS#3549 (comment) [3] NixOS#3459 [4] NixOS#3216 (comment)
The original idea was to implement a git-fetcher in Nix's core that supports content hashes[1]. In NixOS#3549[2] it has been suggested to actually use `fetchTree` for this since it's a fairly generic wrapper over the new fetcher-API[3] and already supports content-hashes. This patch implements a new git-fetcher based on `fetchTree` by incorporating the following changes: * Removed the original `fetchGit`-implementation and replaced it with an alias on the `fetchTree` implementation. * Ensured that the `git`-fetcher from `libfetchers` always computes a content-hash and returns an "empty" revision on dirty trees (the latter one is needed to retain backwards-compatibility). * The hash-mismatch error in the fetcher-API exits with code 102 as it usually happens whenever a hash-mismatch is detected by Nix. * Removed the `flakes`-feature-flag: I didn't see a reason why this API is so tightly coupled to the flakes-API and at least `fetchGit` should remain usable without any feature-flags. * It's only possible to specify a `narHash` for a `git`-tree if either a `ref` or a `rev` is given[4]. * It's now possible to specify an URL without a protocol. If it's missing, `file://` is automatically added as it was the case in the original `fetchGit`-implementation. [1] NixOS#3216 [2] NixOS#3549 (comment) [3] NixOS#3459 [4] NixOS#3216 (comment)
The original idea was to implement a git-fetcher in Nix's core that supports content hashes[1]. In NixOS#3549[2] it has been suggested to actually use `fetchTree` for this since it's a fairly generic wrapper over the new fetcher-API[3] and already supports content-hashes. This patch implements a new git-fetcher based on `fetchTree` by incorporating the following changes: * Removed the original `fetchGit`-implementation and replaced it with an alias on the `fetchTree` implementation. * Ensured that the `git`-fetcher from `libfetchers` always computes a content-hash and returns an "empty" revision on dirty trees (the latter one is needed to retain backwards-compatibility). * The hash-mismatch error in the fetcher-API exits with code 102 as it usually happens whenever a hash-mismatch is detected by Nix. * Removed the `flakes`-feature-flag: I didn't see a reason why this API is so tightly coupled to the flakes-API and at least `fetchGit` should remain usable without any feature-flags. * It's only possible to specify a `narHash` for a `git`-tree if either a `ref` or a `rev` is given[4]. * It's now possible to specify an URL without a protocol. If it's missing, `file://` is automatically added as it was the case in the original `fetchGit`-implementation. [1] NixOS#3216 [2] NixOS#3549 (comment) [3] NixOS#3459 [4] NixOS#3216 (comment)
The original idea was to implement a git-fetcher in Nix's core that supports content hashes[1]. In NixOS#3549[2] it has been suggested to actually use `fetchTree` for this since it's a fairly generic wrapper over the new fetcher-API[3] and already supports content-hashes. This patch implements a new git-fetcher based on `fetchTree` by incorporating the following changes: * Removed the original `fetchGit`-implementation and replaced it with an alias on the `fetchTree` implementation. * Ensured that the `git`-fetcher from `libfetchers` always computes a content-hash and returns an "empty" revision on dirty trees (the latter one is needed to retain backwards-compatibility). * The hash-mismatch error in the fetcher-API exits with code 102 as it usually happens whenever a hash-mismatch is detected by Nix. * Removed the `flakes`-feature-flag: I didn't see a reason why this API is so tightly coupled to the flakes-API and at least `fetchGit` should remain usable without any feature-flags. * It's only possible to specify a `narHash` for a `git`-tree if either a `ref` or a `rev` is given[4]. * It's now possible to specify an URL without a protocol. If it's missing, `file://` is automatically added as it was the case in the original `fetchGit`-implementation. [1] NixOS#3216 [2] NixOS#3549 (comment) [3] NixOS#3459 [4] NixOS#3216 (comment)
cc @globin @xbreak
This is particularly useful to ensure the purity e.g. of a repo which is
pinned to a git tag (tags can be force-pushed which makes builds silently irreproducible)
or when trying to evaluate a nix expression without internet connectivity (if the
cache-entry is expired, the eval will break as the git repo can't be re-fetched).
This change adds an optional
hash
parameter (with an SRI-hash[1]) to the builtinfetchGit
and checks if there's a content-addressable path which matches the hash(the hash can be determined using
nix to-sri --type sha256 $(nix-prefetch-git ...)
).If no such path exists, it will be attempted to fetch the Git repository in
order to compare the hashes. Please note that caching is still used here, so
if the repo is already fetched and the only hash in the expression changes,
the evaluation will fail pretty fast.
[1] https://www.w3.org/TR/SRI/