Skip to content
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.

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

Stabilise fetchTree #10068

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions doc/manual/rl-next/stabilize-fetchtree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
synopsis: Stabilize fetchTree
prs: 10068
---

The core of `fetchTree` is now stable.
This includes
- the `fetchTree` function itself
- all the existing fetchers, except `git` (still unstable because of some reproducibility concerns)
Copy link
Member

Choose a reason for hiding this comment

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

git is the only one we've actually improved during this time.

gitlab has known problems (wrong field names, wrong url syntax)

mercurial and all the other fetchers we haven't talked about are under-used and should be assumed to have repro problems

github is still git archive-based, not tree-based.

  • users who switch between git and github fetchers will be surprised to see their hash changed
  • git can't be reliably be used as a fallback implementation for github fetching because of export-subst (export-ignore might work)

tarball is probably the only fetcher without severe issues and also been reviewed properly. All the other ones should remain feature gated.

5 changes: 2 additions & 3 deletions src/libexpr/primops/fetchTree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ static void fetchTree(
}
input = fetchers::Input::fromAttrs(std::move(attrs));
} else {
if (!experimentalFeatureSettings.isEnabled(Xp::Flakes))
if (!experimentalFeatureSettings.isEnabled(Xp::FetchTreeUrls))
state.error<EvalError>(
"passing a string argument to 'fetchTree' requires the 'flakes' experimental feature"
"passing a string argument to 'fetchTree' requires the 'fetch-tree-urls' experimental feature"
).atPos(pos).debugThrow();
input = fetchers::Input::fromURL(url);
}
Expand Down Expand Up @@ -410,7 +410,6 @@ static RegisterPrimOp primop_fetchTree({
> ```
)",
.fun = prim_fetchTree,
.experimentalFeature = Xp::FetchTree,
});

static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v,
Expand Down
5 changes: 5 additions & 0 deletions src/libfetchers/git.cc
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,11 @@ struct GitInputScheme : InputScheme
return res;
}

std::optional<ExperimentalFeature> experimentalFeature() const override
{
return Xp::FetchTreeGit;
}

void clone(const Input & input, const Path & destDir) const override
{
auto repoInfo = getRepoInfo(input);
Expand Down
8 changes: 7 additions & 1 deletion src/libutil/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,14 @@ template<> std::set<ExperimentalFeature> BaseSetting<std::set<ExperimentalFeatur
for (auto & s : tokenizeString<StringSet>(str)) {
if (auto thisXpFeature = parseExperimentalFeature(s); thisXpFeature) {
res.insert(thisXpFeature.value());
if (thisXpFeature.value() == Xp::Flakes)
if (thisXpFeature.value() == Xp::Flakes) {
res.insert(Xp::FetchTree);
res.insert(Xp::FetchTreeGit);
res.insert(Xp::FetchTreeUrls);
} else if (thisXpFeature.value() == Xp::FetchTree) {
res.insert(Xp::FetchTreeGit);
res.insert(Xp::FetchTreeUrls);
}
} else
warn("unknown experimental feature '%s'", s);
}
Expand Down
24 changes: 17 additions & 7 deletions src/libutil/experimental-features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,23 @@ constexpr std::array<ExperimentalFeatureDetails, numXpFeatures> xpFeatureDetails
.tag = Xp::FetchTree,
.name = "fetch-tree",
.description = R"(
Enable the use of the [`fetchTree`](@docroot@/language/builtins.md#builtins-fetchTree) built-in function in the Nix language.

`fetchTree` exposes a generic interface for fetching remote file system trees from different types of remote sources.
The [`flakes`](#xp-feature-flakes) feature flag always enables `fetch-tree`.
This built-in was previously guarded by the `flakes` experimental feature because of that overlap.

Enabling just this feature serves as a "release candidate", allowing users to try it out in isolation.
Backwards-compatibility alias for
[fetch-tree-git](#xp-feature-fetch-tree-git) and
[fetch-tree-urls](#xp-feature-fetch-tree-urls).
)",
},
{
.tag = Xp::FetchTreeGit,
.name = "fetch-tree-git",
.description = R"(
Enable the use of the `git` [`fetchTree`](@docroot@/language/builtins.md#builtins-fetchTree) fetcher.
)",
},
{
.tag = Xp::FetchTreeUrls,
.name = "fetch-tree-urls",
.description = R"(
Enable the use of the [URL-like syntax](@docroot@/command-ref/new-cli/nix3-flake.html#url-like-syntax) in [`fetchTree`](@docroot@/language/builtins.md#builtins-fetchTree).
)",
},
{
Expand Down
2 changes: 2 additions & 0 deletions src/libutil/experimental-features.hh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ enum struct ExperimentalFeature
ImpureDerivations,
Flakes,
FetchTree,
FetchTreeGit,
FetchTreeUrls,
NixCommand,
GitHashing,
RecursiveNix,
Expand Down
6 changes: 6 additions & 0 deletions tests/functional/common/vars-and-functions.sh.in
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,12 @@ enableFeatures() {
sed -i 's/experimental-features .*/& '"$features"'/' "$NIX_CONF_DIR"/nix.conf
}

disableFeature() {
local feature="$1"
sed -i '/^\(extra-\)\?experimental-features/s/\b'"$feature"'\b//g' "$NIX_CONF_DIR"/nix.conf
sed -i '/^\(extra-\)\?experimental-features/s/\b'"$feature"'\b//g' "$NIX_CONF_DIR"/nix.conf.extra
}

set -x

onError() {
Expand Down
9 changes: 7 additions & 2 deletions tests/functional/config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,13 @@ exp_cores=$(nix config show | grep '^cores' | cut -d '=' -f 2 | xargs)
exp_features=$(nix config show | grep '^experimental-features' | cut -d '=' -f 2 | xargs)
[[ $prev != $exp_cores ]]
[[ $exp_cores == "4242" ]]
# flakes implies fetch-tree
[[ $exp_features == "fetch-tree flakes nix-command" ]]
# flakes implies fetch-tree and a bunch of other things
[[ $exp_features == "fetch-tree fetch-tree-git fetch-tree-urls flakes nix-command" ]]
[[
$(NIX_CONFIG="experimental-features = fetch-tree nix-command" nix config show | grep '^experimental-features' | cut -d '=' -f 2 | xargs) \
== \
"fetch-tree fetch-tree-git fetch-tree-urls nix-command"
]]

# Test that it's possible to retrieve a single setting's value
val=$(nix config show | grep '^warn-dirty' | cut -d '=' -f 2 | xargs)
Expand Down
3 changes: 3 additions & 0 deletions tests/functional/fetchTree-file.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ source common.sh

clearStore

disableFeature "flakes"

cd "$TEST_ROOT"

test_fetch_file () {
Expand All @@ -21,6 +23,7 @@ EOF
# Make sure that `http(s)` and `file` flake inputs are properly extracted when
# they should be, and treated as opaque files when they should be
test_file_flake_input () {
enableFeatures "flakes"
rm -fr "$TEST_ROOT/testFlake";
mkdir "$TEST_ROOT/testFlake";
pushd testFlake
Expand Down
Loading