Skip to content

Commit 56af312

Browse files
authored
[bug] Fix parseStorePathFromInstallableOutput (#2098)
## Summary Fix regression introduced in #2076 I misunderstood the API. A null value (in modern API) or a valid: false (in legacy API) occur when store path doesn't exist. Whether the store path itself is a valid store path doesn't matter, obviously if it is not a valid path, it won't be present in the store, but if it's valid and not installed, it will return null value or valid: false. The users of this function do not want to ignore these paths. Edit: This bug is a bit less bad than initially thought. Because we install non-cached packages in flake, so anything missing would be installed then. ## How was it tested? Unit test. This is tricky to test. I added a test script that can hopefully catch this. Edit: This test doesn't actually catch this because the package ends up getting installed later on (in the flake). I still think this test is good to have.
1 parent b9eee59 commit 56af312

File tree

3 files changed

+27
-11
lines changed

3 files changed

+27
-11
lines changed

internal/nix/store.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,22 +87,17 @@ func StorePathsAreInStore(ctx context.Context, storePaths []string) (map[string]
8787

8888
// Older nix versions (like 2.17) are an array of objects that contain path and valid fields
8989
type pathInfoLegacy struct {
90-
Path string `json:"path"`
91-
Valid bool `json:"valid"`
90+
Path string `json:"path"`
9291
}
9392

9493
// parseStorePathFromInstallableOutput parses the output of `nix store path-from-installable --json`
9594
// This function is decomposed out of StorePathFromInstallable to make it testable.
9695
func parseStorePathFromInstallableOutput(output []byte) (map[string]any, error) {
9796
// Newer nix versions (like 2.20) have output of the form
9897
// {"<store-path>": {}}
99-
// if a store path is used as an installable, the keys will be present even if invalid but
100-
// the values will be null.
98+
// Note that values will be null if paths are not in store.
10199
var out1 map[string]any
102100
if err := json.Unmarshal(output, &out1); err == nil {
103-
maps.DeleteFunc(out1, func(k string, v any) bool {
104-
return v == nil
105-
})
106101
return out1, nil
107102
}
108103

@@ -111,9 +106,7 @@ func parseStorePathFromInstallableOutput(output []byte) (map[string]any, error)
111106
if err := json.Unmarshal(output, &out2); err == nil {
112107
res := map[string]any{}
113108
for _, outValue := range out2 {
114-
if outValue.Valid {
115-
res[outValue.Path] = true
116-
}
109+
res[outValue.Path] = true
117110
}
118111
return res, nil
119112
}

internal/nix/store_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func TestParseStorePathFromInstallableOutput(t *testing.T) {
2121
},
2222
{
2323
name: "go-basic-nix-2-17-0",
24-
input: `[{"path":"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0","valid":true}]`,
24+
input: `[{"path":"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"}]`,
2525
expected: []string{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"},
2626
},
2727
}

testscripts/lockfile/nopaths.txt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Test installing a package without outputs in the store path.
2+
# NOTE: Purposefully using a weird version to ensure it is not already in store.
3+
4+
exec devbox run curl --version | grep -o 'curl\s7\.87\.0'
5+
stdout 'curl 7.87.0'
6+
7+
-- devbox.json --
8+
{
9+
"packages": ["[email protected]"],
10+
}
11+
12+
-- devbox.lock --
13+
{
14+
"lockfile_version": "1",
15+
"packages": {
16+
17+
"last_modified": "2023-02-26T03:47:33Z",
18+
"resolved": "github:NixOS/nixpkgs/9952d6bc395f5841262b006fbace8dd7e143b634#curl",
19+
"source": "devbox-search",
20+
"version": "7.87.0"
21+
}
22+
}
23+
}

0 commit comments

Comments
 (0)