Skip to content

Commit b95f4cd

Browse files
authored
[nix] Fix StorePathsAreInStore (#2104)
## Summary Addresses issue introduced in #2076 and #2098 Basically we have 2 different uses cases when looking up store paths. Sometimes we want all store paths (whether installed or not) and in other cases we only want paths that are in local store. This change fixes that and adds some testing. ## How was it tested? unit tests, manual integration test by installing a package that was not in local store.
1 parent 01c0417 commit b95f4cd

File tree

2 files changed

+48
-35
lines changed

2 files changed

+48
-35
lines changed

internal/nix/store.go

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,12 @@ func StorePathsFromInstallable(ctx context.Context, installable string, allowIns
4949
return nil, err
5050
}
5151

52-
validPaths, err := parseStorePathFromInstallableOutput(resultBytes)
52+
paths, err := parseStorePathFromInstallableOutput(resultBytes)
5353
if err != nil {
5454
return nil, fmt.Errorf("failed to parse path-info for %s: %w", installable, err)
5555
}
5656

57-
return maps.Keys(validPaths), nil
57+
return maps.Keys(paths), nil
5858
}
5959

6060
// StorePathsAreInStore a map of store paths to whether they are in the store.
@@ -71,44 +71,39 @@ func StorePathsAreInStore(ctx context.Context, storePaths []string) (map[string]
7171
return nil, err
7272
}
7373

74-
validPaths, err := parseStorePathFromInstallableOutput(output)
75-
if err != nil {
76-
return nil, err
77-
}
78-
79-
result := map[string]bool{}
80-
for _, storePath := range storePaths {
81-
_, ok := validPaths[storePath]
82-
result[storePath] = ok
83-
}
84-
85-
return result, nil
74+
return parseStorePathFromInstallableOutput(output)
8675
}
8776

8877
// Older nix versions (like 2.17) are an array of objects that contain path and valid fields
89-
type pathInfoLegacy struct {
90-
Path string `json:"path"`
78+
type LegacyPathInfo struct {
79+
Path string `json:"path"`
80+
Valid bool `json:"valid"` // this means path is in store
9181
}
9282

9383
// parseStorePathFromInstallableOutput parses the output of `nix store path-from-installable --json`
84+
// into a map of store paths to whether they are in the store.
9485
// This function is decomposed out of StorePathFromInstallable to make it testable.
95-
func parseStorePathFromInstallableOutput(output []byte) (map[string]any, error) {
86+
func parseStorePathFromInstallableOutput(output []byte) (map[string]bool, error) {
87+
result := map[string]bool{}
88+
9689
// Newer nix versions (like 2.20) have output of the form
9790
// {"<store-path>": {}}
9891
// Note that values will be null if paths are not in store.
99-
var out1 map[string]any
100-
if err := json.Unmarshal(output, &out1); err == nil {
101-
return out1, nil
92+
var modernPathInfo map[string]any
93+
if err := json.Unmarshal(output, &modernPathInfo); err == nil {
94+
for path, val := range modernPathInfo {
95+
result[path] = val != nil
96+
}
97+
return result, nil
10298
}
10399

104-
var out2 []pathInfoLegacy
100+
var legacyPathInfos []LegacyPathInfo
105101

106-
if err := json.Unmarshal(output, &out2); err == nil {
107-
res := map[string]any{}
108-
for _, outValue := range out2 {
109-
res[outValue.Path] = true
102+
if err := json.Unmarshal(output, &legacyPathInfos); err == nil {
103+
for _, outValue := range legacyPathInfos {
104+
result[outValue.Path] = outValue.Valid
110105
}
111-
return res, nil
106+
return result, nil
112107
}
113108

114109
return nil, fmt.Errorf("failed to parse path-info output: %s", output)

internal/nix/store_test.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package nix
22

33
import (
4-
"slices"
54
"testing"
65

76
"golang.org/x/exp/maps"
@@ -11,18 +10,37 @@ func TestParseStorePathFromInstallableOutput(t *testing.T) {
1110
testCases := []struct {
1211
name string
1312
input string
14-
expected []string
13+
expected map[string]bool
1514
}{
1615
{
1716
name: "go-basic-nix-2-20-1",
1817
// snipped the actual output for brevity. We mainly care about the first key in the JSON.
19-
input: `{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0":{"deriver":"/nix/store/clr3bm8njqysvyw4r4x4xmldhz4knrff-go-1.22.0.drv"}}`,
20-
expected: []string{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"},
18+
input: `{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0":{"deriver":"/nix/store/clr3bm8njqysvyw4r4x4xmldhz4knrff-go-1.22.0.drv"}}`,
19+
expected: map[string]bool{
20+
"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0": true,
21+
},
2122
},
2223
{
23-
name: "go-basic-nix-2-17-0",
24-
input: `[{"path":"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"}]`,
25-
expected: []string{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"},
24+
name: "go-basic-nix-2-20-1",
25+
// snipped the actual output for brevity. We mainly care about the first key in the JSON.
26+
input: `{"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0":null}`,
27+
expected: map[string]bool{
28+
"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0": false,
29+
},
30+
},
31+
{
32+
name: "go-basic-nix-2-17-0",
33+
input: `[{"path":"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0"}]`,
34+
expected: map[string]bool{
35+
"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0": false,
36+
},
37+
},
38+
{
39+
name: "go-basic-nix-2-17-0",
40+
input: `[{"path":"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0", "valid": true}]`,
41+
expected: map[string]bool{
42+
"/nix/store/fgkl3qk8p5hnd07b0dhzfky3ys5gxjmq-go-1.22.0": true,
43+
},
2644
},
2745
}
2846

@@ -32,8 +50,8 @@ func TestParseStorePathFromInstallableOutput(t *testing.T) {
3250
if err != nil {
3351
t.Errorf("Expected no error but got error: %s", err)
3452
}
35-
if !slices.Equal(tc.expected, maps.Keys(actual)) {
36-
t.Errorf("Expected store path %s but got %s", tc.expected, actual)
53+
if !maps.Equal(tc.expected, actual) {
54+
t.Errorf("Expected store path %v but got %v", tc.expected, actual)
3755
}
3856
})
3957
}

0 commit comments

Comments
 (0)