Skip to content

Commit 292bd39

Browse files
roberthroberth
committed
Remove setting from Input
This is more straightforward and not subject to undocumented memory safety restrictions. Also easier to test.
1 parent af0ac14 commit 292bd39

File tree

27 files changed

+181
-168
lines changed

27 files changed

+181
-168
lines changed

‎src/libcmd/common-eval-args.cc‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ EvalSettings evalSettings{
3333
// FIXME `parseFlakeRef` should take a `std::string_view`.
3434
auto flakeRef = parseFlakeRef(fetchSettings, std::string{rest}, {}, true, false);
3535
debug("fetching flake search path element '%s''", rest);
36-
auto [accessor, lockedRef] = flakeRef.resolve(state.store).lazyFetch(state.store);
36+
auto [accessor, lockedRef] =
37+
flakeRef.resolve(fetchSettings, state.store).lazyFetch(fetchSettings, state.store);
3738
auto storePath = nix::fetchToStore(
3839
state.fetchSettings,
3940
*state.store,
@@ -131,7 +132,7 @@ MixEvalArgs::MixEvalArgs()
131132
fetchers::Attrs extraAttrs;
132133
if (to.subdir != "")
133134
extraAttrs["dir"] = to.subdir;
134-
fetchers::overrideRegistry(from.input, to.input, extraAttrs);
135+
fetchers::overrideRegistry(fetchSettings, from.input, to.input, extraAttrs);
135136
}},
136137
.completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) {
137138
completeFlakeRef(completions, openStore(), prefix);
@@ -187,7 +188,7 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * bas
187188
else if (hasPrefix(s, "flake:")) {
188189
experimentalFeatureSettings.require(Xp::Flakes);
189190
auto flakeRef = parseFlakeRef(fetchSettings, std::string(s.substr(6)), {}, true, false);
190-
auto [accessor, lockedRef] = flakeRef.resolve(state.store).lazyFetch(state.store);
191+
auto [accessor, lockedRef] = flakeRef.resolve(fetchSettings, state.store).lazyFetch(fetchSettings, state.store);
191192
auto storePath = nix::fetchToStore(
192193
state.fetchSettings, *state.store, SourcePath(accessor), FetchMode::Copy, lockedRef.input.getName());
193194
state.allowPath(storePath);

‎src/libcmd/installables.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ MixFlakeOptions::MixFlakeOptions()
185185
}
186186

187187
overrideRegistry(
188+
fetchSettings,
188189
fetchers::Input::fromAttrs(fetchSettings, {{"type", "indirect"}, {"id", inputName}}),
189190
input3->lockedRef.input,
190191
extraAttrs);

‎src/libcmd/repl.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ void NixRepl::loadFlake(const std::string & flakeRefS)
738738
}
739739

740740
auto flakeRef = parseFlakeRef(fetchSettings, flakeRefS, cwd.string(), true);
741-
if (evalSettings.pureEval && !flakeRef.input.isLocked())
741+
if (evalSettings.pureEval && !flakeRef.input.isLocked(fetchSettings))
742742
throw Error("cannot use ':load-flake' on locked flake reference '%s' (use --impure to override)", flakeRefS);
743743

744744
Value v;

‎src/libexpr/primops/fetchMercurial.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value ** ar
8181
attrs.insert_or_assign("rev", rev->gitRev());
8282
auto input = fetchers::Input::fromAttrs(state.fetchSettings, std::move(attrs));
8383

84-
auto [storePath, input2] = input.fetchToStore(state.store);
84+
auto [storePath, input2] = input.fetchToStore(state.fetchSettings, state.store);
8585

8686
auto attrs2 = state.buildBindings(8);
8787
state.mkStorePathString(storePath, attrs2.alloc(state.s.outPath));

