Skip to content

Commit

Permalink
PosixSourceAccessor: always require a root
Browse files Browse the repository at this point in the history
With the introduction of CanonPath, we don't have roots on paths we
want to access. For example, /nix/store/abc doesn't say whether we
need to use C: or D: on Windows.

This commit keeps getFSSourceAccessor() which does NOT have a root
specified, so it pulls one from the current working directory. This is
silly, since nix.exe could be on Z: but the Nix store could be on C:
which means we'll build wrong paths. More commits will have to come
which removes getFSSourceAccessor() altogether.

This shouldn't change anything on Unix systems, since the root should
always resolve to / for every possible path.
  • Loading branch information
puffnfresh committed Feb 18, 2025
1 parent 1f688d6 commit 6ea4c65
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 19 deletions.
4 changes: 2 additions & 2 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,14 @@ EvalState::EvalState(
, emptyBindings(0)
, rootFS(
settings.restrictEval || settings.pureEval
? ref<SourceAccessor>(AllowListSourceAccessor::create(getFSSourceAccessor(), {},
? ref<SourceAccessor>(AllowListSourceAccessor::create(makeFSSourceAccessor(std::filesystem::path { store->storeDir }.root_path()), {},
[&settings](const CanonPath & path) -> RestrictedPathError {
auto modeInformation = settings.pureEval
? "in pure evaluation mode (use '--impure' to override)"
: "in restricted mode";
throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation);
}))
: getFSSourceAccessor())
: makeFSSourceAccessor(std::filesystem::path { store->storeDir }.root_path()))
, corepkgsFS(make_ref<MemorySourceAccessor>())
, internalFS(make_ref<MemorySourceAccessor>())
, derivationInternal{corepkgsFS->addFile(
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/build/worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ bool Worker::pathContentsGood(const StorePath & path)
res = false;
else {
auto current = hashPath(
{store.getFSAccessor(), CanonPath(store.printStorePath(path))},
{store.getFSAccessor(), store.canonStorePath(path)},
FileIngestionMethod::NixArchive, info->narHash.algo).first;
Hash nullHash(HashAlgorithm::SHA256);
res = info->narHash == nullHash || info->narHash == current;
Expand Down
11 changes: 7 additions & 4 deletions src/libstore/local-fs-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ struct LocalStoreAccessor : PosixSourceAccessor
ref<LocalFSStore> store;
bool requireValidPath;

LocalStoreAccessor(ref<LocalFSStore> store, bool requireValidPath)
: store(store)
LocalStoreAccessor(ref<LocalFSStore> store, bool requireValidPath, std::filesystem::path && root)
: PosixSourceAccessor(std::move(root))
, store(store)
, requireValidPath(requireValidPath)
{ }

CanonPath toRealPath(const CanonPath & path)
{
auto [storePath, rest] = store->toStorePath(path.abs());
auto [storePath, rest] = store->toStorePath((root / path.rel()).string());
if (requireValidPath && !store->isValidPath(storePath))
throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath));
return CanonPath(store->getRealStoreDir()) / storePath.to_string() / CanonPath(rest);
Expand Down Expand Up @@ -75,9 +76,11 @@ struct LocalStoreAccessor : PosixSourceAccessor

ref<SourceAccessor> LocalFSStore::getFSAccessor(bool requireValidPath)
{
auto root = std::filesystem::path { storeDir }.root_path();
return make_ref<LocalStoreAccessor>(ref<LocalFSStore>(
std::dynamic_pointer_cast<LocalFSStore>(shared_from_this())),
requireValidPath);
requireValidPath,
std::move(root));
}

void LocalFSStore::narFromPath(const StorePath & path, Sink & sink)
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1099,7 +1099,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
auto & specified = *info.ca;
auto actualHash = ({
auto accessor = getFSAccessor(false);
CanonPath path { printStorePath(info.path) };
auto path = canonStorePath(info.path);
Hash h { HashAlgorithm::SHA256 }; // throwaway def to appease C++
auto fim = specified.method.getFileIngestionMethod();
switch (fim) {
Expand Down Expand Up @@ -1383,7 +1383,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
checkInterrupt();
auto name = link.path().filename();
printMsg(lvlTalkative, "checking contents of '%s'", name);
PosixSourceAccessor accessor;
PosixSourceAccessor accessor(std::filesystem::current_path().root_path());
std::string hash = hashPath(
PosixSourceAccessor::createAtRoot(link.path()),
FileIngestionMethod::NixArchive, HashAlgorithm::SHA256).first.to_string(HashFormat::Nix32, false);
Expand Down
3 changes: 2 additions & 1 deletion src/libstore/optimise-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
return;
}

auto root = std::filesystem::path { storeDir }.root_path();
/* Hash the file. Note that hashPath() returns the hash over the
NAR serialisation, which includes the execute bit on the file.
Thus, executable and non-executable files with the same
Expand All @@ -150,7 +151,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
the contents of the target (which may not even exist). */
Hash hash = ({
hashPath(
{make_ref<PosixSourceAccessor>(), CanonPath(path)},
{make_ref<PosixSourceAccessor>(std::move(root)), CanonPath(path)},
FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256).first;
});
debug("'%1%' has hash '%2%'", path, hash.to_string(HashFormat::Nix32, true));
Expand Down
6 changes: 6 additions & 0 deletions src/libstore/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ std::string StoreDirConfig::printStorePath(const StorePath & path) const
return (storeDir + "/").append(path.to_string());
}

CanonPath StoreDirConfig::canonStorePath(const StorePath & path) const
{
auto relativeStoreDir = std::filesystem::path(storeDir).relative_path();
return CanonPath(relativeStoreDir.string()) / path.to_string();
}

PathSet StoreDirConfig::printStorePathSet(const StorePathSet & paths) const
{
PathSet res;
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/store-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1232,7 +1232,7 @@ static Derivation readDerivationCommon(Store & store, const StorePath & drvPath,
auto accessor = store.getFSAccessor(requireValidPath);
try {
return parseDerivation(store,
accessor->readFile(CanonPath(store.printStorePath(drvPath))),
accessor->readFile(store.canonStorePath(drvPath)),
Derivation::nameFromPath(drvPath));
} catch (FormatError & e) {
throw Error("error parsing derivation '%s': %s", store.printStorePath(drvPath), e.msg());
Expand Down
11 changes: 11 additions & 0 deletions src/libstore/store-dir-config.hh
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ struct StoreDirConfig : public Config

std::string printStorePath(const StorePath & path) const;

/**
* Temporary.
*
* At the moment FS accessors usually have a root of / or C: or similar.
* Eventually FS accessors will have roots of /nix/store or C:\nix\store
* but for now, we have this helper function which will remove the root.
*
* \todo remove
*/
CanonPath canonStorePath(const StorePath & path) const;

/**
* Deprecated
*
Expand Down
18 changes: 12 additions & 6 deletions src/libutil/posix-source-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,10 @@ namespace nix {
PosixSourceAccessor::PosixSourceAccessor(std::filesystem::path && argRoot)
: root(std::move(argRoot))
{
assert(root.empty() || root.is_absolute());
displayPrefix = root.string();
assert(root.is_absolute());
displayPrefix = "";
}

PosixSourceAccessor::PosixSourceAccessor()
: PosixSourceAccessor(std::filesystem::path {})
{ }

SourcePath PosixSourceAccessor::createAtRoot(const std::filesystem::path & path)
{
Expand Down Expand Up @@ -199,9 +196,18 @@ void PosixSourceAccessor::assertNoSymlinks(CanonPath path)
}
}

std::string PosixSourceAccessor::showPath(const CanonPath & path)
{
return displayPrefix + (root / std::filesystem::path(path.rel())).make_preferred().string() + displaySuffix;
}

ref<SourceAccessor> getFSSourceAccessor()
{
static auto rootFS = make_ref<PosixSourceAccessor>();
// HACK: We need a root, so grab one, even if it has a good chance of being incorrect on Windows.
// For example, nix.exe could be on Z: but the store could actually be in C:
// TODO: Remove getFSSourceAccessor() and only use makeFSSourceAccessor(root)
auto root = std::filesystem::current_path().root_path();
static auto rootFS = make_ref<PosixSourceAccessor>(std::move(root));
return rootFS;
}

Expand Down
3 changes: 2 additions & 1 deletion src/libutil/posix-source-accessor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ struct PosixSourceAccessor : virtual SourceAccessor
*/
const std::filesystem::path root;

PosixSourceAccessor();
PosixSourceAccessor(std::filesystem::path && root);

/**
Expand All @@ -42,6 +41,8 @@ struct PosixSourceAccessor : virtual SourceAccessor

std::optional<std::filesystem::path> getPhysicalPath(const CanonPath & path) override;

virtual std::string showPath(const CanonPath & path) override;

/**
* Create a `PosixSourceAccessor` and `SourcePath` corresponding to
* some native path.
Expand Down
2 changes: 1 addition & 1 deletion src/nix-store/nix-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise)
#endif
if (!hashGiven) {
HashResult hash = hashPath(
{store->getFSAccessor(false), CanonPath { store->printStorePath(info->path) }},
{store->getFSAccessor(false), store->canonStorePath(info->path) },
FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256);
info->narHash = hash.first;
info->narSize = hash.second;
Expand Down

0 comments on commit 6ea4c65

Please sign in to comment.