Skip to content

Commit af05ce0

Browse files
edolstraedolstra
committed
queryMissing(): Return a struct
...instead of having a bunch of pass-by-reference arguments.
1 parent d4f67fd commit af05ce0

File tree

11 files changed

+59
-79
lines changed

11 files changed

+59
-79
lines changed

‎src/libmain/shared.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,8 @@ void printGCWarning()
4646

4747
void printMissing(ref<Store> store, const std::vector<DerivedPath> & paths, Verbosity lvl)
4848
{
49-
uint64_t downloadSize, narSize;
50-
StorePathSet willBuild, willSubstitute, unknown;
51-
store->queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize);
52-
printMissing(store, willBuild, willSubstitute, unknown, downloadSize, narSize, lvl);
49+
auto missing = store->queryMissing(paths);
50+
printMissing(store, missing.willBuild, missing.willSubstitute, missing.unknown, missing.downloadSize, missing.narSize, lvl);
5351
}
5452

5553

‎src/libstore/build/worker.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,7 @@ void Worker::run(const Goals & _topGoals)
324324
}
325325

326326
/* Call queryMissing() to efficiently query substitutes. */
327-
StorePathSet willBuild, willSubstitute, unknown;
328-
uint64_t downloadSize, narSize;
329-
store.queryMissing(topPaths, willBuild, willSubstitute, unknown, downloadSize, narSize);
327+
store.queryMissing(topPaths);
330328

331329
debug("entered goal loop");
332330

‎src/libstore/daemon.cc

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -948,14 +948,12 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
948948
case WorkerProto::Op::QueryMissing: {
949949
auto targets = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
950950
logger->startWork();
951-
StorePathSet willBuild, willSubstitute, unknown;
952-
uint64_t downloadSize, narSize;
953-
store->queryMissing(targets, willBuild, willSubstitute, unknown, downloadSize, narSize);
951+
auto missing = store->queryMissing(targets);
954952
logger->stopWork();
955-
WorkerProto::write(*store, wconn, willBuild);
956-
WorkerProto::write(*store, wconn, willSubstitute);
957-
WorkerProto::write(*store, wconn, unknown);
958-
conn.to << downloadSize << narSize;
953+
WorkerProto::write(*store, wconn, missing.willBuild);
954+
WorkerProto::write(*store, wconn, missing.willSubstitute);
955+
WorkerProto::write(*store, wconn, missing.unknown);
956+
conn.to << missing.downloadSize << missing.narSize;
959957
break;
960958
}
961959

‎src/libstore/include/nix/store/remote-store.hh

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,7 @@ struct RemoteStore :
149149

150150
void addSignatures(const StorePath & storePath, const StringSet & sigs) override;
151151

152-
void queryMissing(const std::vector<DerivedPath> & targets,
153-
StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
154-
uint64_t & downloadSize, uint64_t & narSize) override;
152+
MissingPaths queryMissing(const std::vector<DerivedPath> & targets) override;
155153

156154
void addBuildLog(const StorePath & drvPath, std::string_view log) override;
157155

‎src/libstore/include/nix/store/store-api.hh

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,18 @@ struct KeyedBuildResult;
7171

7272
typedef std::map<StorePath, std::optional<ContentAddress>> StorePathCAMap;
7373

74+
/**
75+
* Information about what paths will be built or substituted, returned
76+
* by Store::queryMissing().
77+
*/
78+
struct MissingPaths
79+
{
80+
StorePathSet willBuild;
81+
StorePathSet willSubstitute;
82+
StorePathSet unknown;
83+
uint64_t downloadSize{0};
84+
uint64_t narSize{0};
85+
};
7486

7587
/**
7688
* About the class hierarchy of the store types:
@@ -694,9 +706,7 @@ public:
694706
* derivations that will be built, and the set of output paths that
695707
* will be substituted.
696708
*/
697-
virtual void queryMissing(const std::vector<DerivedPath> & targets,
698-
StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
699-
uint64_t & downloadSize, uint64_t & narSize);
709+
virtual MissingPaths queryMissing(const std::vector<DerivedPath> & targets);
700710