‎src/libexpr/primops/fetchTree.cc‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ struct FetchTreeParams
8282
static void fetchTree(
8383
EvalState & state, const PosIdx pos, Value ** args, Value & v, const FetchTreeParams & params = FetchTreeParams{})
8484
{
85-
fetchers::Input input{state.fetchSettings};
85+
fetchers::Input input{};
8686
NixStringContext context;
8787
std::optional<std::string> type;
8888
auto fetcher = params.isFetchGit ? "fetchGit" : "fetchTree";
@@ -194,9 +194,9 @@ static void fetchTree(
194194
}
195195

196196
if (!state.settings.pureEval && !input.isDirect() && experimentalFeatureSettings.isEnabled(Xp::Flakes))
197-
input = lookupInRegistries(state.store, input, fetchers::UseRegistries::Limited).first;
197+
input = lookupInRegistries(state.fetchSettings, state.store, input, fetchers::UseRegistries::Limited).first;
198198

199-
if (state.settings.pureEval && !input.isLocked()) {
199+
if (state.settings.pureEval && !input.isLocked(state.fetchSettings)) {
200200
if (input.getNarHash())
201201
warn(
202202
"Input '%s' is unlocked (e.g. lacks a Git revision) but is checked by NAR hash. "
@@ -219,7 +219,8 @@ static void fetchTree(
219219
throw Error("input '%s' is not allowed to use the '__final' attribute", input.to_string());
220220
}
221221

222-
auto cachedInput = state.inputCache->getAccessor(state.store, input, fetchers::UseRegistries::No);
222+
auto cachedInput =
223+
state.inputCache->getAccessor(state.fetchSettings, state.store, input, fetchers::UseRegistries::No);
223224

224225
auto storePath = state.mountInput(cachedInput.lockedInput, input, cachedInput.accessor);
225226

‎src/libfetchers-tests/git.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ TEST_F(GitTest, submodulePeriodSupport)
196196
{"ref", "main"},
197197
});
198198

199-
auto [accessor, i] = input.getAccessor(store);
199+
auto [accessor, i] = input.getAccessor(settings, store);
200200

201201
ASSERT_EQ(accessor->readFile(CanonPath("deps/sub/lib.txt")), "hello from submodule\n");
202202
}

‎src/libfetchers/fetchers.cc‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ Input Input::fromAttrs(const Settings & settings, Attrs && attrs)
8989
// but not all of them. Doing this is to support those other
9090
// operations which are supposed to be robust on
9191
// unknown/uninterpretable inputs.
92-
Input input{settings};
92+
Input input;
9393
input.attrs = attrs;
9494
fixupInput(input);
9595
return input;
@@ -159,9 +159,9 @@ bool Input::isDirect() const
159159
return !scheme || scheme->isDirect(*this);
160160
}
161161

162-
bool Input::isLocked() const
162+
bool Input::isLocked(const Settings & settings) const
163163
{
164-
return scheme && scheme->isLocked(*this);
164+
return scheme && scheme->isLocked(settings, *this);
165165
}
166166

167167
bool Input::isFinal() const
@@ -198,17 +198,17 @@ bool Input::contains(const Input & other) const
198198
}
199199

200200
// FIXME: remove
201-
std::pair<StorePath, Input> Input::fetchToStore(ref<Store> store) const
201+
std::pair<StorePath, Input> Input::fetchToStore(const Settings & settings, ref<Store> store) const
202202
{
203203
if (!scheme)
204204
throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs()));
205205

206206
auto [storePath, input] = [&]() -> std::pair<StorePath, Input> {
207207
try {
208-
auto [accessor, result] = getAccessorUnchecked(store);
208+
auto [accessor, result] = getAccessorUnchecked(settings, store);
209209

210210
auto storePath =
211-
nix::fetchToStore(*settings, *store, SourcePath(accessor), FetchMode::Copy, result.getName());
211+
nix::fetchToStore(settings, *store, SourcePath(accessor), FetchMode::Copy, result.getName());
212212

213213
auto narHash = store->queryPathInfo(storePath)->narHash;
214214
result.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
@@ -297,10 +297,10 @@ void Input::checkLocks(Input specified, Input & result)
297297
}
298298
}
299299

300-
std::pair<ref<SourceAccessor>, Input> Input::getAccessor(ref<Store> store) const
300+
std::pair<ref<SourceAccessor>, Input> Input::getAccessor(const Settings & settings, ref<Store> store) const
301301
{
302302
try {
303-
auto [accessor, result] = getAccessorUnchecked(store);
303+
auto [accessor, result] = getAccessorUnchecked(settings, store);
304304

305305
result.attrs.insert_or_assign("__final", Explicit<bool>(true));
306306

@@ -313,7 +313,7 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessor(ref<Store> store) const
313313
}
314314
}
315315

