Skip to content

Commit 4720853

Browse files
Ericson2314Ericson2314
committed
Make system-features a store setting
This seems more correct. It also means one can specify the features a store should support with --store and remote-store=..., which is useful. I use this to clean up the build remotes test.
1 parent 574bf60 commit 4720853

10 files changed

Lines changed: 68 additions & 38 deletions

File tree

‎src/build-remote/build-remote.cc‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ static AutoCloseFD openSlotLock(const Machine & m, uint64_t slot)
3838
return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri), slot), true);
3939
}
4040

41-
static bool allSupportedLocally(const std::set<std::string>& requiredFeatures) {
41+
static bool allSupportedLocally(Store & store, const std::set<std::string>& requiredFeatures) {
4242
for (auto & feature : requiredFeatures)
43-
if (!settings.systemFeatures.get().count(feature)) return false;
43+
if (!store.systemFeatures.get().count(feature)) return false;
4444
return true;
4545
}
4646

@@ -106,7 +106,7 @@ static int _main(int argc, char * * argv)
106106
auto canBuildLocally = amWilling
107107
&& ( neededSystem == settings.thisSystem
108108
|| settings.extraPlatforms.get().count(neededSystem) > 0)
109-
&& allSupportedLocally(requiredFeatures);
109+
&& allSupportedLocally(*store, requiredFeatures);
110110

111111
/* Error ignored here, will be caught later */
112112
mkdir(currentLoad.c_str(), 0777);
@@ -224,15 +224,7 @@ static int _main(int argc, char * * argv)
224224

225225
Activity act(*logger, lvlTalkative, actUnknown, fmt("connecting to '%s'", bestMachine->storeUri));
226226

227-
Store::Params storeParams;
228-
if (hasPrefix(bestMachine->storeUri, "ssh://")) {
229-
storeParams["max-connections"] = "1";
230-
storeParams["log-fd"] = "4";
231-
if (bestMachine->sshKey != "")
232-
storeParams["ssh-key"] = bestMachine->sshKey;
233-
}
234-
235-
sshStore = openStore(bestMachine->storeUri, storeParams);
227+
sshStore = bestMachine->openStore();
236228
sshStore->connect();
237229
storeUri = bestMachine->storeUri;
238230

‎src/libstore/build.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,7 +1475,7 @@ void DerivationGoal::tryToBuild()
14751475
/* Don't do a remote build if the derivation has the attribute
14761476
`preferLocalBuild' set. Also, check and repair modes are only
14771477
supported for local builds. */
1478-
bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally();
1478+
bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally(worker.store);
14791479

14801480
/* Is the build hook willing to accept this job? */
14811481
if (!buildLocally) {
@@ -1964,13 +1964,13 @@ void linkOrCopy(const Path & from, const Path & to)
19641964
void DerivationGoal::startBuilder()
19651965
{
19661966
/* Right platform? */
1967-
if (!parsedDrv->canBuildLocally())
1967+
if (!parsedDrv->canBuildLocally(worker.store))
19681968
throw Error("a '%s' with features {%s} is required to build '%s', but I am a '%s' with features {%s}",
19691969
drv->platform,
19701970
concatStringsSep(", ", parsedDrv->getRequiredSystemFeatures()),
19711971
worker.store.printStorePath(drvPath),
19721972
settings.thisSystem,
1973-
concatStringsSep<StringSet>(", ", settings.systemFeatures));
1973+
concatStringsSep<StringSet>(", ", worker.store.systemFeatures));
19741974

19751975
if (drv->isBuiltin())
19761976
preloadNSS();
@@ -3179,7 +3179,7 @@ void DerivationGoal::runChild()
31793179
createDirs(chrootRootDir + "/dev/shm");
31803180
createDirs(chrootRootDir + "/dev/pts");
31813181
ss.push_back("/dev/full");
3182-
if (settings.systemFeatures.get().count("kvm") && pathExists("/dev/kvm"))
3182+
if (worker.store.systemFeatures.get().count("kvm") && pathExists("/dev/kvm"))
31833183
ss.push_back("/dev/kvm");
31843184
ss.push_back("/dev/null");
31853185
ss.push_back("/dev/random");

‎src/libstore/machines.cc‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "machines.hh"
22
#include "util.hh"
33
#include "globals.hh"
4+
#include "store-api.hh"
45

56
#include <algorithm>
67

@@ -48,6 +49,29 @@ bool Machine::mandatoryMet(const std::set<string> & features) const {
4849
});
4950
}
5051