701711
/**
702712
* Sort a set of paths topologically under the references

‎src/libstore/misc.cc

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -98,23 +98,17 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv)
9898
return nullptr;
9999
}
100100

101-
void Store::queryMissing(const std::vector<DerivedPath> & targets,
102-
StorePathSet & willBuild_, StorePathSet & willSubstitute_, StorePathSet & unknown_,
103-
uint64_t & downloadSize_, uint64_t & narSize_)
101+
MissingPaths Store::queryMissing(const std::vector<DerivedPath> & targets)
104102
{
105103
Activity act(*logger, lvlDebug, actUnknown, "querying info about missing paths");
106104

107-
downloadSize_ = narSize_ = 0;
108-
109105
// FIXME: make async.
110106
ThreadPool pool(fileTransferSettings.httpConnections);
111107

112108
struct State
113109
{
114110
std::unordered_set<std::string> done;
115-
StorePathSet & unknown, & willSubstitute, & willBuild;
116-
uint64_t & downloadSize;
117-
uint64_t & narSize;
111+
MissingPaths res;
118112
};
119113

120114
struct DrvState
@@ -125,7 +119,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
125119
DrvState(size_t left) : left(left) { }
126120
};
127121

128-
Sync<State> state_(State{{}, unknown_, willSubstitute_, willBuild_, downloadSize_, narSize_});
122+
Sync<State> state_;
129123

130124
std::function<void(DerivedPath)> doPath;
131125

@@ -143,7 +137,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
143137
auto mustBuildDrv = [&](const StorePath & drvPath, const Derivation & drv) {
144138
{
145139
auto state(state_.lock());
146-
state->willBuild.insert(drvPath);
140+
state->res.willBuild.insert(drvPath);
147141
}
148142

149143
for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) {
@@ -203,7 +197,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
203197
if (!isValidPath(drvPath)) {
204198
// FIXME: we could try to substitute the derivation.
205199
auto state(state_.lock());
206-
state->unknown.insert(drvPath);
200+
state->res.unknown.insert(drvPath);
207201
return;
208202
}
209203

@@ -282,7 +276,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
282276

283277
if (infos.empty()) {
284278
auto state(state_.lock());
285-
state->unknown.insert(bo.path);
279+
state->res.unknown.insert(bo.path);
286280
return;
287281
}
288282

@@ -291,9 +285,9 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
291285

292286
{
293287
auto state(state_.lock());
294-
state->willSubstitute.insert(bo.path);
295-
state->downloadSize += info->second.downloadSize;
296-
state->narSize += info->second.narSize;
288+
state->res.willSubstitute.insert(bo.path);
289+
state->res.downloadSize += info->second.downloadSize;
290+
state->res.narSize += info->second.narSize;
297291
}
298292

299293
for (auto & ref : info->second.references)
@@ -306,6 +300,8 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
306300
pool.enqueue(std::bind(doPath, path));
307301

308302
pool.process();
303+
304+
return std::move(state_.lock()->res);
309305
}
310306

311307

‎src/libstore/remote-store.cc

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -855,9 +855,7 @@ void RemoteStore::addSignatures(const StorePath & storePath, const StringSet & s
855855
}
856856

857857

858-
void RemoteStore::queryMissing(const std::vector<DerivedPath> & targets,
859-
StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown,
860-
uint64_t & downloadSize, uint64_t & narSize)
858+
MissingPaths RemoteStore::queryMissing(const std::vector<DerivedPath> & targets)
861859
{
862860
{
863861
auto conn(getConnection());
@@ -868,16 +866,16 @@ void RemoteStore::queryMissing(const std::vector<DerivedPath> & targets,
868866
conn->to << WorkerProto::Op::QueryMissing;
869867
WorkerProto::write(*this, *conn, targets);
870868
conn.processStderr();
871-
willBuild = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
872-
willSubstitute = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
873-
unknown = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
874-
conn->from >> downloadSize >> narSize;
875-
return;
869+
MissingPaths res;
870+
res.willBuild = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
871+
res.willSubstitute = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
872+
res.unknown = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
873+
conn->from >> res.downloadSize >> res.narSize;
874+
return res;
876875
}
877876

878877
fallback:
879-
return Store::queryMissing(targets, willBuild, willSubstitute,
880-
unknown, downloadSize, narSize);
878+
return Store::queryMissing(targets);
881879
}
882880

883881

‎src/libstore/restricted-store.cc

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,7 @@ struct RestrictedStore : public virtual IndirectRootStore, public virtual GcStor
143143
unsupported("addSignatures");
144144
}
145145

146-
void queryMissing(
147-
const std::vector<DerivedPath> & targets,
148-
StorePathSet & willBuild,
149-
StorePathSet & willSubstitute,
150-
StorePathSet & unknown,
151-
uint64_t & downloadSize,
152-
uint64_t & narSize) override;
146+
MissingPaths queryMissing(const std::vector<DerivedPath> & targets) override;
153147

154148
virtual std::optional<std::string> getBuildLogExact(const StorePath & path) override
155149
{
@@ -306,27 +300,27 @@ std::vector<KeyedBuildResult> RestrictedStore::buildPathsWithResults(
306300
return results;
307301
}
308302

309-
void RestrictedStore::queryMissing(
310-
const std::vector<DerivedPath> & targets,
311-
StorePathSet & willBuild,
312-
StorePathSet & willSubstitute,
313-
StorePathSet & unknown,
314-
uint64_t & downloadSize,
315-
uint64_t & narSize)
303+
MissingPaths RestrictedStore::queryMissing(const std::vector<DerivedPath> & targets)
316304
{
317305
/* This is slightly impure since it leaks information to the
318306
client about what paths will be built/substituted or are
319307
already present. Probably not a big deal. */
320308

