Skip to content

Commit ac860fd

Browse files
Ericson2314Ericson2314
committed
Factor out UnkeyedValidPathInfo and test
This makes the path info serialisers ideomatic again, which allows me to test them.
1 parent 1662c6a commit ac860fd

File tree

10 files changed

+218
-34
lines changed

10 files changed

+218
-34
lines changed

‎src/libstore/daemon.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -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-
WorkerProto::Serialise<ValidPathInfo>::write(*store, wconn, *info, false);
822+
WorkerProto::write(*store, wconn, static_cast<const UnkeyedValidPathInfo &>(*info));
823823
} else {
824824
assert(GET_PROTOCOL_MINOR(clientVersion) >= 17);
825825
to << 0;

‎src/libstore/path-info.cc‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,25 @@
33

44
namespace nix {
55

6+
GENERATE_CMP_EXT(
7+
,
8+
UnkeyedValidPathInfo,
9+
me->deriver,
10+
me->narHash,
11+
me->references,
12+
me->registrationTime,
13+
me->narSize,
14+
//me->id,
15+
me->ultimate,
16+
me->sigs,
17+
me->ca);
18+
19+
GENERATE_CMP_EXT(
20+
,
21+
ValidPathInfo,
22+
me->path,
23+
static_cast<const UnkeyedValidPathInfo &>(*me));
24+
625
std::string ValidPathInfo::fingerprint(const Store & store) const
726
{
827
if (narSize == 0)
@@ -102,8 +121,8 @@ ValidPathInfo::ValidPathInfo(
102121
std::string_view name,
103122
ContentAddressWithReferences && ca,
104123
Hash narHash)
105-
: path(store.makeFixedOutputPathFromCA(name, ca))
106-
, narHash(narHash)
124+
: UnkeyedValidPathInfo(narHash)
125+
, path(store.makeFixedOutputPathFromCA(name, ca))
107126
{
108127
std::visit(overloaded {
109128
[this](TextInfo && ti) {

‎src/libstore/path-info.hh‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ struct SubstitutablePathInfo
3232
typedef std::map<StorePath, SubstitutablePathInfo> SubstitutablePathInfos;
3333

3434

35-
struct ValidPathInfo
35+
struct UnkeyedValidPathInfo
3636
{
37-
StorePath path;
3837
std::optional<StorePath> deriver;
3938
/**
4039
* \todo document this
@@ -72,6 +71,20 @@ struct ValidPathInfo
7271
*/
7372
std::optional<ContentAddress> ca;
7473

74+
UnkeyedValidPathInfo(const UnkeyedValidPathInfo & other) = default;
75+
76+
UnkeyedValidPathInfo(Hash narHash) : narHash(narHash) { };
77+
78+
DECLARE_CMP(UnkeyedValidPathInfo);
79+
80+
virtual ~UnkeyedValidPathInfo() { }
81+
};
82+
83+
struct ValidPathInfo : UnkeyedValidPathInfo {
84+
StorePath path;
85+
86+
DECLARE_CMP(ValidPathInfo);
87+
7588
/**
7689
* Return a fingerprint of the store path to be used in binary
7790
* cache signatures. It contains the store path, the base-32
@@ -84,11 +97,11 @@ struct ValidPathInfo
8497

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

87-
/**
88-
* @return The `ContentAddressWithReferences` that determines the
89-
* store path for a content-addressed store object, `std::nullopt`
90-
* for an input-addressed store object.
91-
*/
100+
/**
101+
* @return The `ContentAddressWithReferences` that determines the
102+
* store path for a content-addressed store object, `std::nullopt`
103+
* for an input-addressed store object.
104+
*/
92105
std::optional<ContentAddressWithReferences> contentAddressWithReferences() const;
93106

94107
/**
@@ -114,8 +127,8 @@ struct ValidPathInfo
114127

115128
ValidPathInfo(const ValidPathInfo & other) = default;
116129

117-
ValidPathInfo(StorePath && path, Hash narHash) : path(std::move(path)), narHash(narHash) { };
118-
ValidPathInfo(const StorePath & path, Hash narHash) : path(path), narHash(narHash) { };
130+
ValidPathInfo(StorePath && path, UnkeyedValidPathInfo info) : UnkeyedValidPathInfo(info), path(std::move(path)) { };
131+
ValidPathInfo(const StorePath & path, UnkeyedValidPathInfo info) : UnkeyedValidPathInfo(info), path(path) { };
119132

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

‎src/libstore/remote-store.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,8 @@ 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-
WorkerProto::Serialise<ValidPathInfo>::read(*this, *conn, StorePath{path}));
335+
StorePath{path},
336+
WorkerProto::Serialise<UnkeyedValidPathInfo>::read(*this, *conn));
336337
}
337338
callback(std::move(info));
338339
} catch (...) { callback.rethrow(); }

‎src/libstore/tests/worker-protocol.cc‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,159 @@ VERSIONED_CHARACTERIZATION_TEST(
329329
t;
330330
}))
331331

332+
VERSIONED_CHARACTERIZATION_TEST(
333+
WorkerProtoTest,
334+
unkeyedValidPathInfo_1_15,
335+
"unkeyed-valid-path-info-1.15",
336+
1 << 8 | 15,
337+
(std::tuple<UnkeyedValidPathInfo, UnkeyedValidPathInfo> {
338+
({
339+
UnkeyedValidPathInfo info {
340+
Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
341+
};
342+
info.registrationTime = 23423;
343+
info.narSize = 34878;
344+
info;
345+
}),
346+
({
347+
UnkeyedValidPathInfo info {
348+
Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
349+
};
350+
info.deriver = StorePath {
351+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
352+
};
353+
info.references = {
354+
StorePath {
355+
"g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo.drv",
356+
},
357+
};
358+
info.registrationTime = 23423;
359+
info.narSize = 34878;
360+
info;
361+
}),
362+
}))
363+
364+
VERSIONED_CHARACTERIZATION_TEST(
365+
WorkerProtoTest,
366+
validPathInfo_1_15,
367+
"valid-path-info-1.15",
368+
1 << 8 | 15,
369+
(std::tuple<ValidPathInfo, ValidPathInfo> {
370+
({
371+
ValidPathInfo info {
372+
StorePath {
373+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
374+
},
375+
UnkeyedValidPathInfo {
376+
Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
377+
},
378+
};
379+
info.registrationTime = 23423;
380+
info.narSize = 34878;
381+
info;
382+
}),
383+
({
384+
ValidPathInfo info {
385+
StorePath {
386+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
387+
},
388+
UnkeyedValidPathInfo {
389+
Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
390+
},
391+
};
392+
info.deriver = StorePath {
393+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
394+
};
395+
info.references = {
396+
// other reference
397+
StorePath {
398+
"g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo",
399+
},
400+
// self reference
401+
StorePath {
402+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
403+
},
404+
};
405+
info.registrationTime = 23423;
406+
info.narSize = 34878;
407+
info;
408+
}),
409+
}))
410+
411+
VERSIONED_CHARACTERIZATION_TEST(
412+
WorkerProtoTest,
413+
validPathInfo_1_16,
414+
"valid-path-info-1.16",
415+
1 << 8 | 16,
416+
(std::tuple<ValidPathInfo, ValidPathInfo, ValidPathInfo> {
417+
({
418+
ValidPathInfo info {
419+
StorePath {
420+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
421+
},
422+
UnkeyedValidPathInfo {
423+
Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
424+
},
425+
};
426+
info.registrationTime = 23423;
427+
info.narSize = 34878;
428+
info.ultimate = true;
429+
info;
430+
}),
431+
({
432+
ValidPathInfo info {
433+
StorePath {
434+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
435+
},
436+
UnkeyedValidPathInfo {
437+
Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
438+
},
439+
};
440+
info.deriver = StorePath {
441+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
442+
};
443+
info.references = {
444+
// other reference
445+
StorePath {
446+
"g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo",
447+
},
448+
// self reference
449+
StorePath {
450+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
451+
},
452+
};
453+
info.registrationTime = 23423;
454+
info.narSize = 34878;
455+
info.sigs = {
456+
"fake-sig-1",
457+
"fake-sig-2",
458+
},
459+
info;
460+
}),
461+
({
462+
ValidPathInfo info {
463+
*LibStoreTest::store,
464+
"foo",
465+
FixedOutputInfo {
466+
.method = FileIngestionMethod::Recursive,
467+
.hash = hashString(HashType::htSHA256, "(...)"),
468+
.references = {
469+
.others = {
470+
StorePath {
471+
"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
472+
},
473+
},
474+
.self = true,
475+
},
476+
},
477+
Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
478+
};
479+
info.registrationTime = 23423;
480+
info.narSize = 34878;
481+
info;
482+
}),
483+
}))
484+
332485
VERSIONED_CHARACTERIZATION_TEST(
333486
WorkerProtoTest,
334487
optionalTrustedFlag,

‎src/libstore/worker-protocol.cc‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,24 @@ void WorkerProto::Serialise<BuildResult>::write(const Store & store, WorkerProto
145145
ValidPathInfo WorkerProto::Serialise<ValidPathInfo>::read(const Store & store, ReadConn conn)
146146
{
147147
auto path = WorkerProto::Serialise<StorePath>::read(store, conn);
148-
return WorkerProto::Serialise<ValidPathInfo>::read(store, conn, std::move(path));
148+
return ValidPathInfo {
149+
std::move(path),
150+
WorkerProto::Serialise<UnkeyedValidPathInfo>::read(store, conn),
151+
};
149152
}
150153

151-
ValidPathInfo WorkerProto::Serialise<ValidPathInfo>::read(const Store & store, ReadConn conn, StorePath && path)
154+
void WorkerProto::Serialise<ValidPathInfo>::write(const Store & store, WriteConn conn, const ValidPathInfo & pathInfo)
155+
{
156+
WorkerProto::write(store, conn, pathInfo.path);
157+
WorkerProto::write(store, conn, static_cast<const UnkeyedValidPathInfo &>(pathInfo));
158+
}
159+
160+
161+
UnkeyedValidPathInfo WorkerProto::Serialise<UnkeyedValidPathInfo>::read(const Store & store, ReadConn conn)
152162
{
153163
auto deriver = readString(conn.from);
154164
auto narHash = Hash::parseAny(readString(conn.from), htSHA256);
155-
ValidPathInfo info(path, narHash);
165+
UnkeyedValidPathInfo info(narHash);
156166
if (deriver != "") info.deriver = store.parseStorePath(deriver);
157167
info.references = WorkerProto::Serialise<StorePathSet>::read(store, conn);
158168
conn.from >> info.registrationTime >> info.narSize;
@@ -164,14 +174,8 @@ ValidPathInfo WorkerProto::Serialise<ValidPathInfo>::read(const Store & store, R
164174
return info;
165175
}
166176

167-
void WorkerProto::Serialise<ValidPathInfo>::write(
168-
const Store & store,
169-
WriteConn conn,
170-
const ValidPathInfo & pathInfo,
171-
bool includePath)
177+
void WorkerProto::Serialise<UnkeyedValidPathInfo>::write(const Store & store, WriteConn conn, const UnkeyedValidPathInfo & pathInfo)
172178
{
173-
if (includePath)
174-
conn.to << store.printStorePath(pathInfo.path);
175179
conn.to
176180
<< (pathInfo.deriver ? store.printStorePath(*pathInfo.deriver) : "")
177181
<< pathInfo.narHash.to_string(Base16, false);

‎src/libstore/worker-protocol.hh‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ struct DerivedPath;
3232
struct BuildResult;
3333
struct KeyedBuildResult;
3434
struct ValidPathInfo;
35+
struct UnkeyedValidPathInfo;
3536
enum TrustedFlag : bool;
3637

3738

@@ -207,6 +208,10 @@ DECLARE_WORKER_SERIALISER(BuildResult);
207208
template<>
208209
DECLARE_WORKER_SERIALISER(KeyedBuildResult);
209210
template<>
211+
DECLARE_WORKER_SERIALISER(ValidPathInfo);
212+
template<>
213+
DECLARE_WORKER_SERIALISER(UnkeyedValidPathInfo);
214+
template<>
210215
DECLARE_WORKER_SERIALISER(std::optional<TrustedFlag>);
211216

212217
template<typename T>
@@ -221,15 +226,4 @@ template<typename K, typename V>
221226
DECLARE_WORKER_SERIALISER(std::map<K COMMA_ V>);
222227
#undef COMMA_
223228

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-
235229
}

‎unit-test-data/libstore/worker-protocol/unkeyed-valid-path-info-1.15.bin‎

328 Bytes
Binary file not shown.

‎unit-test-data/libstore/worker-protocol/valid-path-info-1.15.bin‎

488 Bytes
Binary file not shown.

‎unit-test-data/libstore/worker-protocol/valid-path-info-1.16.bin‎

952 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)