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

builtins.getFlake: Allow inputs to be overridden #11952

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

builtins.getFlake: Allow inputs to be overridden #11952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2b62625
Move getFlake tests into a separate file
edolstra Nov 25, 2024
1deca0c
builtins.getFlake: Allow inputs to overridden
edolstra Nov 25, 2024
78abd11
Don't mention fetchTree
edolstra Nov 26, 2024
8fd798a
Add release note
edolstra Dec 9, 2024
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
13 changes: 13 additions & 0 deletions doc/manual/rl-next/get-flake-overrides.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
synopsis: "`builtins.getFlake` allows inputs to be overriden"
prs: [11952]
---

`builtins.getFlake` now allows you to override the inputs of a flake, using the same override mechanism used for flake inputs in `flake.nix`. For example, to fetch and call the `NixOS/nix` flake and override its `nixpkgs` input:

```nix
builtins.getFlake {
url = "github:NixOS/nix/55bc52401966fbffa525c574c14f67b00bc4fb3a";
inputs.nixpkgs.url = "github:NixOS/nixpkgs/c69a9bffbecde46b4b939465422ddc59493d3e4d";
}
```
91 changes: 73 additions & 18 deletions src/libflake/flake/flake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs(
const std::optional<Path> & baseDir, InputPath lockRootPath);

