Skip to content

Commit d5d7ca0

Browse files
edolstraedolstra
authored
Merge pull request #14138 from lovesegfault/nix-fix-4313
fix(libfetchers): substitute fetchTarball and fetchurl
2 parents 76ac375 + 1e92b61 commit d5d7ca0

3 files changed

Lines changed: 189 additions & 3 deletions

File tree

‎src/libexpr/primops/fetchTree.cc‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,14 +561,22 @@ static void fetch(
561561
.hash = *expectedHash,
562562
.references = {}});
563563

564-
if (state.store->isValidPath(expectedPath)) {
564+
// Try to get the path from the local store or substituters
565+
try {
566+
state.store->ensurePath(expectedPath);
567+
debug("using substituted/cached path '%s' for '%s'", state.store->printStorePath(expectedPath), *url);
565568
state.allowAndSetStorePathString(expectedPath, v);
566569
return;
570+
} catch (Error & e) {
571+
debug(
572+
"substitution of '%s' failed, will try to download: %s",
573+
state.store->printStorePath(expectedPath),
574+
e.what());
575+
// Fall through to download
567576
}
568577
}
569578

570-
// TODO: fetching may fail, yet the path may be substitutable.
571-
// https://github.com/NixOS/nix/issues/4313
579+
// Download the file/tarball if substitution failed or no hash was provided
572580
auto storePath = unpack ? fetchToStore(
573581
state.fetchSettings,
574582
*state.store,

‎tests/nixos/default.nix‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,5 +207,7 @@ in
207207

208208
fetchurl = runNixOSTest ./fetchurl.nix;
209209

210+
fetchersSubstitute = runNixOSTest ./fetchers-substitute.nix;
211+
210212
chrootStore = runNixOSTest ./chroot-store.nix;
211213
}

