Skip to content

Commit 6ea4c65

Browse files
committed
PosixSourceAccessor: always require a root
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.
1 parent 1f688d6 commit 6ea4c65

File tree

11 files changed

+47
-19
lines changed

11 files changed

+47
-19
lines changed

src/libexpr/eval.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,14 +247,14 @@ EvalState::EvalState(
247247
, emptyBindings(0)
248248
, rootFS(
249249
settings.restrictEval || settings.pureEval
250-
? ref<SourceAccessor>(AllowListSourceAccessor::create(getFSSourceAccessor(), {},
250+
? ref<SourceAccessor>(AllowListSourceAccessor::create(makeFSSourceAccessor(std::filesystem::path { store->storeDir }.root_path()), {},
251251
[&settings](const CanonPath & path) -> RestrictedPathError {
252252
auto modeInformation = settings.pureEval
253253
? "in pure evaluation mode (use '--impure' to override)"
254254
: "in restricted mode";
255255
throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation);
256256
}))
257-
: getFSSourceAccessor())
257+
: makeFSSourceAccessor(std::filesystem::path { store->storeDir }.root_path()))
258258
, corepkgsFS(make_ref<MemorySourceAccessor>())
259259
, internalFS(make_ref<MemorySourceAccessor>())
260260
, derivationInternal{corepkgsFS->addFile(

src/libstore/build/worker.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ bool Worker::pathContentsGood(const StorePath & path)
568568
res = false;
569569
else {
570570
auto current = hashPath(
571-
{store.getFSAccessor(), CanonPath(store.printStorePath(path))},
571+
{store.getFSAccessor(), store.canonStorePath(path)},
572572
FileIngestionMethod::NixArchive, info->narHash.algo).first;
573573
Hash nullHash(HashAlgorithm::SHA256);
574574
res = info->narHash == nullHash || info->narHash == current;

src/libstore/local-fs-store.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,15 @@ struct LocalStoreAccessor : PosixSourceAccessor
3232
ref<LocalFSStore> store;
3333
bool requireValidPath;
3434

35-
LocalStoreAccessor(ref<LocalFSStore> store, bool requireValidPath)
36-
: store(store)
35+
LocalStoreAccessor(ref<LocalFSStore> store, bool requireValidPath, std::filesystem::path && root)
36+
: PosixSourceAccessor(std::move(root))
37+
, store(store)
3738
, requireValidPath(requireValidPath)
3839
{ }
3940

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

7677
ref<SourceAccessor> LocalFSStore::getFSAccessor(bool requireValidPath)
7778
{
79+
auto root = std::filesystem::path { storeDir }.root_path();
7880
return make_ref<LocalStoreAccessor>(ref<LocalFSStore>(
7981
std::dynamic_pointer_cast<LocalFSStore>(shared_from_this())),
80-
requireValidPath);
82+
requireValidPath,
83+
std::move(root));
8184
}
8285

8386
void LocalFSStore::narFromPath(const StorePath & path, Sink & sink)

src/libstore/local-store.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,7 +1099,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
10991099
auto & specified = *info.ca;
11001100
auto actualHash = ({
11011101
auto accessor = getFSAccessor(false);
1102-
CanonPath path { printStorePath(info.path) };
1102+
auto path = canonStorePath(info.path);
11031103
Hash h { HashAlgorithm::SHA256 }; // throwaway def to appease C++
11041104
auto fim = specified.method.getFileIngestionMethod();
11051105
switch (fim) {
@@ -1383,7 +1383,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
13831383
checkInterrupt();
13841384
auto name = link.path().filename();
13851385
printMsg(lvlTalkative, "checking contents of '%s'", name);
1386-
PosixSourceAccessor accessor;
1386+
PosixSourceAccessor accessor(std::filesystem::current_path().root_path());
13871387
std::string hash = hashPath(
13881388
PosixSourceAccessor::createAtRoot(link.path()),
13891389
FileIngestionMethod::NixArchive, HashAlgorithm::SHA256).first.to_string(HashFormat::Nix32, false);

src/libstore/optimise-store.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
139139
return;
140140
}
141141

142+
auto root = std::filesystem::path { storeDir }.root_path();
142143
/* Hash the file. Note that hashPath() returns the hash over the
143144
NAR serialisation, which includes the execute bit on the file.
144145
Thus, executable and non-executable files with the same
@@ -150,7 +151,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
150151
the contents of the target (which may not even exist). */
151152
Hash hash = ({
152153
hashPath(
153-
{make_ref<PosixSourceAccessor>(), CanonPath(path)},
154+
{make_ref<PosixSourceAccessor>(std::move(root)), CanonPath(path)},
154155
FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256).first;
155156
});
156157
debug("'%1%' has hash '%2%'", path, hash.to_string(HashFormat::Nix32, true));

src/libstore/path.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,12 @@ std::string StoreDirConfig::printStorePath(const StorePath & path) const
120120
return (storeDir + "/").append(path.to_string());
121121
}
122122

123+
CanonPath StoreDirConfig::canonStorePath(const StorePath & path) const
124+
{
125+
auto relativeStoreDir = std::filesystem::path(storeDir).relative_path();
126+
return CanonPath(relativeStoreDir.string()) / path.to_string();
127+
}
128+
123129
PathSet StoreDirConfig::printStorePathSet(const StorePathSet & paths) const
124130
{
125131
PathSet res;

src/libstore/store-api.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1232,7 +1232,7 @@ static Derivation readDerivationCommon(Store & store, const StorePath & drvPath,
12321232
auto accessor = store.getFSAccessor(requireValidPath);
12331233
try {
12341234
return parseDerivation(store,
1235-
accessor->readFile(CanonPath(store.printStorePath(drvPath))),
1235+
accessor->readFile(store.canonStorePath(drvPath)),
12361236
Derivation::nameFromPath(drvPath));
12371237
} catch (FormatError & e) {
12381238
throw Error("error parsing derivation '%s': %s", store.printStorePath(drvPath), e.msg());

src/libstore/store-dir-config.hh

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,17 @@ struct StoreDirConfig : public Config
4343

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

46+
/**
47+
* Temporary.
48+
*
49+
* At the moment FS accessors usually have a root of / or C: or similar.
50+
* Eventually FS accessors will have roots of /nix/store or C:\nix\store
51+
* but for now, we have this helper function which will remove the root.
52+
*
53+
* \todo remove
54+
*/
55+
CanonPath canonStorePath(const StorePath & path) const;
56+
4657
/**
4758
* Deprecated
4859
*

src/libutil/posix-source-accessor.cc

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,10 @@ namespace nix {
1010
PosixSourceAccessor::PosixSourceAccessor(std::filesystem::path && argRoot)
1111
: root(std::move(argRoot))
1212
{
13-
assert(root.empty() || root.is_absolute());
14-
displayPrefix = root.string();
13+
assert(root.is_absolute());
14+
displayPrefix = "";
1515
}
1616

17-
PosixSourceAccessor::PosixSourceAccessor()
18-
: PosixSourceAccessor(std::filesystem::path {})
19-
{ }
2017

2118
SourcePath PosixSourceAccessor::createAtRoot(const std::filesystem::path & path)
2219
{
@@ -199,9 +196,18 @@ void PosixSourceAccessor::assertNoSymlinks(CanonPath path)
199196
}
200197
}
201198

199+
std::string PosixSourceAccessor::showPath(const CanonPath & path)
200+
{
201+
return displayPrefix + (root / std::filesystem::path(path.rel())).make_preferred().string() + displaySuffix;
202+
}
203+
202204
ref<SourceAccessor> getFSSourceAccessor()
203205
{
204-
static auto rootFS = make_ref<PosixSourceAccessor>();
206+
// HACK: We need a root, so grab one, even if it has a good chance of being incorrect on Windows.
207+
// For example, nix.exe could be on Z: but the store could actually be in C:
208+
// TODO: Remove getFSSourceAccessor() and only use makeFSSourceAccessor(root)
209+
auto root = std::filesystem::current_path().root_path();
210+
static auto rootFS = make_ref<PosixSourceAccessor>(std::move(root));
205211
return rootFS;
206212
}
207213

src/libutil/posix-source-accessor.hh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ struct PosixSourceAccessor : virtual SourceAccessor
1818
*/
1919
const std::filesystem::path root;
2020

21-
PosixSourceAccessor();
2221
PosixSourceAccessor(std::filesystem::path && root);
2322

2423
/**
@@ -42,6 +41,8 @@ struct PosixSourceAccessor : virtual SourceAccessor
4241

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

44+
virtual std::string showPath(const CanonPath & path) override;
45+
4546
/**
4647
* Create a `PosixSourceAccessor` and `SourcePath` corresponding to
4748
* some native path.

0 commit comments

Comments
 (0)