Skip to content

Commit

Permalink
Make rootFS's showPath() render the paths from the original accessors
Browse files Browse the repository at this point in the history
This makes paths in error messages behave similar to lazy-trees,
e.g. instead of store paths like

       error: attribute 'foobar' missing
       at /nix/store/ddzfiipzqlrh3gnprmqbadnsnrxsmc9i-source/machine/configuration.nix:209:7:
          208|
          209|       pkgs.foobar
             |       ^
          210|     ];

you now get

       error: attribute 'foobar' missing
       at /home/eelco/Misc/eelco-configurations/machine/configuration.nix:209:7:
          208|
          209|       pkgs.foobar
             |       ^
          210|     ];
  • Loading branch information
edolstra committed Feb 20, 2025
1 parent ec7dc56 commit 1048855
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 23 deletions.
32 changes: 32 additions & 0 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "profiles.hh"
#include "print.hh"
#include "filtering-source-accessor.hh"
#include "forwarding-source-accessor.hh"
#include "memory-source-accessor.hh"
#include "gc-small-vector.hh"
#include "url.hh"
Expand Down Expand Up @@ -180,6 +181,34 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
}
}

struct PathDisplaySourceAccessor : ForwardingSourceAccessor
{
ref<EvalState::StorePathAccessors> storePathAccessors;

PathDisplaySourceAccessor(
ref<SourceAccessor> next,
ref<EvalState::StorePathAccessors> storePathAccessors)
: ForwardingSourceAccessor(next)
, storePathAccessors(storePathAccessors)
{
}

std::string showPath(const CanonPath & path) override
{
/* Find the accessor that produced `path`, if any, and use it
to render a more informative path
(e.g. `«github:foo/bar»/flake.nix` rather than
`/nix/store/hash.../flake.nix`). */
auto ub = storePathAccessors->upper_bound(path);
if (ub != storePathAccessors->begin())
ub--;
if (ub != storePathAccessors->end() && path.isWithin(ub->first))
return ub->second->showPath(path.removePrefix(ub->first));
else
return next->showPath(path);
}
};

static constexpr size_t BASE_ENV_SIZE = 128;

EvalState::EvalState(
Expand Down Expand Up @@ -245,6 +274,7 @@ EvalState::EvalState(
}
, repair(NoRepair)
, emptyBindings(0)
, storePathAccessors(make_ref<StorePathAccessors>())
, rootFS(
({
/* In pure eval mode, we provide a filesystem that only
Expand All @@ -270,6 +300,8 @@ EvalState::EvalState(
: makeUnionSourceAccessor({accessor, storeFS});
}

accessor = make_ref<PathDisplaySourceAccessor>(accessor, storePathAccessors);

/* Apply access control if needed. */
if (settings.restrictEval || settings.pureEval)
accessor = AllowListSourceAccessor::create(accessor, {},
Expand Down
10 changes: 10 additions & 0 deletions src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,16 @@ public:
/** `"unknown"` */
Value vStringUnknown;

using StorePathAccessors = std::map<CanonPath, ref<SourceAccessor>>;

/**
* A map back to the original `SourceAccessor`s used to produce
* store paths. We keep track of this to produce error messages
* that refer to the original flakerefs.
* FIXME: use Sync.
*/
ref<StorePathAccessors> storePathAccessors;

/**
* The accessor for the root filesystem.
*/
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/primops/fetchMercurial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value * * a
if (rev) attrs.insert_or_assign("rev", rev->gitRev());
auto input = fetchers::Input::fromAttrs(state.fetchSettings, std::move(attrs));

auto [storePath, input2] = input.fetchToStore(state.store);
auto [storePath, accessor, input2] = input.fetchToStore(state.store);

auto attrs2 = state.buildBindings(8);
state.mkStorePathString(storePath, attrs2.alloc(state.sOutPath));
Expand Down
4 changes: 3 additions & 1 deletion src/libexpr/primops/fetchTree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,12 @@ static void fetchTree(
throw Error("input '%s' is not allowed to use the '__final' attribute", input.to_string());
}

auto [storePath, input2] = input.fetchToStore(state.store);
auto [storePath, accessor, input2] = input.fetchToStore(state.store);

state.allowPath(storePath);

state.storePathAccessors->insert_or_assign(CanonPath(state.store->printStorePath(storePath)), accessor);

emitTreeAttrs(state, storePath, input2, v, params.emptyRevFallback, false);
}

Expand Down
32 changes: 14 additions & 18 deletions src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,34 +187,30 @@ bool Input::contains(const Input & other) const
}

// FIXME: remove
std::pair<StorePath, Input> Input::fetchToStore(ref<Store> store) const
std::tuple<StorePath, ref<SourceAccessor>, Input> Input::fetchToStore(ref<Store> store) const
{
if (!scheme)
throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs()));

auto [storePath, input] = [&]() -> std::pair<StorePath, Input> {
try {
auto [accessor, result] = getAccessorUnchecked(store);

auto storePath = nix::fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, result.getName());
try {
auto [accessor, result] = getAccessorUnchecked(store);

auto narHash = store->queryPathInfo(storePath)->narHash;
result.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
auto storePath = nix::fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, result.getName());

result.attrs.insert_or_assign("__final", Explicit<bool>(true));
auto narHash = store->queryPathInfo(storePath)->narHash;
result.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));

assert(result.isFinal());
result.attrs.insert_or_assign("__final", Explicit<bool>(true));

checkLocks(*this, result);
assert(result.isFinal());

return {storePath, result};
} catch (Error & e) {
e.addTrace({}, "while fetching the input '%s'", to_string());
throw;
}
}();
checkLocks(*this, result);

