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

Use SourcePath for reading flake.{nix,lock} #10088

Merged
merged 1 commit into from
Mar 4, 2024
Merged

Use SourcePath for reading flake.{nix,lock} #10088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/libcmd/installables.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "url.hh"
#include "registry.hh"
#include "build-result.hh"
#include "fs-input-accessor.hh"

#include <regex>
#include <queue>
Expand Down Expand Up @@ -146,7 +147,7 @@ MixFlakeOptions::MixFlakeOptions()
.category = category,
.labels = {"flake-lock-path"},
.handler = {[&](std::string lockFilePath) {
lockFlags.referenceLockFilePath = lockFilePath;
lockFlags.referenceLockFilePath = getUnfilteredRootPath(CanonPath(absPath(lockFilePath)));
}},
.completer = completePath
});
Expand Down
95 changes: 50 additions & 45 deletions src/libexpr/flake/flake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
attrs.emplace(state.symbols[attr.name], Explicit<bool> { attr.value->boolean });
break;
case nInt:
attrs.emplace(state.symbols[attr.name], (long unsigned int)attr.value->integer);
attrs.emplace(state.symbols[attr.name], (long unsigned int) attr.value->integer);
break;
default:
if (attr.name == state.symbols.create("publicKeys")) {
Expand Down Expand Up @@ -202,43 +202,28 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs(
return inputs;
}

static Flake getFlake(
static Flake readFlake(
EvalState & state,
const FlakeRef & originalRef,
bool allowLookup,
FlakeCache & flakeCache,
InputPath lockRootPath)
const FlakeRef & resolvedRef,
const FlakeRef & lockedRef,
const SourcePath & rootDir,
const InputPath & lockRootPath)
{
auto [storePath, resolvedRef, lockedRef] = fetchOrSubstituteTree(
state, originalRef, allowLookup, flakeCache);
auto flakePath = rootDir / CanonPath(resolvedRef.subdir) / "flake.nix";

Value vInfo;
state.evalFile(flakePath, vInfo, true);

// We need to guard against symlink attacks, but before we start doing
// filesystem operations we should make sure there's a flake.nix in the
// first place.
auto unsafeFlakeDir = state.store->toRealPath(storePath) + "/" + lockedRef.subdir;
auto unsafeFlakeFile = unsafeFlakeDir + "/flake.nix";
if (!pathExists(unsafeFlakeFile))
throw Error("source tree referenced by '%s' does not contain a '%s/flake.nix' file", lockedRef, lockedRef.subdir);

// Guard against symlink attacks.
auto flakeDir = canonPath(unsafeFlakeDir, true);
auto flakeFile = canonPath(flakeDir + "/flake.nix", true);
if (!isInDir(flakeFile, state.store->toRealPath(storePath)))
throw Error("'flake.nix' file of flake '%s' escapes from '%s'",
lockedRef, state.store->printStorePath(storePath));
expectType(state, nAttrs, vInfo, state.positions.add(Pos::Origin(rootDir), 1, 1));

Flake flake {
.originalRef = originalRef,
.resolvedRef = resolvedRef,
.lockedRef = lockedRef,
.storePath = storePath,
.path = flakePath,
};

Value vInfo;
state.evalFile(state.rootPath(CanonPath(flakeFile)), vInfo, true); // FIXME: symlink attack

expectType(state, nAttrs, vInfo, state.positions.add({state.rootPath(CanonPath(flakeFile))}, 1, 1));

if (auto description = vInfo.attrs->get(state.sDescription)) {
expectType(state, nString, *description->value, description->pos);
flake.description = description->value->c_str();
Expand All @@ -247,7 +232,7 @@ static Flake getFlake(
auto sInputs = state.symbols.create("inputs");

if (auto inputs = vInfo.attrs->get(sInputs))
flake.inputs = parseFlakeInputs(state, inputs->value, inputs->pos, flakeDir, lockRootPath);
flake.inputs = parseFlakeInputs(state, inputs->value, inputs->pos, flakePath.parent().path.abs(), lockRootPath); // FIXME

auto sOutputs = state.symbols.create("outputs");

Expand All @@ -264,7 +249,7 @@ static Flake getFlake(
}

} else
throw Error("flake '%s' lacks attribute 'outputs'", lockedRef);
throw Error("flake '%s' lacks attribute 'outputs'", resolvedRef);

auto sNixConfig = state.symbols.create("nixConfig");

Expand All @@ -281,7 +266,7 @@ static Flake getFlake(
NixStringContext emptyContext = {};
flake.config.settings.emplace(
state.symbols[setting.name],
state.coerceToString(setting.pos, *setting.value, emptyContext, "", false, true, true) .toOwned());
state.coerceToString(setting.pos, *setting.value, emptyContext, "", false, true, true).toOwned());
}
else if (setting.value->type() == nInt)
flake.config.settings.emplace(
Expand Down Expand Up @@ -313,12 +298,25 @@ static Flake getFlake(
attr.name != sOutputs &&
attr.name != sNixConfig)
throw Error("flake '%s' has an unsupported attribute '%s', at %s",
lockedRef, state.symbols[attr.name], state.positions[attr.pos]);
resolvedRef, state.symbols[attr.name], state.positions[attr.pos]);
}

return flake;
}

