Skip to content

Allow to substitute FODs with different storeDir #14113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Allow to substitute FODs with different storeDir #14113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/libstore/binary-cache-store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,12 @@ void BinaryCacheStore::init()
auto value = trim(line.substr(colon + 1, std::string::npos));
if (name == "StoreDir") {
if (value != storeDir)
throw Error(
warn(
"binary cache '%s' is for Nix stores with prefix '%s', not '%s'",
config.getHumanReadableURI(),
value,
storeDir);
config.storeDir = value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to have some systematic stuff for local vs remote setting, fyi. E.g. if you explicitly do ?store=..., you probably don't want the remote side to silently overwrite that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, great point. I didn't even think about passing ?store= manually! This diminishes the point of this PR, but I will try to amend it to introduce a way to "merge" local and remote settings.

} else if (name == "WantMassQuery") {
config.wantMassQuery.setDefault(value == "1");
} else if (name == "Priority") {
Expand Down
1 change: 1 addition & 0 deletions src/libstore/http-binary-cache-store.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class HttpBinaryCacheStore : public virtual BinaryCacheStore
if (auto cacheInfo = diskCache->upToDateCacheExists(config->cacheUri.to_string())) {
config->wantMassQuery.setDefault(cacheInfo->wantMassQuery);
config->priority.setDefault(cacheInfo->priority);
config->storeDir = cacheInfo->storeDir;
} else {
try {
BinaryCacheStore::init();
Expand Down
1 change: 1 addition & 0 deletions src/libstore/include/nix/store/nar-info-disk-cache.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public:
int id;
bool wantMassQuery;
int priority;
Path storeDir;
};

virtual std::optional<CacheInfo> upToDateCacheExists(const std::string & uri) = 0;
Expand Down
5 changes: 3 additions & 2 deletions src/libstore/include/nix/store/store-api.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,15 @@ private:

public:

const PathSetting storeDir_{
PathSetting storeDir_{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the first impression this seems like breaking a very important invariant of the stores (store directory is an inherent property of a store). Making storeDir mutable is not the right approach here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to think of an alternative solution but fell short.

It would of course be possible to move the whole nix-cache-info fetching into the constructor, but that seems even worse.

Do you have anything in mind?

this,
getDefaultNixStoreDir(),
"store",
R"(
Logical location of the Nix store, usually
`/nix/store`. Note that you can only copy store paths
between stores if they have the same `store` setting.
between stores if they have the same `store` setting,
with the exception of fixed-output derivation outputs.
)"};
};

Expand Down
2 changes: 1 addition & 1 deletion src/libstore/include/nix/store/store-dir-config.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ MakeError(BadStorePathName, BadStorePath);
*/
struct StoreDirConfig
{
const Path & storeDir;
Path & storeDir;

// pure methods

Expand Down
6 changes: 5 additions & 1 deletion src/libstore/nar-info-disk-cache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,11 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
auto cache(queryCacheRaw(*state, uri));
if (!cache)
return std::nullopt;
return CacheInfo{.id = cache->id, .wantMassQuery = cache->wantMassQuery, .priority = cache->priority};
return CacheInfo{
.id = cache->id,
.wantMassQuery = cache->wantMassQuery,
.priority = cache->priority,
.storeDir = cache->storeDir};
});
}

Expand Down
65 changes: 65 additions & 0 deletions tests/functional/binary-cache.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,3 +306,68 @@ nix-store --delete "$outPath" "$docPath"
# -vvv is the level that logs during the loop
timeout 60 nix-build --no-out-link -E "$expr" --option substituters "file://$cacheDir" \
--option trusted-binary-caches "file://$cacheDir" --no-require-sigs

# Check that we can substitute FODs from a substituter with different StoreDir

# preserve quotes variables in the single-quoted string
# shellcheck disable=SC2016
expr='
with import '"${config_nix}"';
mkDerivation {
name = "fod";
buildCommand = "mkdir $out; echo Foo > $out/foo";
outputs = [ "out" ];
outputHashMode = "recursive";
outputHashAlgo = "sha256";
outputHash = "0sfsas2ngwycqq0f712dbbjzcva2ld9yb39hkvfhlv11lvbd78wp";
}
'
outPath=$(nix-build --no-out-link -E "$expr")
nix copy --to "file://$cacheDir" "$outPath"

other_store="$TEST_ROOT/other_store"
rm -rf "$other_store"
otherOutPath=$(NIX_STORE_DIR="$other_store/nix/store" NIX_STORE="$other_store/nix/store" nix-build --no-out-link -E "$expr" \
--option max-jobs 0 \
--option store "$other_store" \
--option substituters "file://$cacheDir" \
--option trusted-binary-caches "file://$cacheDir" \
--no-require-sigs)

if ! [[ "$(basename "$otherOutPath")" != "$(basename "$outPath")" ]]; then
fail "Store hashes for $otherOutPath and $outPath are the same despite different store dirs"
fi
if ! [[ -d "$otherOutPath" ]]; then
fail "Could not substitute FOD path from a substituter with a different store dir: $otherOutPath does not exist"
fi
if ! diff -r "$otherOutPath" "$outPath"; then
fail "The contents of $otherOutPath and $outPath are different even though they should be the same"
fi

# Check that substituting a non-FOD from a substituter with different StoreDir fails

# preserve quotes variables in the single-quoted string
# shellcheck disable=SC2016
expr='
with import '"${config_nix}"';
mkDerivation {
name = "fod";
buildCommand = "mkdir $out; echo Foo > $out/foo";
outputs = [ "out" ];
}
'

outPath=$(nix-build --no-out-link -E "$expr")
nix copy --to "file://$cacheDir" "$outPath"

other_store="$TEST_ROOT/other_store"

if NIX_STORE_DIR="$other_store/nix/store" NIX_STORE="$other_store/nix/store" nix-build --no-out-link -E "$expr" \
--option max-jobs 0 \
--option store "$other_store" \
--option substituters "file://$cacheDir" \
--option trusted-binary-caches "file://$cacheDir" \
--no-require-sigs;
then
fail "Substituting a non-FOD path from a substituter with a different StoreDir should fail"
fi
Loading