Skip to content

Give nix daemon and nix-store --serve protocols separate serializers with version info #6223

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 23, 2023
Merged

Give nix daemon and nix-store --serve protocols separate serializers with version info #6223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
96c5855
Test more derived paths
Ericson2314 Oct 16, 2023
4372738
Systematize the worker protocol derived path serialiser
Ericson2314 May 26, 2023
ab822af
Factor out serialization for `BuildResult`
Ericson2314 Feb 28, 2021
596bd46
Move `ValidPathInfo` serialization code to `worker-protocol.{cc.hh}`
Ericson2314 Mar 8, 2022
70f8b96
Factor out `UnkeyedValidPathInfo` and test
Ericson2314 Apr 17, 2023
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
33 changes: 6 additions & 27 deletions src/libstore/daemon.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,18 +261,6 @@ struct ClientSettings
}
};

static std::vector<DerivedPath> readDerivedPaths(Store & store, WorkerProto::Version clientVersion, WorkerProto::ReadConn conn)
{
std::vector<DerivedPath> reqs;
if (GET_PROTOCOL_MINOR(clientVersion) >= 30) {
reqs = WorkerProto::Serialise<std::vector<DerivedPath>>::read(store, conn);
} else {
for (auto & s : readStrings<Strings>(conn.from))
reqs.push_back(parsePathWithOutputs(store, s).toDerivedPath());
}
return reqs;
}

static void performOp(TunnelLogger * logger, ref<Store> store,
TrustedFlag trusted, RecursiveFlag recursive, WorkerProto::Version clientVersion,
Source & from, BufferedSink & to, WorkerProto::Op op)
Expand Down Expand Up @@ -434,7 +422,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
}();
logger->stopWork();

pathInfo->write(to, *store, GET_PROTOCOL_MINOR(clientVersion));
WorkerProto::Serialise<ValidPathInfo>::write(*store, wconn, *pathInfo);
} else {
HashType hashAlgo;
std::string baseName;
Expand Down Expand Up @@ -538,7 +526,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
}

case WorkerProto::Op::BuildPaths: {
auto drvs = readDerivedPaths(*store, clientVersion, rconn);
auto drvs = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
BuildMode mode = bmNormal;
if (GET_PROTOCOL_MINOR(clientVersion) >= 15) {
mode = (BuildMode) readInt(from);
Expand All @@ -563,7 +551,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
}

case WorkerProto::Op::BuildPathsWithResults: {
auto drvs = readDerivedPaths(*store, clientVersion, rconn);
auto drvs = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
BuildMode mode = bmNormal;
mode = (BuildMode) readInt(from);

Expand Down Expand Up @@ -647,16 +635,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,

auto res = store->buildDerivation(drvPath, drv, buildMode);
logger->stopWork();
to << res.status << res.errorMsg;
if (GET_PROTOCOL_MINOR(clientVersion) >= 29) {
to << res.timesBuilt << res.isNonDeterministic << res.startTime << res.stopTime;
}
if (GET_PROTOCOL_MINOR(clientVersion) >= 28) {
DrvOutputs builtOutputs;
for (auto & [output, realisation] : res.builtOutputs)
builtOutputs.insert_or_assign(realisation.id, realisation);
WorkerProto::write(*store, wconn, builtOutputs);
}
WorkerProto::write(*store, wconn, res);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to factor out each protocol's in a separate commit.

break;
}

Expand Down Expand Up @@ -840,7 +819,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
if (info) {
if (GET_PROTOCOL_MINOR(clientVersion) >= 17)
to << 1;
info->write(to, *store, GET_PROTOCOL_MINOR(clientVersion), false);
WorkerProto::write(*store, wconn, static_cast<const UnkeyedValidPathInfo &>(*info));
} else {
assert(GET_PROTOCOL_MINOR(clientVersion) >= 17);
to << 0;
Expand Down Expand Up @@ -938,7 +917,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
}

case WorkerProto::Op::QueryMissing: {
auto targets = readDerivedPaths(*store, clientVersion, rconn);
auto targets = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
logger->startWork();
StorePathSet willBuild, willSubstitute, unknown;
uint64_t downloadSize, narSize;
Expand Down
15 changes: 1 addition & 14 deletions src/libstore/legacy-ssh-store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,20 +319,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor

conn->to.flush();

BuildResult status;
status.status = (BuildResult::Status) readInt(conn->from);
conn->from >> status.errorMsg;

if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3)
conn->from >> status.timesBuilt >> status.isNonDeterministic >> status.startTime >> status.stopTime;
if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 6) {
auto builtOutputs = ServeProto::Serialise<DrvOutputs>::read(*this, *conn);
for (auto && [output, realisation] : builtOutputs)
status.builtOutputs.insert_or_assign(
std::move(output.outputName),
std::move(realisation));
}
return status;
return ServeProto::Serialise<BuildResult>::read(*this, *conn);
}

void buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMode buildMode, std::shared_ptr<Store> evalStore) override
Expand Down
77 changes: 21 additions & 56 deletions src/libstore/path-info.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,27 @@
#include "path-info.hh"
#include "worker-protocol.hh"
#include "worker-protocol-impl.hh"
#include "store-api.hh"

namespace nix {

GENERATE_CMP_EXT(
,
UnkeyedValidPathInfo,
me->deriver,
me->narHash,
me->references,
me->registrationTime,
me->narSize,
//me->id,
me->ultimate,
me->sigs,
me->ca);

GENERATE_CMP_EXT(
,
ValidPathInfo,
me->path,
static_cast<const UnkeyedValidPathInfo &>(*me));

std::string ValidPathInfo::fingerprint(const Store & store) const
{
if (narSize == 0)
Expand Down Expand Up @@ -99,14 +116,13 @@ Strings ValidPathInfo::shortRefs() const
return refs;
}


ValidPathInfo::ValidPathInfo(
const Store & store,
std::string_view name,
ContentAddressWithReferences && ca,
Hash narHash)
: path(store.makeFixedOutputPathFromCA(name, ca))
, narHash(narHash)
: UnkeyedValidPathInfo(narHash)
, path(store.makeFixedOutputPathFromCA(name, ca))
{
std::visit(overloaded {
[this](TextInfo && ti) {
Expand All @@ -128,55 +144,4 @@ ValidPathInfo::ValidPathInfo(
}, std::move(ca).raw);
}


ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format)
{
return read(source, store, format, store.parseStorePath(readString(source)));
}

ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format, StorePath && path)
{
auto deriver = readString(source);
auto narHash = Hash::parseAny(readString(source), htSHA256);
ValidPathInfo info(path, narHash);
if (deriver != "") info.deriver = store.parseStorePath(deriver);
info.references = WorkerProto::Serialise<StorePathSet>::read(store,
WorkerProto::ReadConn {
.from = source,
.version = format,
});
source >> info.registrationTime >> info.narSize;
if (format >= 16) {
source >> info.ultimate;
info.sigs = readStrings<StringSet>(source);
info.ca = ContentAddress::parseOpt(readString(source));
}
return info;
}


void ValidPathInfo::write(
Sink & sink,
const Store & store,
unsigned int format,
bool includePath) const
{
if (includePath)
sink << store.printStorePath(path);
sink << (deriver ? store.printStorePath(*deriver) : "")
<< narHash.to_string(HashFormat::Base16, false);
WorkerProto::write(store,
WorkerProto::WriteConn {
.to = sink,
.version = format,
},
references);
sink << registrationTime << narSize;
if (format >= 16) {
sink << ultimate
<< sigs
<< renderContentAddress(ca);
}
}

}
36 changes: 22 additions & 14 deletions src/libstore/path-info.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ struct SubstitutablePathInfo
typedef std::map<StorePath, SubstitutablePathInfo> SubstitutablePathInfos;


struct ValidPathInfo
struct UnkeyedValidPathInfo
{
StorePath path;
std::optional<StorePath> deriver;
/**
* \todo document this
Expand Down Expand Up @@ -72,6 +71,20 @@ struct ValidPathInfo
*/
std::optional<ContentAddress> ca;

UnkeyedValidPathInfo(const UnkeyedValidPathInfo & other) = default;

UnkeyedValidPathInfo(Hash narHash) : narHash(narHash) { };

DECLARE_CMP(UnkeyedValidPathInfo);

virtual ~UnkeyedValidPathInfo() { }
};