321309
std::vector<DerivedPath> allowed;
310+
StorePathSet unknown;
322311
for (auto & req : targets) {
323312
if (goal.isAllowed(req))
324313
allowed.emplace_back(req);
325314
else
326315
unknown.insert(pathPartOfReq(req));
327316
}
328317

329-
next->queryMissing(allowed, willBuild, willSubstitute, unknown, downloadSize, narSize);
318+
auto res = next->queryMissing(allowed);
319+
320+
for (auto & p : unknown)
321+
res.unknown.insert(p);
322+
323+
return res;
330324
}
331325

332326
}

‎src/libstore/store-api.cc

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -790,15 +790,12 @@ void Store::substitutePaths(const StorePathSet & paths)
790790
for (auto & path : paths)
791791
if (!path.isDerivation())
792792
paths2.emplace_back(DerivedPath::Opaque{path});
793-
uint64_t downloadSize, narSize;
794-
StorePathSet willBuild, willSubstitute, unknown;
795-
queryMissing(paths2,
796-
willBuild, willSubstitute, unknown, downloadSize, narSize);
793+
auto missing = queryMissing(paths2);
797794

798-
if (!willSubstitute.empty())
795+
if (!missing.willSubstitute.empty())
799796
try {
800797
std::vector<DerivedPath> subs;
801-
for (auto & p : willSubstitute) subs.emplace_back(DerivedPath::Opaque{p});
798+
for (auto & p : missing.willSubstitute) subs.emplace_back(DerivedPath::Opaque{p});
802799
buildPaths(subs);
803800
} catch (Error & e) {
804801
logWarning(e.info());

‎src/nix-build/nix-build.cc

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -422,13 +422,10 @@ static void main_nix_build(int argc, char * * argv)
422422
auto buildPaths = [&](const std::vector<DerivedPath> & paths) {
423423
/* Note: we do this even when !printMissing to efficiently
424424
fetch binary cache data. */
425-
uint64_t downloadSize, narSize;
426-
StorePathSet willBuild, willSubstitute, unknown;
427-
store->queryMissing(paths,
428-
willBuild, willSubstitute, unknown, downloadSize, narSize);
425+
auto missing = store->queryMissing(paths);
429426

430427
if (settings.printMissing)
431-
printMissing(ref<Store>(store), willBuild, willSubstitute, unknown, downloadSize, narSize);
428+
printMissing(ref<Store>(store), missing.willBuild, missing.willSubstitute, missing.unknown, missing.downloadSize, missing.narSize);
432429

433430
if (!dryRun)
434431
store->buildPaths(paths, buildMode, evalStore);

0 commit comments

Comments
 (0)