static Flake getFlake(
EvalState & state,
const FlakeRef & originalRef,
bool allowLookup,
FlakeCache & flakeCache,
InputPath lockRootPath)
{
auto [storePath, resolvedRef, lockedRef] = fetchOrSubstituteTree(
state, originalRef, allowLookup, flakeCache);

return readFlake(state, originalRef, resolvedRef, lockedRef, state.rootPath(state.store->toRealPath(storePath)), lockRootPath);
}

Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup, FlakeCache & flakeCache)
{
return getFlake(state, originalRef, allowLookup, flakeCache, {});
Expand All @@ -330,6 +328,13 @@ Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup
return getFlake(state, originalRef, allowLookup, flakeCache);
}

static LockFile readLockFile(const SourcePath & lockFilePath)
{
return lockFilePath.pathExists()
? LockFile(lockFilePath.readFile(), fmt("%s", lockFilePath))
: LockFile();
}

/* Compute an in-memory lock file for the specified top-level flake,
and optionally write it to file, if the flake is writable. */
LockedFlake lockFlake(
Expand All @@ -355,17 +360,16 @@ LockedFlake lockFlake(
throw Error("reference lock file was provided, but the `allow-dirty` setting is set to false");
}

// FIXME: symlink attack
auto oldLockFile = LockFile::read(
auto oldLockFile = readLockFile(
lockFlags.referenceLockFilePath.value_or(
state.store->toRealPath(flake.storePath) + "/" + flake.lockedRef.subdir + "/flake.lock"));
flake.lockFilePath()));

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

std::map<InputPath, FlakeInput> overrides;
std::set<InputPath> explicitCliOverrides;
std::set<InputPath> overridesUsed, updatesUsed;
std::map<ref<Node>, StorePath> nodePaths;
std::map<ref<Node>, SourcePath> nodePaths;