‎tests/nixos/fetchers-substitute.nix‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
{
2+
name = "fetchers-substitute";
3+
4+
nodes.substituter =
5+
{ pkgs, ... }:
6+
{
7+
virtualisation.writableStore = true;
8+
9+
nix.settings.extra-experimental-features = [
10+
"nix-command"
11+
"fetch-tree"
12+
];
13+
14+
networking.firewall.allowedTCPPorts = [ 5000 ];
15+
16+
services.nix-serve = {
17+
enable = true;
18+
secretKeyFile =
19+
let
20+
key = pkgs.writeTextFile {
21+
name = "secret-key";
22+
text = ''
23+
substituter:SerxxAca5NEsYY0DwVo+subokk+OoHcD9m6JwuctzHgSQVfGHe6nCc+NReDjV3QdFYPMGix4FMg0+K/TM1B3aA==
24+
'';
25+
};
26+
in
27+
"${key}";
28+
};
29+
};
30+
31+
nodes.importer =
32+
{ lib, ... }:
33+
{
34+
virtualisation.writableStore = true;
35+
36+
nix.settings = {
37+
extra-experimental-features = [
38+
"nix-command"
39+
"fetch-tree"
40+
];
41+
substituters = lib.mkForce [ "http://substituter:5000" ];
42+
trusted-public-keys = lib.mkForce [ "substituter:EkFXxh3upwnPjUXg41d0HRWDzBoseBTINPiv0zNQd2g=" ];
43+
};
44+
};
45+
46+
testScript =
47+
{ nodes }: # python
48+
''
49+
import json
50+
51+
start_all()
52+
53+
substituter.wait_for_unit("multi-user.target")
54+
55+
##########################################
56+
# Test 1: builtins.fetchurl with substitution
57+
##########################################
58+
59+
missing_file = "/only-on-substituter.txt"
60+
61+
substituter.succeed(f"echo 'this should only exist on the substituter' > {missing_file}")
62+
63+
file_hash = substituter.succeed(f"nix hash file {missing_file}").strip()
64+
65+
file_store_path_json = substituter.succeed(f"""
66+
nix-instantiate --eval --json --read-write-mode --expr '
67+
builtins.fetchurl {{
68+
url = "file://{missing_file}";
69+
sha256 = "{file_hash}";
70+
}}
71+
'
72+
""")
73+
74+
file_store_path = json.loads(file_store_path_json)
75+
76+
substituter.succeed(f"nix store sign --key-file ${nodes.substituter.services.nix-serve.secretKeyFile} {file_store_path}")
77+
78+
importer.wait_for_unit("multi-user.target")
79+
80+
print("Testing fetchurl with substitution...")
81+
importer.succeed(f"""
82+
nix-instantiate -vvvvv --eval --json --read-write-mode --expr '
83+
builtins.fetchurl {{
84+
url = "file://{missing_file}";
85+
sha256 = "{file_hash}";
86+
}}
87+
'
88+
""")
89+
print("✓ fetchurl substitution works!")
90+
91+
##########################################
92+
# Test 2: builtins.fetchTarball with substitution
93+
##########################################
94+
95+
missing_tarball = "/only-on-substituter.tar.gz"
96+
97+
# Create a directory with some content
98+
substituter.succeed("""
99+
mkdir -p /tmp/test-tarball
100+
echo 'Hello from tarball!' > /tmp/test-tarball/hello.txt
101+
echo 'Another file' > /tmp/test-tarball/file2.txt
102+
""")
103+
104+
# Create a tarball
105+
substituter.succeed(f"tar czf {missing_tarball} -C /tmp test-tarball")
106+
107+
# For fetchTarball, we need to first fetch it without hash to get the store path,
108+
# then compute the NAR hash of that path
109+
tarball_store_path_json = substituter.succeed(f"""
110+
nix-instantiate --eval --json --read-write-mode --expr '
111+
builtins.fetchTarball {{
112+
url = "file://{missing_tarball}";
113+
}}
114+
'
115+
""")
116+
117+
tarball_store_path = json.loads(tarball_store_path_json)
118+
119+
# Get the NAR hash of the unpacked tarball in SRI format
120+
path_info_json = substituter.succeed(f"nix path-info --json {tarball_store_path}").strip()
121+
path_info_dict = json.loads(path_info_json)
122+
# nix path-info returns a dict with store paths as keys
123+
tarball_hash_sri = path_info_dict[tarball_store_path]["narHash"]
124+
print(f"Tarball NAR hash (SRI): {tarball_hash_sri}")
125+
126+
# Also get the old format hash for fetchTarball (which uses sha256 parameter)
127+
tarball_hash = substituter.succeed(f"nix-store --query --hash {tarball_store_path}").strip()
128+
129+
# Sign the tarball's store path
130+
substituter.succeed(f"nix store sign --recursive --key-file ${nodes.substituter.services.nix-serve.secretKeyFile} {tarball_store_path}")
131+
132+
# Now try to fetch the same tarball on the importer
133+
# The file doesn't exist locally, so it should be substituted
134+
print("Testing fetchTarball with substitution...")
135+
result = importer.succeed(f"""
136+
nix-instantiate -vvvvv --eval --json --read-write-mode --expr '
137+
builtins.fetchTarball {{
138+
url = "file://{missing_tarball}";
139+
sha256 = "{tarball_hash}";
140+
}}
141+
'
142+
""")
143+
144+
result_path = json.loads(result)
145+
print(f"✓ fetchTarball substitution works! Result: {result_path}")
146+
147+
# Verify the content is correct
148+
# fetchTarball strips the top-level directory if there's only one
149+
content = importer.succeed(f"cat {result_path}/hello.txt").strip()
150+
assert content == "Hello from tarball!", f"Content mismatch: {content}"
151+
print("✓ fetchTarball content verified!")
152+
153+
##########################################
154+
# Test 3: Verify fetchTree does NOT substitute (preserves metadata)
155+
##########################################
156+
157+
print("Testing that fetchTree without __final does NOT use substitution...")
158+
159+
# fetchTree with just narHash (not __final) should try to download, which will fail
160+
# since the file doesn't exist on the importer
161+
exit_code = importer.fail(f"""
162+
nix-instantiate --eval --json --read-write-mode --expr '
163+
builtins.fetchTree {{
164+
type = "tarball";
165+
url = "file:///only-on-substituter.tar.gz";
166+
narHash = "{tarball_hash_sri}";
167+
}}
168+
' 2>&1
169+
""")
170+
171+
# Should fail with "does not exist" since it tries to download instead of substituting
172+
assert "does not exist" in exit_code or "Couldn't open file" in exit_code, f"Expected download failure, got: {exit_code}"
173+
print("✓ fetchTree correctly does NOT substitute non-final inputs!")
174+
print(" (This preserves metadata like lastModified from the actual fetch)")
175+
'';
176+
}

0 commit comments

Comments
 (0)