static FlakeInput parseFlakeInput(EvalState & state,
std::string_view inputName, Value * value, const PosIdx pos,
std::optional<std::string_view> inputName, Value * value, const PosIdx pos,
Copy link
Member

Choose a reason for hiding this comment

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

Could inputName be removed? An input shouldn't have to know its name.

I think this might be an issue with the data model, but it seems that we have low hanging fruit, in that we could turn the call with inputName into a separate overload that calls parseFlakeInput(state,value,pos) and postprocesses the return value.

const std::optional<Path> & baseDir, InputPath lockRootPath)
{
expectType(state, nAttrs, *value, pos);
Expand Down Expand Up @@ -185,8 +185,8 @@ static FlakeInput parseFlakeInput(EvalState & state,
input.ref = parseFlakeRef(state.fetchSettings, *url, baseDir, true, input.isFlake);
}

if (!input.follows && !input.ref)
input.ref = FlakeRef::fromAttrs(state.fetchSettings, {{"type", "indirect"}, {"id", std::string(inputName)}});
if (inputName && !input.follows && !input.ref)
input.ref = FlakeRef::fromAttrs(state.fetchSettings, {{"type", "indirect"}, {"id", std::string(*inputName)}});

return input;
}
Expand Down Expand Up @@ -735,7 +735,10 @@ LockedFlake lockFlake(
} else
throw Error("cannot write modified lock file of flake '%s' (use '--no-write-lock-file' to ignore)", topRef);
} else {
warn("not writing modified lock file of flake '%s':\n%s", topRef, chomp(diff));
if (lockFlags.warnModifiedLockFile)
warn("not writing modified lock file of flake '%s':\n%s", topRef, chomp(diff));
else
debug("not writing modified lock file of flake '%s':\n%s", topRef, chomp(diff));
flake.forceDirty = true;
}
}
Expand Down Expand Up @@ -823,20 +826,63 @@ void initLib(const Settings & settings)
{
auto prim_getFlake = [&settings](EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
std::string flakeRefS(state.forceStringNoCtx(*args[0], pos, "while evaluating the argument passed to builtins.getFlake"));
auto flakeRef = parseFlakeRef(state.fetchSettings, flakeRefS, {}, true);
if (state.settings.pureEval && !flakeRef.input.isLocked())
throw Error("cannot call 'getFlake' on unlocked flake reference '%s', at %s (use --impure to override)", flakeRefS, state.positions[pos]);

callFlake(state,
lockFlake(settings, state, flakeRef,
LockFlags {
.updateLockFile = false,
.writeLockFile = false,
.useRegistries = !state.settings.pureEval && settings.useRegistries,
.allowUnlocked = !state.settings.pureEval,
}),
v);
state.forceValue(*args[0], pos);

LockFlags lockFlags {
.updateLockFile = false,
.writeLockFile = false,
.warnModifiedLockFile = false,
.useRegistries = !state.settings.pureEval && settings.useRegistries,
.allowUnlocked = !state.settings.pureEval,
};

auto flakeRef =
args[0]->type() == nString
? ({
std::string flakeRefS(state.forceStringNoCtx(*args[0], pos, "while evaluating the argument passed to builtins.getFlake"));
auto flakeRef = parseFlakeRef(state.fetchSettings, flakeRefS, {}, true);
if (state.settings.pureEval && !flakeRef.input.isLocked())
throw Error("cannot call 'getFlake' on unlocked flake reference '%s', at %s (use --impure to override)", flakeRefS, state.positions[pos]);
flakeRef;
})
: ({
auto flakeInput = parseFlakeInput(state, std::nullopt, args[0], pos, {}, {});
Copy link
Member

Choose a reason for hiding this comment

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

parseFlakeInput expects a "trivial" value, but this is not a reasonable expectation for args[0], which is part of the "normal" expression language where evaluation just happens.
(e.g, url = "git+https://" + foo; will fail)

Possible solutions:

  • deepSeq
    • gets the job done if all is ok, but is too strict: worse error message for errors in invalid (misplaced) attributes.
  • force the relevant values here
    • code gets out of sync with parseFlakeInput, and bugs won't be found immediately, because literals and trivial values are common
  • add a flag to parseFlakeInput to allow it to evaluate when true
    • don't see much of a problem with this

My preference is with the parseFlakeInput flag.

Such a flag would perhaps also be useful for a feature where an "evaluated" flake can be passed as an input, e.g. getFlake { url = ./tests; inputs.foo.value = self; }, although this has also been requested as a feature for static flake inputs:


/* Convert the result of parseFlakeInput() into a
overrides map and a top-level flakeref. */
std::function<void(const InputPath & inputPath, const FlakeInput & input)> recurse;

recurse = [&](const InputPath & inputPath, const FlakeInput & input)
{
if (!input.ref)
Comment on lines +853 to +857
Copy link
Contributor

@xokdvium xokdvium Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's not strictly necessary to use std::function and its inherent indirection for recursive lambdas. Though in this case this shouldn't matter much. With C++23 it's possible to use "deducing this" https://en.cppreference.com/w/cpp/language/member_functions#Explicit_object_member_functions. For pre C++23 one can use the following hack:

auto recurse_impl = [&](auto& self, const InputPath & inputPath, const FlakeInput & input) {
....
  recurse_impl(self, ....);
};

auto recurse = [&](const InputPath & inputPath, const FlakeInput & input) {
  recurse_impl(recurse_impl, inputPath, input);
};

This avoids the virtual call via std::function

Copy link
Member Author

@edolstra edolstra Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Wow nice, didn't know about "deducing this". Apparently it will be available in gcc 14.

state.error<EvalError>("'builtins.getFlake' requires attribute 'url'")
.atPos(*args[0])
.debugThrow();
if (input.follows)
state.error<EvalError>("'builtins.getFlake' does not permit attribute 'follows'")
.atPos(*args[0])
.debugThrow();
if (!input.isFlake)
state.error<EvalError>("'builtins.getFlake' does not permit attribute 'flake = false'")
.atPos(*args[0])
.debugThrow();

for (auto & [inputName, input2] : input.overrides) {
auto inputPath2{inputPath};
inputPath2.push_back(inputName);

recurse(inputPath2, input2);

lockFlags.inputOverrides.insert_or_assign(inputPath2, input2.ref.value());
}
};

recurse({}, flakeInput);

flakeInput.ref.value();
});

callFlake(state, lockFlake(settings, state, flakeRef, lockFlags), v);
};

RegisterPrimOp::primOps->push_back({
Expand All @@ -856,6 +902,15 @@ void initLib(const Settings & settings)
```nix
(builtins.getFlake "github:edolstra/dwarffs").rev
```

It is possible to override inputs of the flake using the same syntax to specify flake inputs in `flake.nix`, e.g.

```nix
builtins.getFlake {
url = "github:NixOS/nix/55bc52401966fbffa525c574c14f67b00bc4fb3a";
inputs.nixpkgs.url = "github:NixOS/nixpkgs/c69a9bffbecde46b4b939465422ddc59493d3e4d";
}
```
)",
.fun = prim_getFlake,
.experimentalFeature = Xp::Flakes,
Expand Down
8 changes: 7 additions & 1 deletion src/libflake/flake/flake.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ struct Flake
*/
SourcePath path;
/**
* pretend that 'lockedRef' is dirty
* Pretend that 'lockedRef' is dirty.
*/
bool forceDirty = false;
std::optional<std::string> description;
Expand Down Expand Up @@ -156,6 +156,12 @@ struct LockFlags
*/
bool writeLockFile = true;

/**
* When `writeLockFile` is false, whether we're warning about
* modified lock files.
*/
bool warnModifiedLockFile = true;

/**
* Whether to use the registries to lookup indirect flake
* references like 'nixpkgs'.
Expand Down
10 changes: 0 additions & 10 deletions tests/functional/flakes/flakes.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,6 @@ nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default"
nix build -o "$flake1Dir/result" "git+file://$flake1Dir"
nix path-info "$flake1Dir/result"

# 'getFlake' on an unlocked flakeref should fail in pure mode, but
# succeed in impure mode.
(! nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"$flake1Dir\").packages.$system.default")
nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"$flake1Dir\").packages.$system.default" --impure

# 'getFlake' on a locked flakeref should succeed even in pure mode.
nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"git+file://$flake1Dir?rev=$hash2\").packages.$system.default"

# Regression test for dirOf on the root of the flake.
[[ $(nix eval --json flake1#parent) = \""$NIX_STORE_DIR"\" ]]

Expand All @@ -131,12 +123,10 @@ nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"git+file://$flake1
# Building a flake with an unlocked dependency should fail in pure mode.
(! nix build -o "$TEST_ROOT/result" flake2#bar --no-registries)
(! nix build -o "$TEST_ROOT/result" flake2#bar --no-use-registries)
(! nix eval --expr "builtins.getFlake \"$flake2Dir\"")

# But should succeed in impure mode.
(! nix build -o "$TEST_ROOT/result" flake2#bar --impure)
nix build -o "$TEST_ROOT/result" flake2#bar --impure --no-write-lock-file
nix eval --expr "builtins.getFlake \"$flake2Dir\"" --impure

# Building a local flake with an unlocked dependency should fail with --no-update-lock-file.
expect 1 nix build -o "$TEST_ROOT/result" "$flake2Dir#bar" --no-update-lock-file 2>&1 | grep 'requires lock file changes'
Expand Down
43 changes: 43 additions & 0 deletions tests/functional/flakes/get-flake.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#!/usr/bin/env bash

source ./common.sh

TODO_NixOS

createFlake1
createFlake2

# 'getFlake' on an unlocked flakeref should fail in pure mode, but
# succeed in impure mode.
(! nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"$flake1Dir\").packages.$system.default")
nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"$flake1Dir\").packages.$system.default" --impure

# 'getFlake' on a locked flakeref should succeed even in pure mode.
hash=$(nix flake metadata flake1 --json --refresh | jq -r .revision)
nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"git+file://$flake1Dir?rev=$hash\").packages.$system.default"

# Building a flake with an unlocked dependency should fail in pure mode.
(! nix eval --expr "builtins.getFlake \"$flake2Dir\"")

# But should succeed in impure mode.
nix eval --expr "builtins.getFlake \"$flake2Dir\"" --impure

# Test overrides in getFlake.
flake1Copy="$flake1Dir-copy"
rm -rf "$flake1Copy"
cp -r "$flake1Dir" "$flake1Copy"
sed -i "$flake1Copy/simple.builder.sh" -e 's/World/Universe/'

# Should fail in pure mode since the override is unlocked.
(! nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake { url = \"$flake2Dir\"; inputs.flake1.url = \"$flake1Copy\"; }).packages.$system.bar")

# Should succeed in impure mode.
nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake { url = \"$flake2Dir\"; inputs.flake1.url = \"$flake1Copy\"; }).packages.$system.bar" --impure
[[ $(cat "$TEST_ROOT/result/hello") = 'Hello Universe!' ]]

# Should succeed if we lock the override.
git -C "$flake1Copy" commit -a -m 'bla'

flake1CopyLocked="$(nix flake metadata --json "$flake1Copy" | jq -r .url)"

nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake { url = \"$flake2Dir\"; inputs.flake1.url = \"$flake1CopyLocked\"; }).packages.$system.bar"
1 change: 1 addition & 0 deletions tests/functional/flakes/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ suites += {
'shebang.sh',
'commit-lock-file-summary.sh',
'non-flake-inputs.sh',
'get-flake.sh',
],
'workdir': meson.current_source_dir(),
}
Loading