for (auto & i : lockFlags.inputOverrides) {
overrides.insert_or_assign(i.first, FlakeInput { .ref = i.second });
Expand Down Expand Up @@ -538,7 +542,7 @@ LockedFlake lockFlake(

if (mustRefetch) {
auto inputFlake = getFlake(state, oldLock->lockedRef, false, flakeCache, inputPath);
nodePaths.emplace(childNode, inputFlake.storePath);
nodePaths.emplace(childNode, inputFlake.path.parent());
computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, lockRootPath, parentPath, false);
} else {
computeLocks(fakeInputs, childNode, inputPath, oldLock, lockRootPath, parentPath, true);
Expand Down Expand Up @@ -587,13 +591,12 @@ LockedFlake lockFlake(
flake. Also, unless we already have this flake
in the top-level lock file, use this flake's
own lock file. */
nodePaths.emplace(childNode, inputFlake.storePath);
nodePaths.emplace(childNode, inputFlake.path.parent());
computeLocks(
inputFlake.inputs, childNode, inputPath,
oldLock
? std::dynamic_pointer_cast<const Node>(oldLock)
: LockFile::read(
state.store->toRealPath(inputFlake.storePath) + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root.get_ptr(),
: readLockFile(inputFlake.lockFilePath()).root.get_ptr(),
oldLock ? lockRootPath : inputPath,
localPath,
false);
Expand All @@ -605,7 +608,7 @@ LockedFlake lockFlake(

auto childNode = make_ref<LockedNode>(lockedRef, ref, false);

nodePaths.emplace(childNode, storePath);
nodePaths.emplace(childNode, state.rootPath(state.store->toRealPath(storePath)));

node->inputs.insert_or_assign(id, childNode);
}
Expand All @@ -619,9 +622,9 @@ LockedFlake lockFlake(
};

// Bring in the current ref for relative path resolution if we have it
auto parentPath = canonPath(state.store->toRealPath(flake.storePath) + "/" + flake.lockedRef.subdir, true);
auto parentPath = flake.path.parent().path.abs();

nodePaths.emplace(newLockFile.root, flake.storePath);
nodePaths.emplace(newLockFile.root, flake.path.parent());

computeLocks(
flake.inputs,
Expand Down Expand Up @@ -746,13 +749,15 @@ void callFlake(EvalState & state,

auto overrides = state.buildBindings(lockedFlake.nodePaths.size());

for (auto & [node, storePath] : lockedFlake.nodePaths) {
for (auto & [node, sourcePath] : lockedFlake.nodePaths) {
auto override = state.buildBindings(2);

auto & vSourceInfo = override.alloc(state.symbols.create("sourceInfo"));

auto lockedNode = node.dynamic_pointer_cast<const LockedNode>();

auto [storePath, subdir] = state.store->toStorePath(sourcePath.path.abs());

emitTreeAttrs(
state,
storePath,
Expand All @@ -766,7 +771,7 @@ void callFlake(EvalState & state,

override
.alloc(state.symbols.create("dir"))
.mkString(lockedNode ? lockedNode->lockedRef.subdir : lockedFlake.flake.lockedRef.subdir);
.mkString(CanonPath(subdir).rel());

overrides.alloc(state.symbols.create(key->second)).mkAttrs(override);
}
Expand Down Expand Up @@ -928,7 +933,7 @@ Fingerprint LockedFlake::getFingerprint() const
// flake.sourceInfo.storePath for the fingerprint.
return hashString(HashAlgorithm::SHA256,
fmt("%s;%s;%d;%d;%s",
flake.storePath.to_string(),
flake.path.to_string(),
flake.lockedRef.subdir,
flake.lockedRef.input.getRevCount().value_or(0),
flake.lockedRef.input.getLastModified().value_or(0),
Expand Down
17 changes: 13 additions & 4 deletions src/libexpr/flake/flake.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,27 @@ struct Flake
* the specific local store result of invoking the fetcher
*/
FlakeRef lockedRef;
/**
* The path of `flake.nix`.
*/
SourcePath path;
/**
* pretend that 'lockedRef' is dirty
*/
bool forceDirty = false;
std::optional<std::string> description;
StorePath storePath;
FlakeInputs inputs;
/**
* 'nixConfig' attribute
*/
ConfigFile config;

~Flake();

SourcePath lockFilePath()
{
return path.parent() / "flake.lock";
}
};

Flake getFlake(EvalState & state, const FlakeRef & flakeRef, bool allowLookup);
Expand All @@ -104,11 +113,11 @@ struct LockedFlake
LockFile lockFile;

/**
* Store paths of nodes that have been fetched in
* Source tree accessors for nodes that have been fetched in
* lockFlake(); in particular, the root node and the overriden
* inputs.
*/
std::map<ref<Node>, StorePath> nodePaths;
std::map<ref<Node>, SourcePath> nodePaths;

Fingerprint getFingerprint() const;
};
Expand Down Expand Up @@ -165,7 +174,7 @@ struct LockFlags
/**
* The path to a lock file to read instead of the `flake.lock` file in the top-level flake
*/
std::optional<std::string> referenceLockFilePath;
std::optional<SourcePath> referenceLockFilePath;

/**
* The path to a lock file to write to instead of the `flake.lock` file in the top-level flake
Expand Down
10 changes: 3 additions & 7 deletions src/libexpr/flake/lockfile.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,10 @@ std::shared_ptr<Node> LockFile::findInput(const InputPath & path)
return doFind(root, path, visited);
}

LockFile::LockFile(const nlohmann::json & json, const Path & path)
LockFile::LockFile(std::string_view contents, std::string_view path)
{
auto json = nlohmann::json::parse(contents);

auto version = json.value("version", 0);
if (version < 5 || version > 7)
throw Error("lock file '%s' has unsupported version %d", path, version);
Expand Down Expand Up @@ -203,12 +205,6 @@ std::pair<std::string, LockFile::KeyMap> LockFile::to_string() const
return {json.dump(2), std::move(nodeKeys)};
}

LockFile LockFile::read(const Path & path)
{
if (!pathExists(path)) return LockFile();
return LockFile(nlohmann::json::parse(readFile(path)), path);
}

std::ostream & operator <<(std::ostream & stream, const LockFile & lockFile)
{
stream << lockFile.toJSON().first.dump(2);
Expand Down
4 changes: 1 addition & 3 deletions src/libexpr/flake/lockfile.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,14 @@ struct LockFile
ref<Node> root = make_ref<Node>();

LockFile() {};
LockFile(const nlohmann::json & json, const Path & path);
LockFile(std::string_view contents, std::string_view path);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
LockFile(std::string_view contents, std::string_view path);
LockFile(nlohmann::json json, std::string_view path);

I think I would prefer that: let the caller parse and move in. Just as efficient, and keeps are more informative method type.

Copy link
Member Author

@edolstra edolstra Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think that would be better. To the caller the lockfile is just an opaque string. The fact that it's JSON is an implementation detail of LockFile.

Copy link
Member

Choose a reason for hiding this comment

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

Well, the caller does want to know that toJSON() and this are related by a round trip law, right?

I don't mind also exposing the current signature as a simple wrapper if it is easier for call sites.


typedef std::map<ref<const Node>, std::string> KeyMap;

std::pair<nlohmann::json, KeyMap> toJSON() const;

std::pair<std::string, KeyMap> to_string() const;

static LockFile read(const Path & path);

/**
* Check whether this lock file has any unlocked inputs. If so,
* return one.
Expand Down
13 changes: 9 additions & 4 deletions src/nix/flake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
auto lockedFlake = lockFlake();
auto & flake = lockedFlake.flake;

// Currently, all flakes are in the Nix store via the rootFS accessor.
auto storePath = store->printStorePath(store->toStorePath(flake.path.path.abs()).first);

if (json) {
nlohmann::json j;
if (flake.description)
Expand All @@ -223,7 +226,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
j["revCount"] = *revCount;
if (auto lastModified = flake.lockedRef.input.getLastModified())
j["lastModified"] = *lastModified;
j["path"] = store->printStorePath(flake.storePath);
j["path"] = storePath;
j["locks"] = lockedFlake.lockFile.toJSON().first;
logger->cout("%s", j.dump());
} else {
Expand All @@ -239,7 +242,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
*flake.description);
logger->cout(
ANSI_BOLD "Path:" ANSI_NORMAL " %s",
store->printStorePath(flake.storePath));
storePath);
if (auto rev = flake.lockedRef.input.getRev())
logger->cout(
ANSI_BOLD "Revision:" ANSI_NORMAL " %s",
Expand Down Expand Up @@ -1031,7 +1034,9 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun

StorePathSet sources;

sources.insert(flake.flake.storePath);
auto storePath = store->toStorePath(flake.flake.path.path.abs()).first;

sources.insert(storePath);

// FIXME: use graph output, handle cycles.
std::function<nlohmann::json(const Node & node)> traverse;
Expand Down Expand Up @@ -1060,7 +1065,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun

if (json) {
nlohmann::json jsonRoot = {
{"path", store->printStorePath(flake.flake.storePath)},
{"path", store->printStorePath(storePath)},
{"inputs", traverse(*flake.lockFile.root)},
};
logger->cout("%s", jsonRoot);
Expand Down
Loading