struct ValidPathInfo : UnkeyedValidPathInfo {
StorePath path;

DECLARE_CMP(ValidPathInfo);

/**
* Return a fingerprint of the store path to be used in binary
* cache signatures. It contains the store path, the base-32
Expand All @@ -84,11 +97,11 @@ struct ValidPathInfo

void sign(const Store & store, const SecretKey & secretKey);

/**
* @return The `ContentAddressWithReferences` that determines the
* store path for a content-addressed store object, `std::nullopt`
* for an input-addressed store object.
*/
/**
* @return The `ContentAddressWithReferences` that determines the
* store path for a content-addressed store object, `std::nullopt`
* for an input-addressed store object.
*/
std::optional<ContentAddressWithReferences> contentAddressWithReferences() const;

/**
Expand All @@ -114,18 +127,13 @@ struct ValidPathInfo

ValidPathInfo(const ValidPathInfo & other) = default;

ValidPathInfo(StorePath && path, Hash narHash) : path(std::move(path)), narHash(narHash) { };
ValidPathInfo(const StorePath & path, Hash narHash) : path(path), narHash(narHash) { };
ValidPathInfo(StorePath && path, UnkeyedValidPathInfo info) : UnkeyedValidPathInfo(info), path(std::move(path)) { };
ValidPathInfo(const StorePath & path, UnkeyedValidPathInfo info) : UnkeyedValidPathInfo(info), path(path) { };

ValidPathInfo(const Store & store,
std::string_view name, ContentAddressWithReferences && ca, Hash narHash);

virtual ~ValidPathInfo() { }

static ValidPathInfo read(Source & source, const Store & store, unsigned int format);
static ValidPathInfo read(Source & source, const Store & store, unsigned int format, StorePath && path);

void write(Sink & sink, const Store & store, unsigned int format, bool includePath = true) const;
};

typedef std::map<StorePath, ValidPathInfo> ValidPathInfos;
Expand Down
60 changes: 13 additions & 47 deletions src/libstore/remote-store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path,
if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path));
}
info = std::make_shared<ValidPathInfo>(
ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion), StorePath{path}));
StorePath{path},
WorkerProto::Serialise<UnkeyedValidPathInfo>::read(*this, *conn));
}
callback(std::move(info));
} catch (...) { callback.rethrow(); }
Expand Down Expand Up @@ -445,7 +446,7 @@ ref<const ValidPathInfo> RemoteStore::addCAToStore(
}

return make_ref<ValidPathInfo>(
ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion)));
WorkerProto::Serialise<ValidPathInfo>::read(*this, *conn));
}
else {
if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25");
Expand Down Expand Up @@ -570,7 +571,12 @@ void RemoteStore::addMultipleToStore(
auto source = sinkToSource([&](Sink & sink) {
sink << pathsToCopy.size();
for (auto & [pathInfo, pathSource] : pathsToCopy) {
pathInfo.write(sink, *this, 16);
WorkerProto::Serialise<ValidPathInfo>::write(*this,
WorkerProto::WriteConn {
.to = sink,
.version = 16,
},
pathInfo);
pathSource->drainInto(sink);
}
});
Expand Down Expand Up @@ -655,33 +661,6 @@ void RemoteStore::queryRealisationUncached(const DrvOutput & id,
} catch (...) { return callback.rethrow(); }
}

static void writeDerivedPaths(RemoteStore & store, RemoteStore::Connection & conn, const std::vector<DerivedPath> & reqs)
{
if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 30) {
WorkerProto::write(store, conn, reqs);
} else {
Strings ss;
for (auto & p : reqs) {
auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(p);
std::visit(overloaded {
[&](const StorePathWithOutputs & s) {
ss.push_back(s.to_string(store));
},
[&](const StorePath & drvPath) {
throw Error("trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file",
store.printStorePath(drvPath),
GET_PROTOCOL_MAJOR(conn.daemonVersion),
GET_PROTOCOL_MINOR(conn.daemonVersion));
},
[&](std::monostate) {
throw Error("wanted to build a derivation that is itself a build product, but the legacy 'ssh://' protocol doesn't support that. Try using 'ssh-ng://'");
},
}, sOrDrvPath);
}
conn.to << ss;
}
}

void RemoteStore::copyDrvsFromEvalStore(
const std::vector<DerivedPath> & paths,
std::shared_ptr<Store> evalStore)
Expand Down Expand Up @@ -711,7 +690,7 @@ void RemoteStore::buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMod
auto conn(getConnection());
conn->to << WorkerProto::Op::BuildPaths;
assert(GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13);
writeDerivedPaths(*this, *conn, drvPaths);
WorkerProto::write(*this, *conn, drvPaths);
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15)
conn->to << buildMode;
else
Expand All @@ -735,7 +714,7 @@ std::vector<KeyedBuildResult> RemoteStore::buildPathsWithResults(

if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 34) {
conn->to << WorkerProto::Op::BuildPathsWithResults;
writeDerivedPaths(*this, *conn, paths);
WorkerProto::write(*this, *conn, paths);
conn->to << buildMode;
conn.processStderr();
return WorkerProto::Serialise<std::vector<KeyedBuildResult>>::read(*this, *conn);
Expand Down Expand Up @@ -815,20 +794,7 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD
writeDerivation(conn->to, *this, drv);
conn->to << buildMode;
conn.processStderr();
BuildResult res;
res.status = (BuildResult::Status) readInt(conn->from);
conn->from >> res.errorMsg;
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) {
conn->from >> res.timesBuilt >> res.isNonDeterministic >> res.startTime >> res.stopTime;
}
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 28) {
auto builtOutputs = WorkerProto::Serialise<DrvOutputs>::read(*this, *conn);
for (auto && [output, realisation] : builtOutputs)
res.builtOutputs.insert_or_assign(
std::move(output.outputName),
std::move(realisation));
}
return res;
return WorkerProto::Serialise<BuildResult>::read(*this, *conn);
}


Expand Down Expand Up @@ -929,7 +895,7 @@ void RemoteStore::queryMissing(const std::vector<DerivedPath> & targets,
// to prevent a deadlock.
goto fallback;
conn->to << WorkerProto::Op::QueryMissing;
writeDerivedPaths(*this, *conn, targets);
WorkerProto::write(*this, *conn, targets);
conn.processStderr();
willBuild = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
willSubstitute = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
Expand Down
Loading