From f3032298f5dae1d502fd669836de9a28f4291d6f Mon Sep 17 00:00:00 2001 From: Jonathan Ringer Date: Fri, 15 Mar 2024 20:39:46 -0700 Subject: [PATCH 1/8] Init fetchfromgithub RFC --- rfcs/0xxx-fetch-from-github.md | 114 +++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 rfcs/0xxx-fetch-from-github.md diff --git a/rfcs/0xxx-fetch-from-github.md b/rfcs/0xxx-fetch-from-github.md new file mode 100644 index 000000000..a6c219a1a --- /dev/null +++ b/rfcs/0xxx-fetch-from-github.md @@ -0,0 +1,114 @@ +--- +feature: Default name of fetchFromGithub FOD to include revision +start-date: 2024-03-15 +author: Jonathan Ringer +co-authors: +shepherd-team: +shepherd-leader: +related-issues: TODO: link to old PR +--- + +# Summary +[summary]: #summary + +Updating the hash on Fixed-Output Derivations (FODs) is a very error prone process. It's not intuitive to invalidate the existing hash, attempt to realize the FOD, then replace the hash value with the value that Nix just calculated. This RFC advocates for influencing the default derivation name of the fetchFromGithub helper with the "repo" and "rev" values to ensure that changed URLs force the FOD to be re-built. + +# Motivation +[motivation]: #motivation + +This will hopefully provide immediate feedback that an FOD contains a stale hash. One must either build the FOD without the previous FOD in their Nix store, or run the FOD build with `--check` to verify that the FOD is not stale. Although fetchFromGithub is one of many fetchers; it is very common, and generally has a user specify the source information in granular which makes intent clear. + +As a secondary effect, this will also give a more meaningful name to the FODs than "source". E.g. `/nix/store/...-source -> /nix/store/...-protobuf-v24.1-src` + +# Detailed design +[design]: #detailed-design + +Change the default name of fetchFromGithub fetcher from `"source"` to `lib.sanitizeDerivationName "${repo}-${rev}-src"`. + +# Examples and Interactions +[examples-and-interactions]: #examples-and-interactions + +``` +# previous sha256 is still valid +$ nix-build -A nix-template.src --check +checking outputs of '/nix/store/ib74331l6pl6f8s2hsakf68lhg2jsl5i-nix-template-0.1.4-src.drv'... + +trying https://github.com/jonringer/nix-template/archive/v0.1.4.tar.gz + % Total % Received % Xferd Average Speed Time Time Time Current + Dload Upload Total Spent Left Speed +100 130 100 130 0 0 425 0 --:--:-- --:--:-- --:--:-- 426 +100 27955 0 27955 0 0 36653 0 --:--:-- --:--:-- --:--:-- 311k +unpacking source archive /build/v0.1.4.tar.gz +/nix/store/lfbgmqvpq5365q5ivv6ccck7xg88syw5-nix-template-0.1.4-src + +# explicit commit hash example +$ nix-build -A artyFX.src +this derivation will be built: + /nix/store/ir4k3n5q7nmb2wh533pq1ma1cabyr8h7-openAV-ArtyFX-8c542627d9-src.drv +building '/nix/store/ir4k3n5q7nmb2wh533pq1ma1cabyr8h7-openAV-ArtyFX-8c542627d9-src.drv'... + +trying https://github.com/openAVproductions/openAV-ArtyFX/archive/8c542627d936a01b1d97825e7f26a8e95633f7aa.tar.gz + % Total % Received % Xferd Average Speed Time Time Time Current + Dload Upload Total Spent Left Speed +100 173 100 173 0 0 754 0 --:--:-- --:--:-- --:--:-- 755 +100 627k 0 627k 0 0 604k 0 --:--:-- 0:00:01 --:--:-- 1014k +unpacking source archive /build/8c542627d936a01b1d97825e7f26a8e95633f7aa.tar.gz +/nix/store/dkvcfm90ckrlgmv89s8sr15vcidwlxhs-openAV-ArtyFX-8c542627d9-src +``` + +# Drawbacks +[drawbacks]: #drawbacks + +- All derivations which don't pass a "name" parameter will need to be re-realized + - This will be a download-intensive one-time cost to realize the new FOD derivations. + - NAR hash should not need to be recommputed assuming it was deterministic and not stale. + - Cache should be minimally impacted as NARs are content addressable, thus deterministic sources should not contribute to cache bloat. + - Potential for sources which are no longer available to be broken. + - These can have their name manually set to "source" to perserve previous behavior. + - Ideally source availability would remedied with more appropriate methods. E.g. being made available. +- "Interchangability" with other fetchers is diminished as the derivation name is diferent + - In practice, fetchFromGitHub is never used in this way. It is generally the only fetcher, so there is never another FOD to dedupilicate. +- Out-of-tree repositories may get hash mismatch errors + - If the cause of the mismatch is staleness, this is good and working as intended + - If the cause is non-determinism, this is frustrating. +- Some derivations assume "source" to be the name of sourceRoot + - This has been mitigated over two years within Nixpkgs + - Out-of-tree code may need break if they assume "source" is the name + - Can be mitigated with realease notes describing the issue + +# Alternatives +[alternatives]: #alternatives + +- Do nothing. Retain current status quo. + +- In https://github.com/NixOS/nixpkgs/pull/153386#issuecomment-1007729116, @Ericson2314 mentioned that this may be solved at the Nix tooling level. And that a year should be give to see if an implementation can be done. + - That was 2+ years ago, and an ounce of prevention today is worth ten ounces of remedy tomorrow. + +# Prior art +[prior-art]: #prior-art + +- https://github.com/NixOS/nixpkgs/pull/153386 + +# Unresolved questions +[unresolved]: #unresolved-questions + +- Full commit hashes could be truncated. This sacrifices a bit of simplicity for better looking derivation names: +``` +let + version = builtins.replaceStrings [ "refs/tags/" ] [ "" ] rev; + # Git commit hashes are 40 characters long, assume that very long versions are version-control labels + ref = if (builtins.stringLength rev) > 15 then builtins.substring 0 8 version else version; +in lib.sanitizeDerivationName "${repo}-${ref}-src"; +``` + +- Similar treatment to similar fetchFromGitX helpers? + +# Future work +[future]: #future-work + +- Apply name change to fetchFromGitHub in PR to staging: + - Resolve stale FODs, most of these can be PRs made against master + - Resolve assumed usage of "source" as `src.name`, most of these changes can be made against master + - Revert name to "source" for removed source urls. +- Release notes around potential breakages with usage of "source" + From 6e0bfdf3b03a5eacaa60f2e93f1eeb2eeb704b6a Mon Sep 17 00:00:00 2001 From: Jonathan Ringer Date: Tue, 19 Mar 2024 18:54:54 -0700 Subject: [PATCH 2/8] Determine RFC number --- rfcs/{0xxx-fetch-from-github.md => 0171-fetch-from-github.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename rfcs/{0xxx-fetch-from-github.md => 0171-fetch-from-github.md} (100%) diff --git a/rfcs/0xxx-fetch-from-github.md b/rfcs/0171-fetch-from-github.md similarity index 100% rename from rfcs/0xxx-fetch-from-github.md rename to rfcs/0171-fetch-from-github.md From 31959b7711b132328ad091af8cbf749be09b5795 Mon Sep 17 00:00:00 2001 From: Jonathan Ringer Date: Tue, 19 Mar 2024 18:56:01 -0700 Subject: [PATCH 3/8] Cleanup header --- rfcs/0171-fetch-from-github.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rfcs/0171-fetch-from-github.md b/rfcs/0171-fetch-from-github.md index a6c219a1a..11f9862cb 100644 --- a/rfcs/0171-fetch-from-github.md +++ b/rfcs/0171-fetch-from-github.md @@ -2,10 +2,10 @@ feature: Default name of fetchFromGithub FOD to include revision start-date: 2024-03-15 author: Jonathan Ringer -co-authors: -shepherd-team: +co-authors: +shepherd-team: shepherd-leader: -related-issues: TODO: link to old PR +related-issues: --- # Summary From 0e35da42d7f68294ad21485b494bbd1947b42377 Mon Sep 17 00:00:00 2001 From: Jonathan Ringer Date: Tue, 19 Mar 2024 19:04:26 -0700 Subject: [PATCH 4/8] Update rfcs/0171-fetch-from-github.md --- rfcs/0171-fetch-from-github.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0171-fetch-from-github.md b/rfcs/0171-fetch-from-github.md index 11f9862cb..8fed748bc 100644 --- a/rfcs/0171-fetch-from-github.md +++ b/rfcs/0171-fetch-from-github.md @@ -16,7 +16,7 @@ Updating the hash on Fixed-Output Derivations (FODs) is a very error prone proce # Motivation [motivation]: #motivation -This will hopefully provide immediate feedback that an FOD contains a stale hash. One must either build the FOD without the previous FOD in their Nix store, or run the FOD build with `--check` to verify that the FOD is not stale. Although fetchFromGithub is one of many fetchers; it is very common, and generally has a user specify the source information in granular which makes intent clear. +This will hopefully provide immediate feedback that an FOD contains a stale hash. One must either build the FOD without the previous FOD in their Nix store, or run the FOD build with `--check` to verify that the FOD is not stale. Although fetchFromGithub is one of many fetchers; it is very common, and generally has a user specify granular source information which makes differentiating between sources easy. As a secondary effect, this will also give a more meaningful name to the FODs than "source". E.g. `/nix/store/...-source -> /nix/store/...-protobuf-v24.1-src` From 7534dd488c67a5efe57969bb76fa98d10c9b79ea Mon Sep 17 00:00:00 2001 From: Jonathan Ringer Date: Tue, 19 Mar 2024 19:04:38 -0700 Subject: [PATCH 5/8] Update rfcs/0171-fetch-from-github.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Schütz --- rfcs/0171-fetch-from-github.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rfcs/0171-fetch-from-github.md b/rfcs/0171-fetch-from-github.md index 8fed748bc..8860bbc1c 100644 --- a/rfcs/0171-fetch-from-github.md +++ b/rfcs/0171-fetch-from-github.md @@ -61,20 +61,20 @@ unpacking source archive /build/8c542627d936a01b1d97825e7f26a8e95633f7aa.tar.gz - All derivations which don't pass a "name" parameter will need to be re-realized - This will be a download-intensive one-time cost to realize the new FOD derivations. - - NAR hash should not need to be recommputed assuming it was deterministic and not stale. + - NAR hash should not need to be recomputed assuming it was deterministic and not stale. - Cache should be minimally impacted as NARs are content addressable, thus deterministic sources should not contribute to cache bloat. - Potential for sources which are no longer available to be broken. - These can have their name manually set to "source" to perserve previous behavior. - - Ideally source availability would remedied with more appropriate methods. E.g. being made available. -- "Interchangability" with other fetchers is diminished as the derivation name is diferent + - Ideally source availability would be remedied with more appropriate methods. E.g. being made available. +- "Interchangeability" with other fetchers is diminished as the derivation name is different - In practice, fetchFromGitHub is never used in this way. It is generally the only fetcher, so there is never another FOD to dedupilicate. - Out-of-tree repositories may get hash mismatch errors - If the cause of the mismatch is staleness, this is good and working as intended - If the cause is non-determinism, this is frustrating. - Some derivations assume "source" to be the name of sourceRoot - This has been mitigated over two years within Nixpkgs - - Out-of-tree code may need break if they assume "source" is the name - - Can be mitigated with realease notes describing the issue + - Out-of-tree code may break if they assume "source" is the name + - Can be mitigated with release notes describing the issue # Alternatives [alternatives]: #alternatives From 1ac472d4f0b82ec9f6781cd8c5a4f88fabef631e Mon Sep 17 00:00:00 2001 From: Jonathan Ringer Date: Tue, 19 Mar 2024 19:11:15 -0700 Subject: [PATCH 6/8] Update rfcs/0171-fetch-from-github.md --- rfcs/0171-fetch-from-github.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0171-fetch-from-github.md b/rfcs/0171-fetch-from-github.md index 8860bbc1c..673ae499d 100644 --- a/rfcs/0171-fetch-from-github.md +++ b/rfcs/0171-fetch-from-github.md @@ -110,5 +110,5 @@ in lib.sanitizeDerivationName "${repo}-${ref}-src"; - Resolve stale FODs, most of these can be PRs made against master - Resolve assumed usage of "source" as `src.name`, most of these changes can be made against master - Revert name to "source" for removed source urls. -- Release notes around potential breakages with usage of "source" +- Add release notes for potential breakages when assuming the name of the unpacked directory is "source" From 401c9fda1e38102483f1fcd5646e7f473f1a7474 Mon Sep 17 00:00:00 2001 From: Jonathan Ringer Date: Wed, 3 Apr 2024 14:32:22 -0700 Subject: [PATCH 7/8] Update rfcs/0171-fetch-from-github.md Co-authored-by: Yueh-Shun Li --- rfcs/0171-fetch-from-github.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rfcs/0171-fetch-from-github.md b/rfcs/0171-fetch-from-github.md index 673ae499d..f6835fee2 100644 --- a/rfcs/0171-fetch-from-github.md +++ b/rfcs/0171-fetch-from-github.md @@ -66,8 +66,9 @@ unpacking source archive /build/8c542627d936a01b1d97825e7f26a8e95633f7aa.tar.gz - Potential for sources which are no longer available to be broken. - These can have their name manually set to "source" to perserve previous behavior. - Ideally source availability would be remedied with more appropriate methods. E.g. being made available. -- "Interchangeability" with other fetchers is diminished as the derivation name is different - - In practice, fetchFromGitHub is never used in this way. It is generally the only fetcher, so there is never another FOD to dedupilicate. +- "Interchangeability" with other fetchers is diminished as the derivation name is different. + - Since Nixpkgs 23.11, `sourceRoot` is required to be `src.name` or its sub-directrory when `src` is produced by `fetchzip`-based fetchers. This guarantees the interchangeability of these fetchers. + - For other fetchers, such as `fetchurl`, one can specify a relevant `sourceRoot` value alongside the new `src`. - Out-of-tree repositories may get hash mismatch errors - If the cause of the mismatch is staleness, this is good and working as intended - If the cause is non-determinism, this is frustrating. From 7a91d9849dad4c4ba93903627d7d86fd17dc0fa6 Mon Sep 17 00:00:00 2001 From: Jonathan Ringer Date: Sat, 6 Apr 2024 07:29:43 -0700 Subject: [PATCH 8/8] Update rfcs/0171-fetch-from-github.md Co-authored-by: Pieter Gerets <123326988+gerpiet@users.noreply.github.com> --- rfcs/0171-fetch-from-github.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0171-fetch-from-github.md b/rfcs/0171-fetch-from-github.md index f6835fee2..b8cb00074 100644 --- a/rfcs/0171-fetch-from-github.md +++ b/rfcs/0171-fetch-from-github.md @@ -64,7 +64,7 @@ unpacking source archive /build/8c542627d936a01b1d97825e7f26a8e95633f7aa.tar.gz - NAR hash should not need to be recomputed assuming it was deterministic and not stale. - Cache should be minimally impacted as NARs are content addressable, thus deterministic sources should not contribute to cache bloat. - Potential for sources which are no longer available to be broken. - - These can have their name manually set to "source" to perserve previous behavior. + - These can have their name manually set to "source" to preserve previous behavior. - Ideally source availability would be remedied with more appropriate methods. E.g. being made available. - "Interchangeability" with other fetchers is diminished as the derivation name is different. - Since Nixpkgs 23.11, `sourceRoot` is required to be `src.name` or its sub-directrory when `src` is produced by `fetchzip`-based fetchers. This guarantees the interchangeability of these fetchers.