316-
std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> store) const
316+
std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(const Settings & settings, ref<Store> store) const
317317
{
318318
// FIXME: cache the accessor
319319

@@ -349,7 +349,7 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> sto
349349
if (accessor->fingerprint) {
350350
ContentAddressMethod method = ContentAddressMethod::Raw::NixArchive;
351351
auto cacheKey = makeFetchToStoreCacheKey(getName(), *accessor->fingerprint, method, "/");
352-
settings->getCache()->upsert(cacheKey, *store, {}, storePath);
352+
settings.getCache()->upsert(cacheKey, *store, {}, storePath);
353353
}
354354

355355
accessor->setPathDisplay("«" + to_string() + "»");
@@ -360,7 +360,7 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> sto
360360
}
361361
}
362362

363-
auto [accessor, result] = scheme->getAccessor(store, *this);
363+
auto [accessor, result] = scheme->getAccessor(settings, store, *this);
364364

365365
if (!accessor->fingerprint)
366366
accessor->fingerprint = result.getFingerprint(store);
@@ -377,10 +377,10 @@ Input Input::applyOverrides(std::optional<std::string> ref, std::optional<Hash>
377377
return scheme->applyOverrides(*this, ref, rev);
378378
}
379379

380-
void Input::clone(const Path & destDir) const
380+
void Input::clone(const Settings & settings, const Path & destDir) const
381381
{
382382
assert(scheme);
383-
scheme->clone(*this, destDir);
383+
scheme->clone(settings, *this, destDir);
384384
}
385385

386386
std::optional<std::filesystem::path> Input::getSourcePath() const
@@ -493,7 +493,7 @@ void InputScheme::putFile(
493493
throw Error("input '%s' does not support modifying file '%s'", input.to_string(), path);
494494
}
495495

496-
void InputScheme::clone(const Input & input, const Path & destDir) const
496+
void InputScheme::clone(const Settings & settings, const Input & input, const Path & destDir) const
497497
{
498498
throw Error("do not know how to clone input '%s'", input.to_string());
499499
}

‎src/libfetchers/git.cc‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ struct GitInputScheme : InputScheme
229229
if (auto ref = maybeGetStrAttr(attrs, "ref"); ref && !isLegalRefName(*ref))
230230
throw BadURL("invalid Git branch/tag name '%s'", *ref);
231231

232-
Input input{settings};
232+
Input input{};
233233
input.attrs = attrs;
234234
input.attrs["url"] = fixGitURL(getStrAttr(attrs, "url")).to_string();
235235
getShallowAttr(input);
@@ -278,7 +278,7 @@ struct GitInputScheme : InputScheme
278278
return res;
279279
}
280280

281-
void clone(const Input & input, const Path & destDir) const override
281+
void clone(const Settings & settings, const Input & input, const Path & destDir) const override
282282
{
283283
auto repoInfo = getRepoInfo(input);
284284

@@ -623,7 +623,7 @@ struct GitInputScheme : InputScheme
623623
}
624624

625625
std::pair<ref<SourceAccessor>, Input>
626-
getAccessorFromCommit(ref<Store> store, RepoInfo & repoInfo, Input && input) const
626+
getAccessorFromCommit(const Settings & settings, ref<Store> store, RepoInfo & repoInfo, Input && input) const
627627
{
628628
assert(!repoInfo.workdirInfo.isDirty);
629629

@@ -733,10 +733,10 @@ struct GitInputScheme : InputScheme
733733

734734
auto rev = *input.getRev();
735735

736-
input.attrs.insert_or_assign("lastModified", getLastModified(*input.settings, repoInfo, repoDir, rev));
736+
input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev));
737737

738738
if (!getShallowAttr(input))
739-
input.attrs.insert_or_assign("revCount", getRevCount(*input.settings, repoInfo, repoDir, rev));
739+
input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev));
740740

741741
printTalkative("using revision %s of repo '%s'", rev.gitRev(), repoInfo.locationToArg());
742742

@@ -779,8 +779,8 @@ struct GitInputScheme : InputScheme
779779
attrs.insert_or_assign("submodules", Explicit<bool>{true});
780780
attrs.insert_or_assign("lfs", Explicit<bool>{smudgeLfs});
781781
attrs.insert_or_assign("allRefs", Explicit<bool>{true});
782-
auto submoduleInput = fetchers::Input::fromAttrs(*input.settings, std::move(attrs));
783-
auto [submoduleAccessor, submoduleInput2] = submoduleInput.getAccessor(store);
782+
auto submoduleInput = fetchers::Input::fromAttrs(settings, std::move(attrs));
783+
auto [submoduleAccessor, submoduleInput2] = submoduleInput.getAccessor(settings, store);
784784
submoduleAccessor->setPathDisplay("«" + submoduleInput.to_string() + "»");
785785
mounts.insert_or_assign(submodule.path, submoduleAccessor);
786786
}
@@ -797,7 +797,7 @@ struct GitInputScheme : InputScheme
797797
}
798798

