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.

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

Make ValidPath (nar) hash optional if ca field is present #4059

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Introduce These and ContentAddresses in path-info
  • Loading branch information
meditans committed Aug 26, 2020
commit 57fc85ae07ab080bf2f7a223ebe762005c485fef
5 changes: 4 additions & 1 deletion src/libexpr/primops/fetchTree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,11 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v,
auto path = state.store->toRealPath(storePath);

if (expectedHash) {
auto narHashResult = *viewFirstConst(state.store->queryPathInfo(storePath)->cas);
assert(narHashResult);
auto narHash = narHashResult->first;
auto hash = unpack
? state.store->queryPathInfo(storePath)->narHash
? narHash
: hashFile(htSHA256, path);
if (hash != *expectedHash)
throw Error((unsigned int) 102, "hash mismatch in file downloaded from '%s':\n wanted: %s\n got: %s",
Expand Down
2 changes: 1 addition & 1 deletion src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ std::pair<Tree, Input> Input::fetch(ref<Store> store) const
if (tree.actualPath == "")
tree.actualPath = store->toRealPath(tree.storePath);

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

if (auto prevNarHash = getNarHash()) {
Expand Down
10 changes: 4 additions & 6 deletions src/libfetchers/tarball.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,10 @@ DownloadFileResult downloadFile(
auto hash = hashString(htSHA256, *res.data);
ValidPathInfo info {
store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name),
hashString(htSHA256, *sink.s),
};
info.narSize = sink.s->size();
info.ca = FixedOutputHash {
.method = FileIngestionMethod::Flat,
.hash = hash,
std::pair<HashResult, ContentAddress> {
{ hashString(htSHA256, *sink.s), sink.s->size() },
FixedOutputHash { .method = FileIngestionMethod::Flat, .hash = hash },
},
};
auto source = StringSource { *sink.s };
store->addToStore(info, source, NoRepair, NoCheckSigs);
Expand Down
14 changes: 6 additions & 8 deletions src/libstore/binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ struct FileSource : FdSource
void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource,
RepairFlag repair, CheckSigsFlag checkSigs)
{
assert(info.narSize);
assert(info.narSize());

if (!repair && isValidPath(info.path)) {
// FIXME: copyNAR -> null sink
Expand Down Expand Up @@ -175,8 +175,6 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource
auto now2 = std::chrono::steady_clock::now();

auto narInfo = make_ref<NarInfo>(info);
narInfo->narSize = info.narSize;
narInfo->narHash = info.narHash;
narInfo->compression = compression;
auto [fileHash, fileSize] = fileHashSink.finish();
narInfo->fileHash = fileHash;
Expand All @@ -189,8 +187,8 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource

auto duration = std::chrono::duration_cast<std::chrono::milliseconds>(now2 - now1).count();
printMsg(lvlTalkative, "copying path '%1%' (%2% bytes, compressed %3$.1f%% in %4% ms) to binary cache",
printStorePath(narInfo->path), info.narSize,
((1.0 - (double) fileSize / info.narSize) * 100.0),
printStorePath(narInfo->path), info.narSize(),
((1.0 - (double) fileSize / info.narSize()) * 100.0),
duration);

/* Verify that all references are valid. This may do some .narinfo
Expand Down Expand Up @@ -288,7 +286,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource
} else
stats.narWriteAverted++;

stats.narWriteBytes += info.narSize;
stats.narWriteBytes += info.narSize();
stats.narWriteCompressedBytes += fileSize;
stats.narWriteCompressionTimeMs += duration;

Expand Down Expand Up @@ -383,7 +381,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath

ValidPathInfo info {
makeFixedOutputPath(method, *h, name),
Hash::dummy, // Will be fixed in addToStore, which recomputes nar hash
This<HashResult> { { Hash::dummy, 0 } }, // Will be fixed in addToStore, which recomputes nar hash
};

auto source = StringSource { *sink.s };
Expand All @@ -397,7 +395,7 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s
{
ValidPathInfo info {
computeStorePathForText(name, s, references),
Hash::dummy, // Will be fixed in addToStore, which recomputes nar hash
This<HashResult> { { Hash::dummy, 0 } }, // Will be fixed in addToStore, which recomputes nar hash
};
info.references = references;

Expand Down
24 changes: 12 additions & 12 deletions src/libstore/build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3820,7 +3820,7 @@ void DerivationGoal::registerOutputs()
if (buildMode == bmCheck) {
if (!worker.store.isValidPath(worker.store.parseStorePath(path))) continue;
ValidPathInfo info(*worker.store.queryPathInfo(worker.store.parseStorePath(path)));
if (hash.first != info.narHash) {
if (hash.first != info.narHash()) {
worker.checkMismatch = true;
if (settings.runDiffHook || settings.keepFailed) {
Path dst = worker.store.toRealPath(path + checkSuffix);
Expand Down Expand Up @@ -3867,18 +3867,17 @@ void DerivationGoal::registerOutputs()

ValidPathInfo info {
worker.store.parseStorePath(path),
hash.first,
This<HashResult> { hash },
};
info.narSize = hash.second;
info.references = std::move(references);
info.deriver = drvPath;
info.ultimate = true;
info.ca = ca;
viewSecond(info.cas).add(ca);
worker.store.signPathInfo(info);

if (!info.references.empty()) {
// FIXME don't we have an experimental feature for fixed output with references?
info.ca = {};
viewSecond(info.cas) = std::nullopt;
}

infos.emplace(i.first, std::move(info));
Expand Down Expand Up @@ -3998,12 +3997,12 @@ void DerivationGoal::checkOutputs(const std::map<Path, ValidPathInfo> & outputs)

auto i = outputsByPath.find(worker.store.printStorePath(path));
if (i != outputsByPath.end()) {
closureSize += i->second.narSize;
closureSize += i->second.narSize();
for (auto & ref : i->second.references)
pathsLeft.push(ref);
} else {
auto info = worker.store.queryPathInfo(path);
closureSize += info->narSize;
closureSize += info->narSize();
for (auto & ref : info->references)
pathsLeft.push(ref);
}
Expand All @@ -4014,9 +4013,9 @@ void DerivationGoal::checkOutputs(const std::map<Path, ValidPathInfo> & outputs)

auto applyChecks = [&](const Checks & checks)
{
if (checks.maxSize && info.narSize > *checks.maxSize)
if (checks.maxSize && info.narSize() > *checks.maxSize)
throw BuildError("path '%s' is too large at %d bytes; limit is %d bytes",
worker.store.printStorePath(info.path), info.narSize, *checks.maxSize);
worker.store.printStorePath(info.path), info.narSize(), *checks.maxSize);

if (checks.maxClosureSize) {
uint64_t closureSize = getClosure(info.path).second;
Expand Down Expand Up @@ -4499,7 +4498,7 @@ void SubstitutionGoal::tryNext()
/* Update the total expected download size. */
auto narInfo = std::dynamic_pointer_cast<const NarInfo>(info);

maintainExpectedNar = std::make_unique<MaintainCount<uint64_t>>(worker.expectedNarSize, info->narSize);
maintainExpectedNar = std::make_unique<MaintainCount<uint64_t>>(worker.expectedNarSize, info->narSize());

maintainExpectedDownload =
narInfo && narInfo->fileSize
Expand Down Expand Up @@ -5073,9 +5072,10 @@ bool Worker::pathContentsGood(const StorePath & path)
if (!pathExists(store.printStorePath(path)))
res = false;
else {
HashResult current = hashPath(info->narHash.type, store.printStorePath(path));
Hash narHash = (*viewFirstConst(info->cas))->first;
HashResult current = hashPath(narHash.type, store.printStorePath(path));
Hash nullHash(htSHA256);
res = info->narHash == nullHash || info->narHash == current.first;
res = narHash == nullHash || narHash == current.first;
}
pathContentsGoodCache.insert_or_assign(path, res);
if (!res)
Expand Down
12 changes: 12 additions & 0 deletions src/libstore/content-address.hh
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,24 @@ enum struct FileIngestionMethod : uint8_t {

struct TextHash {
Hash hash;
bool operator ==(TextHash otherHash) const noexcept {
return hash == otherHash.hash;
};
bool operator !=(TextHash otherHash) const noexcept {
return hash != otherHash.hash;
};
};

/// Pair of a hash, and how the file system was ingested
struct FixedOutputHash {
FileIngestionMethod method;
Hash hash;
bool operator ==(FixedOutputHash otherHash) const noexcept {
return method == otherHash.method && hash == otherHash.hash;
};
bool operator !=(FixedOutputHash otherHash) const noexcept {
return method != otherHash.method && hash == otherHash.hash;
Ericson2314 marked this conversation as resolved.
Show resolved Hide resolved
};
std::string printMethodAlgo() const;
};

Expand Down
28 changes: 20 additions & 8 deletions src/libstore/daemon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,11 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
case wopQueryPathHash: {
auto path = store->parseStorePath(readString(from));
logger->startWork();
auto hash = store->queryPathInfo(path)->narHash;
auto narHashResult = *viewFirstConst(store->queryPathInfo(path)->cas);
assert(narHashResult);
auto narHash = narHashResult->first;
logger->stopWork();
to << hash.to_string(Base16, false);
to << narHash.to_string(Base16, false);
break;
}

Expand Down Expand Up @@ -675,14 +677,18 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
if (info) {
if (GET_PROTOCOL_MINOR(clientVersion) >= 17)
to << 1;
auto narHashResult = *viewFirstConst(info->cas);
assert(narHashResult);
auto narHash = narHashResult->first;
auto narSize = narHashResult->second;
to << (info->deriver ? store->printStorePath(*info->deriver) : "")
<< info->narHash.to_string(Base16, false);
<< narHash.to_string(Base16, false);
writeStorePaths(*store, to, info->references);
to << info->registrationTime << info->narSize;
to << info->registrationTime << narSize;
if (GET_PROTOCOL_MINOR(clientVersion) >= 16) {
to << info->ultimate
<< info->sigs
<< renderContentAddress(info->ca);
<< renderContentAddress(*viewSecondConst(info->cas));
}
} else {
assert(GET_PROTOCOL_MINOR(clientVersion) >= 17);
Expand Down Expand Up @@ -735,13 +741,19 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
auto path = store->parseStorePath(readString(from));
auto deriver = readString(from);
auto narHash = Hash::parseAny(readString(from), htSHA256);
ValidPathInfo info { path, narHash };
ValidPathInfo info { path, This<HashResult> { { narHash, 0 } } };
if (deriver != "")
info.deriver = store->parseStorePath(deriver);
info.references = readStorePaths<StorePathSet>(*store, from);
from >> info.registrationTime >> info.narSize >> info.ultimate;
from >> info.registrationTime;
{
auto tempNarSize = readInt(from);
auto tempHashResult = **viewFirst(info.cas);
viewFirst(info.cas) = { tempHashResult.first, tempNarSize };
}
from >> info.ultimate;
info.sigs = readStrings<StringSet>(from);
info.ca = parseContentAddressOpt(readString(from));
viewSecond(info.cas).add(parseContentAddressOpt(readString(from)));
from >> repair >> dontCheckSigs;
if (!trusted && dontCheckSigs)
dontCheckSigs = false;
Expand Down
13 changes: 9 additions & 4 deletions src/libstore/export-import.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ void Store::exportPath(const StorePath & path, Sink & sink)
filesystem corruption from spreading to other machines.
Don't complain if the stored hash is zero (unknown). */
Hash hash = hashSink.currentHash().first;
if (hash != info->narHash && info->narHash != Hash(info->narHash.type))
auto narHashResult = *viewFirstConst(info->cas);
assert(narHashResult);
auto narHash = narHashResult->first;
if (hash != narHash && narHash != Hash(narHash.type))
throw Error("hash of path '%s' has changed from '%s' to '%s'!",
printStorePath(path), info->narHash.to_string(Base32, true), hash.to_string(Base32, true));
printStorePath(path), narHash.to_string(Base32, true), hash.to_string(Base32, true));

teeSink
<< exportMagic
Expand Down Expand Up @@ -77,11 +80,13 @@ StorePaths Store::importPaths(Source & source, CheckSigsFlag checkSigs)
auto deriver = readString(source);
auto narHash = hashString(htSHA256, *saved.s);

ValidPathInfo info { path, narHash };
ValidPathInfo info {
path,
ContentAddresses { This<HashResult> { std::pair { narHash, saved.s->size() } } },
};
if (deriver != "")
info.deriver = parseStorePath(deriver);
info.references = references;
info.narSize = saved.s->size();

// Ignore optional legacy signature.
if (readInt(source) == 1)
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/gc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ void LocalStore::deletePathRecursive(GCState & state, const Path & path)
queryReferrers(*storePath, referrers);
for (auto & i : referrers)
if (printStorePath(i) != path) deletePathRecursive(state, printStorePath(i));
size = queryPathInfo(*storePath)->narSize;
size = queryPathInfo(*storePath)->narSize();
invalidatePathChecked(*storePath);
}

Expand Down
17 changes: 10 additions & 7 deletions src/libstore/legacy-ssh-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,23 +106,26 @@ struct LegacySSHStore : public Store
auto path2 = parseStorePath(p);
assert(path == path2);
/* Hash will be set below. FIXME construct ValidPathInfo at end. */
auto info = std::make_shared<ValidPathInfo>(path, Hash::dummy);
auto info = std::make_shared<ValidPathInfo>(path, This<HashResult> { { Hash::dummy, 0 } });

PathSet references;
auto deriver = readString(conn->from);
if (deriver != "")
info->deriver = parseStorePath(deriver);
info->references = readStorePaths<StorePathSet>(*this, conn->from);
readLongLong(conn->from); // download size
info->narSize = readLongLong(conn->from);

auto narHashResult = *viewFirst(info->cas);
assert(narHashResult);
narHashResult->second = readLongLong(conn->from);

{
auto s = readString(conn->from);
if (s == "")
throw Error("NAR hash is now mandatory");
info->narHash = Hash::parseAnyPrefixed(s);
narHashResult->first = Hash::parseAnyPrefixed(s);
}
info->ca = parseContentAddressOpt(readString(conn->from));
viewSecond(info->cas) = parseContentAddressOpt(readString(conn->from));
info->sigs = readStrings<StringSet>(conn->from);

auto s = readString(conn->from);
Expand All @@ -145,14 +148,14 @@ struct LegacySSHStore : public Store
<< cmdAddToStoreNar
<< printStorePath(info.path)
<< (info.deriver ? printStorePath(*info.deriver) : "")
<< info.narHash.to_string(Base16, false);
<< (*viewFirstConst(info.cas))->first.to_string(Base16, false);
writeStorePaths(*this, conn->to, info.references);
conn->to
<< info.registrationTime
<< info.narSize
<< info.narSize()
<< info.ultimate
<< info.sigs
<< renderContentAddress(info.ca);
<< renderContentAddress(*viewSecondConst(info.cas));
try {
copyNAR(source, conn->to);
} catch (...) {
Expand Down
Loading