From c1c37f32002e2e2ad167bcac2e5d76a3a3ad6a35 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Mon, 13 Jun 2022 20:39:09 +0200 Subject: [PATCH 1/3] flakes: throw an error if `follows`-declaration for an input is invalid I recently got fairly confused why the following expression didn't have any effect { description = "Foobar"; inputs.sops-nix = { url = github:mic92/sops-nix; inputs.nixpkgs_22_05.follows = "nixpkgs"; }; } until I found out that the input was called `nixpkgs-22_05` (please note the dash vs. underscore). IMHO it's not a good idea to not throw an error in that case and probably leave end-users rather confused, so I implemented a small check for that which basically checks whether `follows`-declaration from overrides actually have corresponding inputs in the transitive flake. In fact this was done by accident already in our own test-suite where the removal of a `follows` was apparently forgotten[1]. Since the key of the `std::map` that holds the `overrides` is a vector and we have to find the last element of each vector (i.e. the override) this has to be done with a for loop in O(n) complexity with `n` being the total amount of overrides (which shouldn't be that large though). Please note that this doesn't work with nested expressions, i.e. inputs.fenix.inputs.nixpkgs.follows = "..."; which is a known problem[2]. For the expression demonstrated above, an error like this will be thrown: error: sops-nix has a `follows'-declaration for a non-existant input nixpkgs_22_05! [1] 2664a216e57169ec57d7f51be1b8383c1be83fd5 [2] https://github.com/NixOS/nix/issues/5790 --- src/libexpr/flake/flake.cc | 23 +++++++++++++++++++++++ tests/flakes.sh | 21 ++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 920726b73..b97780a9c 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -373,6 +373,29 @@ LockedFlake lockFlake( { debug("computing lock file node '%s'", printInputPath(inputPathPrefix)); + auto overrides2 = overrides; + for (auto & [inputPath, inputOverride] : overrides2) { + auto inputPath2(inputPath); + auto follow = inputPath2.back(); + inputPath2.pop_back(); + if (inputPath2 == inputPathPrefix + && flakeInputs.find(follow) == flakeInputs.end() + ) { + std::string root; + for (auto & element : inputPath2) { + root.append(element); + if (element != inputPath2.back()) { + root.append(".inputs."); + } + } + throw Error( + "%s has a `follows'-declaration for a non-existant input %s!", + root, + follow + ); + } + } + /* Get the overrides (i.e. attributes of the form 'inputs.nixops.inputs.nixpkgs.url = ...'). */ for (auto & [id, input] : flakeInputs) { diff --git a/tests/flakes.sh b/tests/flakes.sh index 35cf4d8e7..868c410f4 100644 --- a/tests/flakes.sh +++ b/tests/flakes.sh @@ -801,6 +801,8 @@ nix flake metadata $flakeFollowsA nix flake update $flakeFollowsA +nix flake lock $flakeFollowsA + oldLock="$(cat "$flakeFollowsA/flake.lock")" # Ensure that locking twice doesn't change anything @@ -823,7 +825,6 @@ cat > $flakeFollowsA/flake.nix <$flakeFollowsA/flake.nix <&1 | grep "error: B has a \`follows'-declaration for a non-existant input invalid!" From 411111a3bc0e47520797106e1697aaa11631a101 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 12 Jul 2022 11:22:35 +0200 Subject: [PATCH 2/3] Turn error for non-existant follows into a warning --- src/libexpr/flake/flake.cc | 5 ++--- tests/flakes.sh | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index b97780a9c..c59a42d56 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -373,8 +373,7 @@ LockedFlake lockFlake( { debug("computing lock file node '%s'", printInputPath(inputPathPrefix)); - auto overrides2 = overrides; - for (auto & [inputPath, inputOverride] : overrides2) { + for (auto [inputPath, inputOverride] : overrides) { auto inputPath2(inputPath); auto follow = inputPath2.back(); inputPath2.pop_back(); @@ -388,7 +387,7 @@ LockedFlake lockFlake( root.append(".inputs."); } } - throw Error( + warn( "%s has a `follows'-declaration for a non-existant input %s!", root, follow diff --git a/tests/flakes.sh b/tests/flakes.sh index 868c410f4..9087e1978 100644 --- a/tests/flakes.sh +++ b/tests/flakes.sh @@ -877,4 +877,4 @@ EOF git -C $flakeFollowsA add flake.nix -nix flake lock $flakeFollowsA 2>&1 | grep "error: B has a \`follows'-declaration for a non-existant input invalid!" +nix flake lock $flakeFollowsA 2>&1 | grep "warning: B has a \`follows'-declaration for a non-existant input invalid!" From 1f771065f1353ba462d73641b047b4fb2f02f482 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 12 Jul 2022 11:25:33 +0200 Subject: [PATCH 3/3] Move follows-check into its own function --- src/libexpr/flake/flake.cc | 47 ++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index c59a42d56..906e11251 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -353,26 +353,15 @@ LockedFlake lockFlake( std::vector parents; std::function node, const InputPath & inputPathPrefix, - std::shared_ptr oldNode, - const InputPath & lockRootPath, - const Path & parentPath, - bool trustLock)> - computeLocks; + const FlakeInputs & flakeInputs + )> + checkFollowsDeclarations; - computeLocks = [&]( - const FlakeInputs & flakeInputs, - std::shared_ptr node, + checkFollowsDeclarations = [&]( const InputPath & inputPathPrefix, - std::shared_ptr oldNode, - const InputPath & lockRootPath, - const Path & parentPath, - bool trustLock) - { - debug("computing lock file node '%s'", printInputPath(inputPathPrefix)); - + const FlakeInputs & flakeInputs + ) { for (auto [inputPath, inputOverride] : overrides) { auto inputPath2(inputPath); auto follow = inputPath2.back(); @@ -394,6 +383,30 @@ LockedFlake lockFlake( ); } } + }; + + std::function node, + const InputPath & inputPathPrefix, + std::shared_ptr oldNode, + const InputPath & lockRootPath, + const Path & parentPath, + bool trustLock)> + computeLocks; + + computeLocks = [&]( + const FlakeInputs & flakeInputs, + std::shared_ptr node, + const InputPath & inputPathPrefix, + std::shared_ptr oldNode, + const InputPath & lockRootPath, + const Path & parentPath, + bool trustLock) + { + debug("computing lock file node '%s'", printInputPath(inputPathPrefix)); + + checkFollowsDeclarations(inputPathPrefix, flakeInputs); /* Get the overrides (i.e. attributes of the form 'inputs.nixops.inputs.nixpkgs.url = ...'). */