Skip to content

Commit 1662c6a

Browse files
committed
Move ValidPathInfo serialization code to worker-protocol.{cc.hh}
It does not belong with the data type itself. This also materializes the fact that `copyPath` does not do any version negotiation just just hard-codes "16". The non-standard interface of these serializers makes it harder to test, but this is fixed in the next commit which then adds those tests.
1 parent 7839bf1 commit 1662c6a

File tree

7 files changed

+76
-66
lines changed

7 files changed

+76
-66
lines changed

src/libstore/daemon.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
422422
}();
423423
logger->stopWork();
424424

425-
pathInfo->write(to, *store, GET_PROTOCOL_MINOR(clientVersion));
425+
WorkerProto::Serialise<ValidPathInfo>::write(*store, wconn, *pathInfo);
426426
} else {
427427
HashType hashAlgo;
428428
std::string baseName;
@@ -819,7 +819,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
819819
if (info) {
820820
if (GET_PROTOCOL_MINOR(clientVersion) >= 17)
821821
to << 1;
822-
info->write(to, *store, GET_PROTOCOL_MINOR(clientVersion), false);
822+
WorkerProto::Serialise<ValidPathInfo>::write(*store, wconn, *info, false);
823823
} else {
824824
assert(GET_PROTOCOL_MINOR(clientVersion) >= 17);
825825
to << 0;

src/libstore/path-info.cc

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
#include "path-info.hh"
2-
#include "worker-protocol.hh"
3-
#include "worker-protocol-impl.hh"
42
#include "store-api.hh"
53

64
namespace nix {
@@ -99,7 +97,6 @@ Strings ValidPathInfo::shortRefs() const
9997
return refs;
10098
}
10199

102-
103100
ValidPathInfo::ValidPathInfo(
104101
const Store & store,
105102
std::string_view name,
@@ -128,55 +125,4 @@ ValidPathInfo::ValidPathInfo(
128125
}, std::move(ca).raw);
129126
}
130127

131-
132-
ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format)
133-
{
134-
return read(source, store, format, store.parseStorePath(readString(source)));
135-
}
136-
137-
ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format, StorePath && path)
138-
{
139-
auto deriver = readString(source);
140-
auto narHash = Hash::parseAny(readString(source), htSHA256);
141-
ValidPathInfo info(path, narHash);
142-
if (deriver != "") info.deriver = store.parseStorePath(deriver);
143-
info.references = WorkerProto::Serialise<StorePathSet>::read(store,
144-
WorkerProto::ReadConn {
145-
.from = source,
146-
.version = format,
147-
});
148-
source >> info.registrationTime >> info.narSize;
149-
if (format >= 16) {
150-
source >> info.ultimate;
151-
info.sigs = readStrings<StringSet>(source);
152-
info.ca = ContentAddress::parseOpt(readString(source));
153-
}
154-
return info;
155-
}
156-
157-
158-
void ValidPathInfo::write(
159-
Sink & sink,
160-
const Store & store,
161-
unsigned int format,
162-
bool includePath) const
163-
{
164-
if (includePath)
165-
sink << store.printStorePath(path);
166-
sink << (deriver ? store.printStorePath(*deriver) : "")
167-
<< narHash.to_string(HashFormat::Base16, false);
168-
WorkerProto::write(store,
169-
WorkerProto::WriteConn {
170-
.to = sink,
171-
.version = format,
172-
},
173-
references);
174-
sink << registrationTime << narSize;
175-
if (format >= 16) {
176-
sink << ultimate
177-
<< sigs
178-
<< renderContentAddress(ca);
179-
}
180-
}
181-
182128
}

src/libstore/path-info.hh

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,6 @@ struct ValidPathInfo
121121
std::string_view name, ContentAddressWithReferences && ca, Hash narHash);
122122

123123
virtual ~ValidPathInfo() { }
124-
125-
static ValidPathInfo read(Source & source, const Store & store, unsigned int format);
126-
static ValidPathInfo read(Source & source, const Store & store, unsigned int format, StorePath && path);
127-
128-
void write(Sink & sink, const Store & store, unsigned int format, bool includePath = true) const;
129124
};
130125

131126
typedef std::map<StorePath, ValidPathInfo> ValidPathInfos;

src/libstore/remote-store.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path,
332332
if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path));
333333
}
334334
info = std::make_shared<ValidPathInfo>(
335-
ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion), StorePath{path}));
335+
WorkerProto::Serialise<ValidPathInfo>::read(*this, *conn, StorePath{path}));
336336
}
337337
callback(std::move(info));
338338
} catch (...) { callback.rethrow(); }
@@ -445,7 +445,7 @@ ref<const ValidPathInfo> RemoteStore::addCAToStore(
445445
}
446446