return {std::move(storePath), input};
return {std::move(storePath), accessor, result};
} catch (Error & e) {
e.addTrace({}, "while fetching the input '%s'", to_string());
throw;
}
}

void Input::checkLocks(Input specified, Input & result)
Expand Down
2 changes: 1 addition & 1 deletion src/libfetchers/fetchers.hh
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public:
* Fetch the entire input into the Nix store, returning the
* location in the Nix store and the locked input.
*/
std::pair<StorePath, Input> fetchToStore(ref<Store> store) const;
std::tuple<StorePath, ref<SourceAccessor>, Input> fetchToStore(ref<Store> store) const;

/**
* Check the locking attributes in `result` against
Expand Down
2 changes: 2 additions & 0 deletions src/libflake/flake/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ static StorePath copyInputToStore(

state.allowPath(storePath);

state.storePathAccessors->insert_or_assign(CanonPath(state.store->printStorePath(storePath)), accessor);

auto narHash = state.store->queryPathInfo(storePath)->narHash;
input.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));

Expand Down
57 changes: 57 additions & 0 deletions src/libutil/forwarding-source-accessor.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#pragma once

#include "source-accessor.hh"

namespace nix {

/**
* A source accessor that just forwards every operation to another
* accessor. This is not useful in itself but can be used as a
* superclass for accessors that do change some operations.
*/
struct ForwardingSourceAccessor : SourceAccessor
{
ref<SourceAccessor> next;

ForwardingSourceAccessor(ref<SourceAccessor> next)
: next(next)
{
}

std::string readFile(const CanonPath & path) override
{
return next->readFile(path);
}

void readFile(const CanonPath & path, Sink & sink, std::function<void(uint64_t)> sizeCallback) override
{
next->readFile(path, sink, sizeCallback);
}

std::optional<Stat> maybeLstat(const CanonPath & path) override
{
return next->maybeLstat(path);
}

DirEntries readDirectory(const CanonPath & path) override
{
return next->readDirectory(path);
}

std::string readLink(const CanonPath & path) override
{
return next->readLink(path);
}

std::string showPath(const CanonPath & path) override
{
return next->showPath(path);
}

std::optional<std::filesystem::path> getPhysicalPath(const CanonPath & path) override
{
return next->getPhysicalPath(path);
}
};

}
1 change: 1 addition & 0 deletions src/libutil/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ headers = [config_h] + files(
'file-system.hh',
'finally.hh',
'fmt.hh',
'forwarding-source-accessor.hh',
'fs-sink.hh',
'git.hh',
'hash.hh',
Expand Down
4 changes: 2 additions & 2 deletions src/nix/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1093,9 +1093,9 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun
auto storePath =
dryRun
? (*inputNode)->lockedRef.input.computeStorePath(*store)
: (*inputNode)->lockedRef.input.fetchToStore(store).first;
: std::get<0>((*inputNode)->lockedRef.input.fetchToStore(store));
if (json) {
auto& jsonObj3 = jsonObj2[inputName];
auto & jsonObj3 = jsonObj2[inputName];
jsonObj3["path"] = store->printStorePath(storePath);
sources.insert(std::move(storePath));
jsonObj3["inputs"] = traverse(**inputNode);
Expand Down

0 comments on commit 1048855

Please sign in to comment.