Skip to content

Commit

Permalink
Combine AbstractPos, PosAdapter, and Pos
Browse files Browse the repository at this point in the history
Also move `SourcePath` into `libutil`.

These changes allow `error.hh` and `error.cc` to access source path and
position information, which we can use to produce better error messages
(for example, we could consider omitting filenames when two or more
consecutive stack frames originate from the same file).
  • Loading branch information
9999years committed Jan 4, 2024
1 parent 1ed245a commit e824cb8
Show file tree
Hide file tree
Showing 36 changed files with 611 additions and 517 deletions.
1 change: 1 addition & 0 deletions src/libcmd/editor-for.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "editor-for.hh"
#include "environment-variables.hh"
#include "source-path.hh"

namespace nix {

Expand Down
2 changes: 1 addition & 1 deletion src/libcmd/editor-for.hh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
///@file

#include "types.hh"
#include "input-accessor.hh"
#include "source-path.hh"

namespace nix {

Expand Down
3 changes: 2 additions & 1 deletion src/libcmd/installable-value.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "installable-value.hh"
#include "eval-cache.hh"
#include "fetch-to-store.hh"

namespace nix {

Expand Down Expand Up @@ -44,7 +45,7 @@ ref<InstallableValue> InstallableValue::require(ref<Installable> installable)
std::optional<DerivedPathWithInfo> InstallableValue::trySinglePathToDerivedPaths(Value & v, const PosIdx pos, std::string_view errorCtx)
{
if (v.type() == nPath) {
auto storePath = v.path().fetchToStore(*state->store);
auto storePath = fetchToStore(*state->store, v.path());
return {{
.path = DerivedPath::Opaque {
.path = std::move(storePath),
Expand Down
2 changes: 1 addition & 1 deletion src/libcmd/repl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ static std::ostream & showDebugTrace(std::ostream & out, const PosTable & positi
// prefer direct pos, but if noPos then try the expr.
auto pos = dt.pos
? dt.pos
: static_cast<std::shared_ptr<AbstractPos>>(positions[dt.expr.getPos() ? dt.expr.getPos() : noPos]);
: positions[dt.expr.getPos() ? dt.expr.getPos() : noPos];

if (pos) {
out << pos;
Expand Down
9 changes: 5 additions & 4 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "signals.hh"
#include "gc-small-vector.hh"
#include "url.hh"
#include "fetch-to-store.hh"

#include <algorithm>
#include <chrono>
Expand Down Expand Up @@ -870,7 +871,7 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr &
? std::make_unique<DebugTraceStacker>(
*this,
DebugTrace {
.pos = error->info().errPos ? error->info().errPos : static_cast<std::shared_ptr<AbstractPos>>(positions[expr.getPos()]),
.pos = error->info().errPos ? error->info().errPos : positions[expr.getPos()],
.expr = expr,
.env = env,
.hint = error->info().msg,
Expand Down Expand Up @@ -909,7 +910,7 @@ static std::unique_ptr<DebugTraceStacker> makeDebugTraceStacker(
EvalState & state,
Expr & expr,
Env & env,
std::shared_ptr<AbstractPos> && pos,
std::shared_ptr<Pos> && pos,
const char * s,
const std::string & s2)
{
Expand Down Expand Up @@ -1187,7 +1188,7 @@ void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)
*this,
*e,
this->baseEnv,
e->getPos() ? static_cast<std::shared_ptr<AbstractPos>>(positions[e->getPos()]) : nullptr,
e->getPos() ? std::make_shared<Pos>(positions[e->getPos()]) : nullptr,
"while evaluating the file '%1%':", resolvedPath.to_string())
: nullptr;

Expand Down Expand Up @@ -2368,7 +2369,7 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat
auto dstPath = i != srcToStore.end()
? i->second
: [&]() {
auto dstPath = path.fetchToStore(*store, path.baseName(), FileIngestionMethod::Recursive, nullptr, repair);
auto dstPath = fetchToStore(*store, path, path.baseName(), FileIngestionMethod::Recursive, nullptr, repair);
allowPath(dstPath);
srcToStore.insert_or_assign(path, dstPath);
printMsg(lvlChatty, "copied source '%1%' -> '%2%'", path, store->printStorePath(dstPath));
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ struct RegexCache;
std::shared_ptr<RegexCache> makeRegexCache();

struct DebugTrace {
std::shared_ptr<AbstractPos> pos;
std::shared_ptr<Pos> pos;
const Expr & expr;
const Env & env;
hintformat hint;
Expand Down
63 changes: 0 additions & 63 deletions src/libexpr/nixexpr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,58 +11,6 @@ namespace nix {

ExprBlackHole eBlackHole;

struct PosAdapter : AbstractPos
{
Pos::Origin origin;

PosAdapter(Pos::Origin origin)
: origin(std::move(origin))
{
}

std::optional<std::string> getSource() const override
{
return std::visit(overloaded {
[](const Pos::none_tag &) -> std::optional<std::string> {
return std::nullopt;
},
[](const Pos::Stdin & s) -> std::optional<std::string> {
// Get rid of the null terminators added by the parser.
return std::string(s.source->c_str());
},
[](const Pos::String & s) -> std::optional<std::string> {
// Get rid of the null terminators added by the parser.
return std::string(s.source->c_str());
},
[](const SourcePath & path) -> std::optional<std::string> {
try {
return path.readFile();
} catch (Error &) {
return std::nullopt;
}
}
}, origin);
}

void print(std::ostream & out) const override
{
std::visit(overloaded {
[&](const Pos::none_tag &) { out << "«none»"; },
[&](const Pos::Stdin &) { out << "«stdin»"; },
[&](const Pos::String & s) { out << "«string»"; },
[&](const SourcePath & path) { out << path; }
}, origin);
}
};

Pos::operator std::shared_ptr<AbstractPos>() const
{
auto pos = std::make_shared<PosAdapter>(origin);
pos->line = line;
pos->column = column;
return pos;
}

// FIXME: remove, because *symbols* are abstract and do not have a single
// textual representation; see printIdentifier()
std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol)
Expand Down Expand Up @@ -268,17 +216,6 @@ void ExprPos::show(const SymbolTable & symbols, std::ostream & str) const
}


std::ostream & operator << (std::ostream & str, const Pos & pos)
{
if (auto pos2 = (std::shared_ptr<AbstractPos>) pos) {
str << *pos2;
} else
str << "undefined position";

return str;
}


std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath)
{
std::ostringstream out;
Expand Down
26 changes: 2 additions & 24 deletions src/libexpr/nixexpr.hh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "symbol-table.hh"
#include "error.hh"
#include "chunked-vector.hh"
#include "position.hh"

namespace nix {

Expand All @@ -28,27 +29,6 @@ public:
using EvalError::EvalError;
};

/**
* Position objects.
*/
struct Pos
{
uint32_t line;
uint32_t column;

struct none_tag { };
struct Stdin { ref<std::string> source; };
struct String { ref<std::string> source; };

typedef std::variant<none_tag, Stdin, String, SourcePath> Origin;

Origin origin;

explicit operator bool() const { return line > 0; }

operator std::shared_ptr<AbstractPos>() const;
};

class PosIdx {
friend class PosTable;

Expand Down Expand Up @@ -81,7 +61,7 @@ public:
mutable uint32_t idx = std::numeric_limits<uint32_t>::max();

// Used for searching in PosTable::[].
explicit Origin(uint32_t idx): idx(idx), origin{Pos::none_tag()} {}
explicit Origin(uint32_t idx): idx(idx), origin{std::monostate()} {}

public:
const Pos::Origin origin;
Expand Down Expand Up @@ -132,8 +112,6 @@ public:

inline PosIdx noPos = {};

std::ostream & operator << (std::ostream & str, const Pos & pos);


struct Env;
struct Value;
Expand Down
3 changes: 2 additions & 1 deletion src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "value-to-xml.hh"
#include "primops.hh"
#include "fs-input-accessor.hh"
#include "fetch-to-store.hh"

#include <boost/container/small_vector.hpp>
#include <nlohmann/json.hpp>
Expand Down Expand Up @@ -2240,7 +2241,7 @@ static void addPath(
});

if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) {
auto dstPath = path.fetchToStore(*state.store, name, method, filter.get(), state.repair);
auto dstPath = fetchToStore(*state.store, path, name, method, filter.get(), state.repair);
if (expectedHash && expectedStorePath != dstPath)
state.debugThrowLastTrace(Error("store path mismatch in (possibly filtered) path added from '%s'", path));
state.allowAndSetStorePathString(dstPath, v);
Expand Down
1 change: 1 addition & 0 deletions src/libexpr/value.hh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "symbol-table.hh"
#include "value/context.hh"
#include "input-accessor.hh"
#include "source-path.hh"

#if HAVE_BOEHMGC
#include <gc/gc_allocator.h>
Expand Down
68 changes: 68 additions & 0 deletions src/libfetchers/fetch-to-store.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#include "fetch-to-store.hh"
#include "fetchers.hh"
#include "cache.hh"

namespace nix {

StorePath fetchToStore(
Store & store,
const SourcePath & path,
std::string_view name,
ContentAddressMethod method,
PathFilter * filter,
RepairFlag repair)
{
// FIXME: add an optimisation for the case where the accessor is
// an FSInputAccessor pointing to a store path.

std::optional<fetchers::Attrs> cacheKey;

if (!filter && path.accessor->fingerprint) {
cacheKey = fetchers::Attrs{
{"_what", "fetchToStore"},
{"store", store.storeDir},
{"name", std::string(name)},
{"fingerprint", *path.accessor->fingerprint},
{
"method",
std::visit(overloaded {
[](const TextIngestionMethod &) {
return "text";
},
[](const FileIngestionMethod & fim) {
switch (fim) {
case FileIngestionMethod::Flat: return "flat";
case FileIngestionMethod::Recursive: return "nar";
default: assert(false);
}
},
}, method.raw),
},
{"path", path.path.abs()}
};
if (auto res = fetchers::getCache()->lookup(store, *cacheKey)) {
debug("store path cache hit for '%s'", path);
return res->second;
}
} else
debug("source path '%s' is uncacheable", path);

Activity act(*logger, lvlChatty, actUnknown, fmt("copying '%s' to the store", path));

auto filter2 = filter ? *filter : defaultPathFilter;

auto storePath =
settings.readOnlyMode
? store.computeStorePath(
name, *path.accessor, path.path, method, HashAlgorithm::SHA256, {}, filter2).first
: store.addToStore(
name, *path.accessor, path.path, method, HashAlgorithm::SHA256, {}, filter2, repair);

if (cacheKey)
fetchers::getCache()->add(store, *cacheKey, {}, storePath, true);

return storePath;
}


}
22 changes: 22 additions & 0 deletions src/libfetchers/fetch-to-store.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#pragma once

#include "file-ingestion-method.hh"
#include "source-path.hh"
#include "store-api.hh"
#include "file-system.hh"
#include "repair-flag.hh"

namespace nix {

/**
* Copy the `path` to the Nix store.
*/
StorePath fetchToStore(
Store & store,
const SourcePath & path,
std::string_view name = "source",
ContentAddressMethod method = FileIngestionMethod::Recursive,
PathFilter * filter = nullptr,
RepairFlag repair = NoRepair);

}
4 changes: 3 additions & 1 deletion src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "fetchers.hh"
#include "store-api.hh"
#include "input-accessor.hh"
#include "source-path.hh"
#include "fetch-to-store.hh"

#include <nlohmann/json.hpp>

Expand Down Expand Up @@ -374,7 +376,7 @@ void InputScheme::clone(const Input & input, const Path & destDir) const
std::pair<StorePath, Input> InputScheme::fetch(ref<Store> store, const Input & input)
{
auto [accessor, input2] = getAccessor(store, input);
auto storePath = SourcePath(accessor).fetchToStore(*store, input2.getName());
auto storePath = fetchToStore(*store, SourcePath(accessor), input2.getName());
return {storePath, input2};
}

Expand Down
1 change: 1 addition & 0 deletions src/libfetchers/filtering-input-accessor.hh
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "input-accessor.hh"
#include "source-path.hh"

namespace nix {

Expand Down
1 change: 1 addition & 0 deletions src/libfetchers/fs-input-accessor.hh
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "input-accessor.hh"
#include "source-path.hh"

namespace nix {

Expand Down
Loading

0 comments on commit e824cb8

Please sign in to comment.