52+
ref<Store> Machine::openStore() const {
53+
Store::Params storeParams;
54+
if (hasPrefix(storeUri, "ssh://")) {
55+
storeParams["max-connections"] = "1";
56+
storeParams["log-fd"] = "4";
57+
if (sshKey != "")
58+
storeParams["ssh-key"] = sshKey;
59+
}
60+
{
61+
auto & fs = storeParams["system-features"];
62+
auto append = [&](auto feats) {
63+
for (auto & f : feats) {
64+
if (fs.size() > 0) fs += ' ';
65+
fs += f;
66+
}
67+
};
68+
append(supportedFeatures);
69+
append(mandatoryFeatures);
70+
}
71+
72+
return nix::openStore(storeUri, storeParams);
73+
}
74+
5175
void parseMachines(const std::string & s, Machines & machines)
5276
{
5377
for (auto line : tokenizeString<std::vector<string>>(s, "\n;")) {

‎src/libstore/machines.hh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace nix {
66

7+
class Store;
8+
79
struct Machine {
810

911
const string storeUri;
@@ -28,6 +30,8 @@ struct Machine {
2830
decltype(supportedFeatures) supportedFeatures,
2931
decltype(mandatoryFeatures) mandatoryFeatures,
3032
decltype(sshPublicHostKey) sshPublicHostKey);
33+
34+
ref<Store> openStore() const;
3135
};
3236

3337
typedef std::vector<Machine> Machines;

‎src/libstore/parsed-derivations.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,22 +94,22 @@ StringSet ParsedDerivation::getRequiredSystemFeatures() const
9494
return res;
9595
}
9696

97-
bool ParsedDerivation::canBuildLocally() const
97+
bool ParsedDerivation::canBuildLocally(Store & localStore) const
9898
{
9999
if (drv.platform != settings.thisSystem.get()
100100
&& !settings.extraPlatforms.get().count(drv.platform)
101101
&& !drv.isBuiltin())
102102
return false;
103103

104104
for (auto & feature : getRequiredSystemFeatures())
105-
if (!settings.systemFeatures.get().count(feature)) return false;
105+
if (!localStore.systemFeatures.get().count(feature)) return false;
106106

107107
return true;
108108
}
109109

110-
bool ParsedDerivation::willBuildLocally() const
110+
bool ParsedDerivation::willBuildLocally(Store & localStore) const
111111
{
112-
return getBoolAttr("preferLocalBuild") && canBuildLocally();
112+
return getBoolAttr("preferLocalBuild") && canBuildLocally(localStore);
113113
}
114114

115115
bool ParsedDerivation::substitutesAllowed() const

‎src/libstore/parsed-derivations.hh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ public:
2929

3030
StringSet getRequiredSystemFeatures() const;
3131

32-
bool canBuildLocally() const;
32+
bool canBuildLocally(Store & localStore) const;
3333

34-
bool willBuildLocally() const;
34+
bool willBuildLocally(Store & localStore) const;
3535

3636
bool substitutesAllowed() const;
3737
};

‎src/libstore/store-api.hh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,10 @@ public:
164164

165165
Setting<bool> wantMassQuery{this, false, "want-mass-query", "whether this substituter can be queried efficiently for path validity"};
166166

167+
Setting<StringSet> systemFeatures{this, settings.systemFeatures,
168+
"system-features",
169+
"Optional features that the system this store builds on implements (like \"kvm\")."};
170+
167171
protected:
168172

169173
struct PathInfoCacheValue {

‎tests/build-hook.nix‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ let
2323
shell = busybox;
2424
name = "build-remote-input-2";
2525
buildCommand = "echo BAR > $out";
26+
requiredSystemFeatures = ["bar"];
2627
};
2728

2829
in
@@ -34,6 +35,6 @@ in
3435
''
3536
read x < ${input1}
3637
read y < ${input2}
37-
echo $x$y > $out
38+
echo "$x $y" > $out
3839
'';
3940
}

‎tests/build-remote.sh‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,36 @@
11
source common.sh
22

3-
clearStore
4-
53
if ! canUseSandbox; then exit; fi
64
if ! [[ $busybox =~ busybox ]]; then exit; fi
75

8-
chmod -R u+w $TEST_ROOT/machine0 || true
9-
chmod -R u+w $TEST_ROOT/machine1 || true
10-
chmod -R u+w $TEST_ROOT/machine2 || true
11-
rm -rf $TEST_ROOT/machine0 $TEST_ROOT/machine1 $TEST_ROOT/machine2
12-
rm -f $TEST_ROOT/result
13-
146
unset NIX_STORE_DIR
157
unset NIX_STATE_DIR
168

9+
function join_by { local d=$1; shift; echo -n "$1"; shift; printf "%s" "${@/#/$d}"; }
10+
11+
builders=(
12+
# system-features will automatically be added to the outer URL, but not inner
13+
# remote-store URL.
14+
"ssh://localhost?remote-store=$TEST_ROOT/machine1?system-features=foo - - 1 1 foo"
15+
"$TEST_ROOT/machine2 - - 1 1 bar"
16+
)
17+
1718
# Note: ssh://localhost bypasses ssh, directly invoking nix-store as a
1819
# child process. This allows us to test LegacySSHStore::buildDerivation().
20+
# ssh-ng://... likewise allows us to test RemoteStore::buildDerivation().
1921
nix build -L -v -f build-hook.nix -o $TEST_ROOT/result --max-jobs 0 \
2022
--arg busybox $busybox \
2123
--store $TEST_ROOT/machine0 \
22-
--builders "ssh://localhost?remote-store=$TEST_ROOT/machine1; $TEST_ROOT/machine2 - - 1 1 foo" \
23-
--system-features foo
24+
--builders "$(join_by '; ' "${builders[@]}")"
2425

2526
outPath=$(readlink -f $TEST_ROOT/result)
2627

27-
cat $TEST_ROOT/machine0/$outPath | grep FOOBAR
28+
grep 'FOO BAR' $TEST_ROOT/machine0/$outPath
29+
30+
# Ensure that input1 was built on store1 due to the required feature.
31+
(! nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-1.sh)
32+
nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-1.sh
2833

29-
# Ensure that input1 was built on store2 due to the required feature.
30-
(! nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-1.sh)
31-
nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-1.sh
34+
# Ensure that input2 was built on store2 due to the required feature.
35+
(! nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-2.sh)
36+
nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-2.sh

‎tests/local.mk‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
nix_tests = \
2-
init.sh hash.sh lang.sh add.sh simple.sh dependencies.sh \
2+
hash.sh lang.sh add.sh simple.sh dependencies.sh \
33
config.sh \
44
gc.sh \
55
gc-concurrent.sh \

0 commit comments

Comments
 (0)