799799
std::pair<ref<SourceAccessor>, Input>
800-
getAccessorFromWorkdir(ref<Store> store, RepoInfo & repoInfo, Input && input) const
800+
getAccessorFromWorkdir(const Settings & settings, ref<Store> store, RepoInfo & repoInfo, Input && input) const
801801
{
802802
auto repoPath = repoInfo.getPath().value();
803803

@@ -829,8 +829,8 @@ struct GitInputScheme : InputScheme
829829
// TODO: fall back to getAccessorFromCommit-like fetch when submodules aren't checked out
830830
// attrs.insert_or_assign("allRefs", Explicit<bool>{ true });
831831

832-
auto submoduleInput = fetchers::Input::fromAttrs(*input.settings, std::move(attrs));
833-
auto [submoduleAccessor, submoduleInput2] = submoduleInput.getAccessor(store);
832+
auto submoduleInput = fetchers::Input::fromAttrs(settings, std::move(attrs));
833+
auto [submoduleAccessor, submoduleInput2] = submoduleInput.getAccessor(settings, store);
834834
submoduleAccessor->setPathDisplay("«" + submoduleInput.to_string() + "»");
835835

836836
/* If the submodule is dirty, mark this repo dirty as
@@ -857,12 +857,12 @@ struct GitInputScheme : InputScheme
857857
input.attrs.insert_or_assign("rev", rev.gitRev());
858858
if (!getShallowAttr(input)) {
859859
input.attrs.insert_or_assign(
860-
"revCount", rev == nullRev ? 0 : getRevCount(*input.settings, repoInfo, repoPath, rev));
860+
"revCount", rev == nullRev ? 0 : getRevCount(settings, repoInfo, repoPath, rev));
861861
}
862862

863863
verifyCommit(input, repo);
864864
} else {
865-
repoInfo.warnDirty(*input.settings);
865+
repoInfo.warnDirty(settings);
866866

867867
if (repoInfo.workdirInfo.headRev) {
868868
input.attrs.insert_or_assign("dirtyRev", repoInfo.workdirInfo.headRev->gitRev() + "-dirty");
@@ -874,14 +874,14 @@ struct GitInputScheme : InputScheme
874874

875875
input.attrs.insert_or_assign(
876876
"lastModified",
877-
repoInfo.workdirInfo.headRev
878-
? getLastModified(*input.settings, repoInfo, repoPath, *repoInfo.workdirInfo.headRev)
879-
: 0);
877+
repoInfo.workdirInfo.headRev ? getLastModified(settings, repoInfo, repoPath, *repoInfo.workdirInfo.headRev)
878+
: 0);
880879

881880
return {accessor, std::move(input)};
882881
}
883882

884-
std::pair<ref<SourceAccessor>, Input> getAccessor(ref<Store> store, const Input & _input) const override
883+
std::pair<ref<SourceAccessor>, Input>
884+
getAccessor(const Settings & settings, ref<Store> store, const Input & _input) const override
885885
{
886886
Input input(_input);
887887

@@ -897,8 +897,8 @@ struct GitInputScheme : InputScheme
897897
}
898898

899899
auto [accessor, final] = input.getRef() || input.getRev() || !repoInfo.getPath()
900-
? getAccessorFromCommit(store, repoInfo, std::move(input))
901-
: getAccessorFromWorkdir(store, repoInfo, std::move(input));
900+
? getAccessorFromCommit(settings, store, repoInfo, std::move(input))
901+
: getAccessorFromWorkdir(settings, store, repoInfo, std::move(input));
902902

903903
return {accessor, std::move(final)};
904904
}
@@ -934,7 +934,7 @@ struct GitInputScheme : InputScheme
934934
}
935935
}
936936

937-
bool isLocked(const Input & input) const override
937+
bool isLocked(const Settings & settings, const Input & input) const override
938938
{
939939
auto rev = input.getRev();
940940
return rev && rev != nullRev;

0 commit comments

Comments
 (0)