447447
return make_ref<ValidPathInfo>(
448-
ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion)));
448+
WorkerProto::Serialise<ValidPathInfo>::read(*this, *conn));
449449
}
450450
else {
451451
if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25");
@@ -570,7 +570,12 @@ void RemoteStore::addMultipleToStore(
570570
auto source = sinkToSource([&](Sink & sink) {
571571
sink << pathsToCopy.size();
572572
for (auto & [pathInfo, pathSource] : pathsToCopy) {
573-
pathInfo.write(sink, *this, 16);
573+
WorkerProto::Serialise<ValidPathInfo>::write(*this,
574+
WorkerProto::WriteConn {
575+
.to = sink,
576+
.version = 16,
577+
},
578+
pathInfo);
574579
pathSource->drainInto(sink);
575580
}
576581
});

src/libstore/store-api.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
#include "archive.hh"
1212
#include "callback.hh"
1313
#include "remote-store.hh"
14+
// FIXME this should not be here, see TODO below on
15+
// `addMultipleToStore`.
16+
#include "worker-protocol.hh"
1417

1518
#include <nlohmann/json.hpp>
1619
#include <regex>
@@ -357,7 +360,13 @@ void Store::addMultipleToStore(
357360
{
358361
auto expected = readNum<uint64_t>(source);
359362
for (uint64_t i = 0; i < expected; ++i) {
360-
auto info = ValidPathInfo::read(source, *this, 16);
363+
// FIXME we should not be using the worker protocol here, let
364+
// alone the worker protocol with a hard-coded version!
365+
auto info = WorkerProto::Serialise<ValidPathInfo>::read(*this,
366+
WorkerProto::ReadConn {
367+
.from = source,
368+
.version = 16,
369+
});
361370
info.ultimate = false;
362371
addToStore(info, source, repair, checkSigs);
363372
}

src/libstore/worker-protocol.cc

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
#include "worker-protocol.hh"
77
#include "worker-protocol-impl.hh"
88
#include "archive.hh"
9-
#include "derivations.hh"
9+
#include "path-info.hh"
1010

1111
#include <nlohmann/json.hpp>
1212

@@ -142,4 +142,47 @@ void WorkerProto::Serialise<BuildResult>::write(const Store & store, WorkerProto
142142
}
143143

144144

145+
ValidPathInfo WorkerProto::Serialise<ValidPathInfo>::read(const Store & store, ReadConn conn)
146+
{
147+
auto path = WorkerProto::Serialise<StorePath>::read(store, conn);
148+
return WorkerProto::Serialise<ValidPathInfo>::read(store, conn, std::move(path));
149+
}
150+
151+
ValidPathInfo WorkerProto::Serialise<ValidPathInfo>::read(const Store & store, ReadConn conn, StorePath && path)
152+
{
153+
auto deriver = readString(conn.from);
154+
auto narHash = Hash::parseAny(readString(conn.from), htSHA256);
155+
ValidPathInfo info(path, narHash);
156+
if (deriver != "") info.deriver = store.parseStorePath(deriver);
157+
info.references = WorkerProto::Serialise<StorePathSet>::read(store, conn);
158+
conn.from >> info.registrationTime >> info.narSize;
159+
if (GET_PROTOCOL_MINOR(conn.version) >= 16) {
160+
conn.from >> info.ultimate;
161+
info.sigs = readStrings<StringSet>(conn.from);
162+
info.ca = ContentAddress::parseOpt(readString(conn.from));
163+
}
164+
return info;
165+
}
166+
167+
void WorkerProto::Serialise<ValidPathInfo>::write(
168+
const Store & store,
169+
WriteConn conn,
170+
const ValidPathInfo & pathInfo,
171+
bool includePath)
172+
{
173+
if (includePath)
174+
conn.to << store.printStorePath(pathInfo.path);
175+
conn.to
176+
<< (pathInfo.deriver ? store.printStorePath(*pathInfo.deriver) : "")
177+
<< pathInfo.narHash.to_string(Base16, false);
178+
WorkerProto::write(store, conn, pathInfo.references);
179+
conn.to << pathInfo.registrationTime << pathInfo.narSize;
180+
if (GET_PROTOCOL_MINOR(conn.version) >= 16) {
181+
conn.to
182+
<< pathInfo.ultimate
183+
<< pathInfo.sigs
184+
<< renderContentAddress(pathInfo.ca);
185+
}
186+
}
187+
145188
}

src/libstore/worker-protocol.hh

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ struct Source;
3131
struct DerivedPath;
3232
struct BuildResult;
3333
struct KeyedBuildResult;
34+
struct ValidPathInfo;
3435
enum TrustedFlag : bool;
3536

3637

@@ -220,4 +221,15 @@ template<typename K, typename V>
220221
DECLARE_WORKER_SERIALISER(std::map<K COMMA_ V>);
221222
#undef COMMA_
222223

224+
/* These are a non-standard form for historical reasons. */
225+
226+
template<>
227+
struct WorkerProto::Serialise<ValidPathInfo>
228+
{
229+
static ValidPathInfo read(const Store & store, WorkerProto::ReadConn conn);
230+
static ValidPathInfo read(const Store & store, WorkerProto::ReadConn conn, StorePath && path);
231+
232+
static void write(const Store & store, WriteConn conn, const ValidPathInfo & pathInfo, bool includePath = true);
233+
};
234+
223235
}

0 commit comments

Comments
 (0)