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.

Sign up for GitHub

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

Jump to bottom

Allow specifying alternative paths for reading/writing flake locks #8042

Merged
merged 5 commits into from
Apr 3, 2023
Merged

Allow specifying alternative paths for reading/writing flake locks #8042

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e91596e
Allow specifying alternative paths for reading/writing flake locks
lheckemann Mar 13, 2023
3a1de4c
Apply review suggestions
lheckemann Mar 14, 2023
ea207a2
Add tests for alternate lockfile path functionality
lheckemann Mar 19, 2023
f1c9d83
Only allow reference lock files when allow-dirty is set
lheckemann Mar 19, 2023
3c3bd07
Create test lockfiles in TEST_ROOT
lheckemann Mar 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/libcmd/installables.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,28 @@ MixFlakeOptions::MixFlakeOptions()
}}
});

addFlag({
.longName = "reference-lock-file",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if there is a real use case for a separate --reference-lock-file and --output-lock-file. With a unified --lock-file, the caller can always write the input lock file to that location to be overwritten.

Copy link
Member Author

@lheckemann lheckemann Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I like to keep the lock files of my projects in sync with that of my main deployment. With the separated options, I can use nix flake lock --reference-lock-file ~/deploy/flake.lock to keep them in sync (provided my deployment has a superset of the inputs of the project I'm updating).

It's also useful for updating individual inputs (but leaving everything else unmodified) without having a checkout of the whole repository, e.g. nix flake lock github:NixOS/patchelf --output-lock-file flake.lock --update-input nixpkgs or nix flake lock github:NixOS/patchelf --output-lock-file flake.lock --override-input nixpkgs github:nixos/nixpkgs/nixos-22.11. This is what I had in mind for Hydra, since it invokes multiple commands on the flake for a single evaluation and should use the same set of inputs each time.

Copy link
Member

Choose a reason for hiding this comment

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

without having a checkout of the whole repository

Just for clarity: you mean checkout out the repository into a temporary directory because it will still land "checked out" in the nix store.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not necessarily in the future (#6530).

Copy link
Member

Choose a reason for hiding this comment

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

If we go for --output-lock-file, why not call it --input-lock-file?

Copy link
Member Author

Choose a reason for hiding this comment

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

"reference" seems more appropriate here, since the lock file may be completely foreign (e.g. I'm working on a project that uses nixos-22.11 and want it to use my deployment's lock file, so I run nix flake lock --reference-lock-file ~/deploy/flake.lock). This is very much my personal opinion though and if there's broad agreement that it should be called "input" I'd be willing to change it.

.description = "Read the given lock file instead of `flake.lock` within the top-level flake.",
.category = category,
.labels = {"flake-lock-path"},
.handler = {[&](std::string lockFilePath) {
lockFlags.referenceLockFilePath = lockFilePath;
}},
.completer = completePath
});

addFlag({
.longName = "output-lock-file",
.description = "Write the given lock file instead of `flake.lock` within the top-level flake.",
.category = category,
.labels = {"flake-lock-path"},
.handler = {[&](std::string lockFilePath) {
lockFlags.outputLockFilePath = lockFilePath;
}},
.completer = completePath
});

addFlag({
.longName = "inputs-from",
.description = "Use the inputs of the specified flake as registry entries.",
Expand Down
31 changes: 19 additions & 12 deletions src/libexpr/flake/flake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,8 @@ LockedFlake lockFlake(

// FIXME: symlink attack
auto oldLockFile = LockFile::read(
flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir + "/flake.lock");
lockFlags.referenceLockFilePath.value_or(
flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir + "/flake.lock"));

debug("old lock file: %s", oldLockFile);

Expand Down Expand Up @@ -619,39 +620,45 @@ LockedFlake lockFlake(

debug("new lock file: %s", newLockFile);

auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock";
auto sourcePath = topRef.input.getSourcePath();
auto outputLockFilePath = sourcePath ? std::optional{*sourcePath + "/" + relPath} : std::nullopt;
if (lockFlags.outputLockFilePath) {
outputLockFilePath = lockFlags.outputLockFilePath;
}

/* Check whether we need to / can write the new lock file. */
if (!(newLockFile == oldLockFile)) {
if (newLockFile != oldLockFile || lockFlags.outputLockFilePath) {

auto diff = LockFile::diff(oldLockFile, newLockFile);

if (lockFlags.writeLockFile) {
if (auto sourcePath = topRef.input.getSourcePath()) {
if (outputLockFilePath) {
if (auto unlockedInput = newLockFile.isUnlocked()) {
if (fetchSettings.warnDirty)
warn("will not write lock file of flake '%s' because it has an unlocked input ('%s')", topRef, *unlockedInput);
} else {
if (!lockFlags.updateLockFile)
throw Error("flake '%s' requires lock file changes but they're not allowed due to '--no-update-lock-file'", topRef);

auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock";

auto path = *sourcePath + "/" + relPath;

bool lockFileExists = pathExists(path);
bool lockFileExists = pathExists(*outputLockFilePath);

if (lockFileExists) {
auto s = chomp(diff);
if (s.empty())
warn("updating lock file '%s'", path);
warn("updating lock file '%s'", *outputLockFilePath);
else
warn("updating lock file '%s':\n%s", path, s);
warn("updating lock file '%s':\n%s", *outputLockFilePath, s);
} else
warn("creating lock file '%s'", path);
warn("creating lock file '%s'", *outputLockFilePath);

newLockFile.write(path);
newLockFile.write(*outputLockFilePath);

std::optional<std::string> commitMessage = std::nullopt;
if (lockFlags.commitLockFile) {
if (lockFlags.outputLockFilePath) {
throw Error("--commit-lock-file and --output-lock-file are currently incompatible");
}
Comment on lines +662 to +664
Copy link
Member

Choose a reason for hiding this comment

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

If output-lock-file ends up under the git repository it could be commited like flake.lock. Any none absolute path that flattened does not start with .. should be commitable.

Copy link
Member Author

Choose a reason for hiding this comment

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

This does not apply if symlinks are in use, and generating accurate commit messages would require reading the original lock file (if one already exists at the given path) as well as the reference lock file in order to generate an accurate commit message. Most of the value from this PR comes with the basic functionality and I'd welcome if someone else were to implement committing support in the future, but do not want to implement it myself at this time.

std::string cm;

cm = fetchSettings.commitLockFileSummary.get();
Expand Down
6 changes: 6 additions & 0 deletions src/libexpr/flake/flake.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ struct LockFlags
/* Whether to commit changes to flake.lock. */
bool commitLockFile = false;

/* The path to a lock file to read instead of the `flake.lock` file in the top-level flake */
std::optional<std::string> referenceLockFilePath;

/* The path to a lock file to write to instead of the `flake.lock` file in the top-level flake */
std::optional<Path> outputLockFilePath;

/* Flake inputs to be overridden. */
std::map<InputPath, FlakeRef> inputOverrides;

Expand Down