From 1fd0867389c2dd3e98d06decd4d35067885550a0 Mon Sep 17 00:00:00 2001 From: Felix Uhl Date: Fri, 11 Aug 2023 21:47:16 +0200 Subject: [PATCH 1/4] Fix missing output when creating lockfile --- src/libexpr/flake/flake.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 45c9ec8f3..8cc803ccf 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -651,14 +651,14 @@ LockedFlake lockFlake( bool lockFileExists = pathExists(outputLockFilePath); + auto s = chomp(diff); if (lockFileExists) { - auto s = chomp(diff); if (s.empty()) warn("updating lock file '%s'", outputLockFilePath); else warn("updating lock file '%s':\n%s", outputLockFilePath, s); } else - warn("creating lock file '%s'", outputLockFilePath); + warn("creating lock file '%s': \n%s", outputLockFilePath, s); std::optional commitMessage = std::nullopt; From c762b65dc5314ed631381cf4bf26f5976e825bdc Mon Sep 17 00:00:00 2001 From: Felix Uhl Date: Fri, 11 Aug 2023 21:51:03 +0200 Subject: [PATCH 2/4] Fix documentation of flake command output --- src/nix/flake-lock.md | 7 +++++-- src/nix/flake-update.md | 11 ++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/nix/flake-lock.md b/src/nix/flake-lock.md index 2af0ad81e..100987a88 100644 --- a/src/nix/flake-lock.md +++ b/src/nix/flake-lock.md @@ -7,8 +7,11 @@ R""( ```console # nix flake lock --update-input nixpkgs --update-input nix - * Updated 'nix': 'github:NixOS/nix/9fab14adbc3810d5cc1f88672fde1eee4358405c' -> 'github:NixOS/nix/8927cba62f5afb33b01016d5c4f7f8b7d0adde3c' - * Updated 'nixpkgs': 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' -> 'github:NixOS/nixpkgs/a3a3dda3bacf61e8a39258a0ed9c924eeca8e293' + warning: creating lock file '/home/myself/repos/testflake/flake.lock': + • Added input 'nix': + 'github:NixOS/nix/9fab14adbc3810d5cc1f88672fde1eee4358405c' (2023-06-28) + • Added input 'nixpkgs': + 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' (2023-06-30) ``` # Description diff --git a/src/nix/flake-update.md b/src/nix/flake-update.md index 8c6042d94..b5a5ff0ec 100644 --- a/src/nix/flake-update.md +++ b/src/nix/flake-update.md @@ -6,9 +6,14 @@ R""( lock file: ```console - # nix flake update --commit-lock-file - * Updated 'nix': 'github:NixOS/nix/9fab14adbc3810d5cc1f88672fde1eee4358405c' -> 'github:NixOS/nix/8927cba62f5afb33b01016d5c4f7f8b7d0adde3c' - * Updated 'nixpkgs': 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' -> 'github:NixOS/nixpkgs/a3a3dda3bacf61e8a39258a0ed9c924eeca8e293' + # nix flake update + warning: updating lock file '/home/myself/repos/testflake/flake.lock': + • Updated input 'nix': + 'github:NixOS/nix/9fab14adbc3810d5cc1f88672fde1eee4358405c' (2023-06-28) + → 'github:NixOS/nix/8927cba62f5afb33b01016d5c4f7f8b7d0adde3c' (2023-07-11) + • Updated input 'nixpkgs': + 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' (2023-06-30) + → 'github:NixOS/nixpkgs/a3a3dda3bacf61e8a39258a0ed9c924eeca8e293' (2023-07-05) … warning: committed new revision '158bcbd9d6cc08ab859c0810186c1beebc982aad' ``` From c7dcdb8325be7b8ecc3d480217808be899fc865a Mon Sep 17 00:00:00 2001 From: Felix Uhl Date: Sat, 12 Aug 2023 20:51:19 +0200 Subject: [PATCH 3/4] Overhaul nix flake update and lock commands Closes #5110 --- doc/manual/src/release-notes/rl-next.md | 14 +++++++ src/libcmd/command.hh | 6 +++ src/libcmd/installables.cc | 22 +--------- src/libexpr/flake/flake.cc | 11 +++-- src/nix/flake-lock.md | 45 ++++++++++----------- src/nix/flake-update.md | 53 +++++++++++++++++-------- src/nix/flake.cc | 33 +++++++++++++-- tests/functional/completions.sh | 7 ++-- tests/functional/flakes/circular.sh | 3 +- tests/functional/flakes/flakes.sh | 6 +-- tests/functional/flakes/follow-paths.sh | 4 +- 11 files changed, 124 insertions(+), 80 deletions(-) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 276252c37..3cfb53998 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -15,3 +15,17 @@ - `nix-shell` shebang lines now support single-quoted arguments. - `builtins.fetchTree` is now marked as stable. + + +- The interface for creating and updating lock files has been overhauled: + + - [`nix flake lock`](@docroot@/command-ref/new-cli/nix3-flake-lock.md) only creates lock files and adds missing inputs now. + It will *never* update existing inputs. + + - [`nix flake update`](@docroot@/command-ref/new-cli/nix3-flake-update.md) does the same, but *will* update inputs. + - Passing no arguments will update all inputs of the current flake, just like it already did. + - Passing input names as arguments will ensure only those are updated. This replaces the functionality of `nix flake lock --update-input` + - To operate on a flake outside the current directory, you must now pass `--flake path/to/flake`. + + - The flake-specific flags `--recreate-lock-file` and `--update-input` have been removed from all commands operating on installables. + They are superceded by `nix flake update`. diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index dafc0db3b..120c832ac 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -326,6 +326,12 @@ struct MixEnvironment : virtual Args { void setEnviron(); }; +void completeFlakeInputPath( + AddCompletions & completions, + ref evalState, + const std::vector & flakeRefs, + std::string_view prefix); + void completeFlakeRef(AddCompletions & completions, ref store, std::string_view prefix); void completeFlakeRefWithFragment( diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index eff18bbf6..3aff601e0 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -28,7 +28,7 @@ namespace nix { -static void completeFlakeInputPath( +void completeFlakeInputPath( AddCompletions & completions, ref evalState, const std::vector & flakeRefs, @@ -46,13 +46,6 @@ MixFlakeOptions::MixFlakeOptions() { auto category = "Common flake-related options"; - addFlag({ - .longName = "recreate-lock-file", - .description = "Recreate the flake's lock file from scratch.", - .category = category, - .handler = {&lockFlags.recreateLockFile, true} - }); - addFlag({ .longName = "no-update-lock-file", .description = "Do not allow any updates to the flake's lock file.", @@ -85,19 +78,6 @@ MixFlakeOptions::MixFlakeOptions() .handler = {&lockFlags.commitLockFile, true} }); - addFlag({ - .longName = "update-input", - .description = "Update a specific flake input (ignoring its previous entry in the lock file).", - .category = category, - .labels = {"input-path"}, - .handler = {[&](std::string s) { - lockFlags.inputUpdates.insert(flake::parseInputPath(s)); - }}, - .completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) { - completeFlakeInputPath(completions, getEvalState(), getFlakeRefsForCompletion(), prefix); - }} - }); - addFlag({ .longName = "override-input", .description = "Override a specific flake input (e.g. `dwarffs/nixpkgs`). This implies `--no-write-lock-file`.", diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 8cc803ccf..70ae7b584 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -447,8 +447,8 @@ LockedFlake lockFlake( assert(input.ref); - /* Do we have an entry in the existing lock file? And we - don't have a --update-input flag for this input? */ + /* Do we have an entry in the existing lock file? + And the input is not in updateInputs? */ std::shared_ptr oldLock; updatesUsed.insert(inputPath); @@ -472,9 +472,8 @@ LockedFlake lockFlake( node->inputs.insert_or_assign(id, childNode); - /* If we have an --update-input flag for an input - of this input, then we must fetch the flake to - update it. */ + /* If we have this input in updateInputs, then we + must fetch the flake to update it. */ auto lb = lockFlags.inputUpdates.lower_bound(inputPath); auto mustRefetch = @@ -616,7 +615,7 @@ LockedFlake lockFlake( for (auto & i : lockFlags.inputUpdates) if (!updatesUsed.count(i)) - warn("the flag '--update-input %s' does not match any input", printInputPath(i)); + warn("'%s' does not match any input of this flake", printInputPath(i)); /* Check 'follows' inputs. */ newLockFile.check(); diff --git a/src/nix/flake-lock.md b/src/nix/flake-lock.md index 100987a88..6d10258e3 100644 --- a/src/nix/flake-lock.md +++ b/src/nix/flake-lock.md @@ -2,11 +2,10 @@ R""( # Examples -* Update the `nixpkgs` and `nix` inputs of the flake in the current - directory: +* Create the lock file for the flake in the current directory: ```console - # nix flake lock --update-input nixpkgs --update-input nix + # nix flake lock warning: creating lock file '/home/myself/repos/testflake/flake.lock': • Added input 'nix': 'github:NixOS/nix/9fab14adbc3810d5cc1f88672fde1eee4358405c' (2023-06-28) @@ -14,28 +13,28 @@ R""( 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' (2023-06-30) ``` +* Add missing inputs to the lock file for a flake in a different directory: + + ```console + # nix flake lock ~/repos/another + warning: updating lock file '/home/myself/repos/another/flake.lock': + • Added input 'nixpkgs': + 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' (2023-06-30) + ``` + + > **Note** + > + > When trying to refer to a flake in a subdirectory, write `./another` + > instead of `another`. + > Otherwise Nix will try to look up the flake in the registry. + # Description -This command updates the lock file of a flake (`flake.lock`) so that -it contains a lock for every flake input specified in -`flake.nix`. Existing lock file entries are not updated unless -required by a flag such as `--update-input`. +This command adds inputs to the lock file of a flake (`flake.lock`) +so that it contains a lock for every flake input specified in +`flake.nix`. Existing lock file entries are not updated. -Note that every command that operates on a flake will also update the -lock file if needed, and supports the same flags. Therefore, - -```console -# nix flake lock --update-input nixpkgs -# nix build -``` - -is equivalent to: - -```console -# nix build --update-input nixpkgs -``` - -Thus, this command is only useful if you want to update the lock file -separately from any other action such as building. +If you want to update existing lock entries, use +[`nix flake update`](@docroot@/command-ref/new-cli/nix3-flake-update.md) )"" diff --git a/src/nix/flake-update.md b/src/nix/flake-update.md index b5a5ff0ec..63df3b12a 100644 --- a/src/nix/flake-update.md +++ b/src/nix/flake-update.md @@ -2,8 +2,7 @@ R""( # Examples -* Recreate the lock file (i.e. update all inputs) and commit the new - lock file: +* Update all inputs (i.e. recreate the lock file from scratch): ```console # nix flake update @@ -14,26 +13,46 @@ R""( • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' (2023-06-30) → 'github:NixOS/nixpkgs/a3a3dda3bacf61e8a39258a0ed9c924eeca8e293' (2023-07-05) - … - warning: committed new revision '158bcbd9d6cc08ab859c0810186c1beebc982aad' ``` +* Update only a single input: + + ```console + # nix flake update nixpkgs + warning: updating lock file '/home/myself/repos/testflake/flake.lock': + • Updated input 'nixpkgs': + 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' (2023-06-30) + → 'github:NixOS/nixpkgs/a3a3dda3bacf61e8a39258a0ed9c924eeca8e293' (2023-07-05) + ``` + +* Update only a single input of a flake in a different directory: + + ```console + # nix flake update nixpkgs --flake ~/repos/another + warning: updating lock file '/home/myself/repos/another/flake.lock': + • Updated input 'nixpkgs': + 'github:NixOS/nixpkgs/3d2d8f281a27d466fa54b469b5993f7dde198375' (2023-06-30) + → 'github:NixOS/nixpkgs/a3a3dda3bacf61e8a39258a0ed9c924eeca8e293' (2023-07-05) + ``` + + > **Note** + > + > When trying to refer to a flake in a subdirectory, write `./another` + > instead of `another`. + > Otherwise Nix will try to look up the flake in the registry. + # Description -This command recreates the lock file of a flake (`flake.lock`), thus -updating the lock for every unlocked input (like `nixpkgs`) to its -current version. This is equivalent to passing `--recreate-lock-file` -to any command that operates on a flake. That is, +This command updates the inputs in a lock file (`flake.lock`). +**By default, all inputs are updated**. If the lock file doesn't exist +yet, it will be created. If inputs are not in the lock file yet, they will be added. -```console -# nix flake update -# nix build -``` +Unlike other `nix flake` commands, `nix flake update` takes a list of names of inputs +to update as its positional arguments and operates on the flake in the current directory. +You can pass a different flake-url with `--flake` to override that default. -is equivalent to: - -```console -# nix build --recreate-lock-file -``` +The related command [`nix flake lock`](@docroot@/command-ref/new-cli/nix3-flake-lock.md) +also creates lock files and adds missing inputs, but is safer as it +will never update inputs already in the lock file. )"" diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 0116eff2e..e8906a252 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -24,8 +24,10 @@ using namespace nix; using namespace nix::flake; using json = nlohmann::json; +struct CmdFlakeUpdate; class FlakeCommand : virtual Args, public MixFlakeOptions { +protected: std::string flakeUrl = "."; public: @@ -63,6 +65,8 @@ public: struct CmdFlakeUpdate : FlakeCommand { +public: + std::string description() override { return "update flake lock file"; @@ -70,9 +74,31 @@ struct CmdFlakeUpdate : FlakeCommand CmdFlakeUpdate() { + expectedArgs.clear(); + addFlag({ + .longName="flake", + .description="The flake to operate on. Default is the current directory.", + .labels={"flake-url"}, + .handler={&flakeUrl}, + .completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) { + completeFlakeRef(completions, getStore(), prefix); + }} + }); + expectArgs({ + .label="inputs", + .optional=true, + .handler={[&](std::string inputToUpdate){ + auto inputPath = flake::parseInputPath(inputToUpdate); + if (lockFlags.inputUpdates.contains(inputPath)) + warn("Input '%s' was specified multiple times. You may have done this by accident."); + lockFlags.inputUpdates.insert(inputPath); + }}, + .completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) { + completeFlakeInputPath(completions, getEvalState(), getFlakeRefsForCompletion(), prefix); + }} + }); + /* Remove flags that don't make sense. */ - removeFlag("recreate-lock-file"); - removeFlag("update-input"); removeFlag("no-update-lock-file"); removeFlag("no-write-lock-file"); } @@ -87,8 +113,9 @@ struct CmdFlakeUpdate : FlakeCommand void run(nix::ref store) override { settings.tarballTtl = 0; + auto updateAll = lockFlags.inputUpdates.empty(); - lockFlags.recreateLockFile = true; + lockFlags.recreateLockFile = updateAll; lockFlags.writeLockFile = true; lockFlags.applyNixConfig = true; diff --git a/tests/functional/completions.sh b/tests/functional/completions.sh index 7c1e4b287..b9886623a 100644 --- a/tests/functional/completions.sh +++ b/tests/functional/completions.sh @@ -48,11 +48,10 @@ EOF [[ "$(NIX_GET_COMPLETIONS=5 nix build ./foo ./bar --override-input '')" == $'normal\na\t\nb\t' ]] ## With tilde expansion [[ "$(HOME=$PWD NIX_GET_COMPLETIONS=4 nix build '~/foo' --override-input '')" == $'normal\na\t' ]] -[[ "$(HOME=$PWD NIX_GET_COMPLETIONS=5 nix flake show '~/foo' --update-input '')" == $'normal\na\t' ]] -[[ "$(HOME=$PWD NIX_GET_COMPLETIONS=4 nix run '~/foo' --update-input '')" == $'normal\na\t' ]] +[[ "$(HOME=$PWD NIX_GET_COMPLETIONS=5 nix flake update --flake '~/foo' '')" == $'normal\na\t' ]] ## Out of order -[[ "$(NIX_GET_COMPLETIONS=3 nix build --update-input '' ./foo)" == $'normal\na\t' ]] -[[ "$(NIX_GET_COMPLETIONS=4 nix build ./foo --update-input '' ./bar)" == $'normal\na\t\nb\t' ]] +[[ "$(NIX_GET_COMPLETIONS=3 nix build --override-input '' '' ./foo)" == $'normal\na\t' ]] +[[ "$(NIX_GET_COMPLETIONS=4 nix build ./foo --override-input '' '' ./bar)" == $'normal\na\t\nb\t' ]] # Cli flag completion NIX_GET_COMPLETIONS=2 nix build --log-form | grep -- "--log-format" diff --git a/tests/functional/flakes/circular.sh b/tests/functional/flakes/circular.sh index 09cd02edf..d3bb8e8a3 100644 --- a/tests/functional/flakes/circular.sh +++ b/tests/functional/flakes/circular.sh @@ -42,7 +42,8 @@ git -C $flakeB commit -a -m 'Foo' sed -i $flakeB/flake.nix -e 's/456/789/' git -C $flakeB commit -a -m 'Foo' -[[ $(nix eval --update-input b $flakeA#foo) = 1912 ]] +nix flake update b --flake $flakeA +[[ $(nix eval $flakeA#foo) = 1912 ]] # Test list-inputs with circular dependencies nix flake metadata $flakeA diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index 70de28628..b0038935c 100644 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -300,7 +300,7 @@ nix build -o "$TEST_ROOT/result" flake4#xyzzy nix flake lock "$flake3Dir" [[ -z $(git -C "$flake3Dir" diff master || echo failed) ]] -nix flake update "$flake3Dir" --override-flake flake2 nixpkgs +nix flake update --flake "$flake3Dir" --override-flake flake2 nixpkgs [[ ! -z $(git -C "$flake3Dir" diff master || echo failed) ]] # Make branch "removeXyzzy" where flake3 doesn't have xyzzy anymore @@ -437,7 +437,7 @@ cat > "$flake3Dir/flake.nix" < Date: Tue, 24 Oct 2023 22:22:05 +0200 Subject: [PATCH 4/4] fix: segfault in positional arg completion Adding the inputPath as a positional feature uncovered this bug. As positional argument forms were discarded from the `expectedArgs` list, their closures were not. When the `.completer` closure was then called, part of the surrounding object did not exist anymore. This didn't cause an issue before, but with the new call to `getEvalState()` in the "inputs" completer in nix/flake.cc, a segfault was triggered reproducibly on invalid memory access to the `this` pointer, which was always 0. The solution of splicing the argument forms into a new list to extend their lifetime is a bit of a hack, but I was unable to get the "nicer" iterator-based solution to work. --- src/libutil/args.cc | 13 ++++++++++++- src/libutil/args.hh | 14 +++++++++++++- tests/functional/completions.sh | 4 ++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/libutil/args.cc b/src/libutil/args.cc index 6bc3cae07..811353c18 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -255,7 +255,18 @@ bool Args::processArgs(const Strings & args, bool finish) } if (!anyCompleted) exp.handler.fun(ss); - expectedArgs.pop_front(); + + /* Move the list element to the processedArgs. This is almost the same as + `processedArgs.push_back(expectedArgs.front()); expectedArgs.pop_front()`, + except that it will only adjust the next and prev pointers of the list + elements, meaning the actual contents don't move in memory. This is + critical to prevent invalidating internal pointers! */ + processedArgs.splice( + processedArgs.end(), + expectedArgs, + expectedArgs.begin(), + ++expectedArgs.begin()); + res = true; } diff --git a/src/libutil/args.hh b/src/libutil/args.hh index ff2bf3cab..e3b41313f 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -200,13 +200,25 @@ protected: /** * Queue of expected positional argument forms. * - * Positional arugment descriptions are inserted on the back. + * Positional argument descriptions are inserted on the back. * * As positional arguments are passed, these are popped from the * front, until there are hopefully none left as all args that were * expected in fact were passed. */ std::list expectedArgs; + /** + * List of processed positional argument forms. + * + * All items removed from `expectedArgs` are added here. After all + * arguments were processed, this list should be exactly the same as + * `expectedArgs` was before. + * + * This list is used to extend the lifetime of the argument forms. + * If this is not done, some closures that reference the command + * itself will segfault. + */ + std::list processedArgs; /** * Process some positional arugments diff --git a/tests/functional/completions.sh b/tests/functional/completions.sh index b9886623a..d3d5bbd48 100644 --- a/tests/functional/completions.sh +++ b/tests/functional/completions.sh @@ -44,6 +44,10 @@ EOF # Input override completion [[ "$(NIX_GET_COMPLETIONS=4 nix build ./foo --override-input '')" == $'normal\na\t' ]] [[ "$(NIX_GET_COMPLETIONS=5 nix flake show ./foo --override-input '')" == $'normal\na\t' ]] +cd ./foo +[[ "$(NIX_GET_COMPLETIONS=3 nix flake update '')" == $'normal\na\t' ]] +cd .. +[[ "$(NIX_GET_COMPLETIONS=5 nix flake update --flake './foo' '')" == $'normal\na\t' ]] ## With multiple input flakes [[ "$(NIX_GET_COMPLETIONS=5 nix build ./foo ./bar --override-input '')" == $'normal\na\t\nb\t' ]] ## With tilde expansion