Skip to content

Commit 4a0ad52

Browse files
gcurtisgcurtis
authored
devpkg: use struct field values in FlakeInstallable.String (#1600)
Improve `FlakeInstallable.String` so that it reassembles the installable string using the current field values instead of returning the original parsed string that it came from. This mirrors the other commit that updates `FlakeRef.String`. Similar to `FlakeRef.String`, `FlakeInstallable.String` normalizes the resulting string so that if two installables are equal, then their strings will always be equal.
1 parent d89a373 commit 4a0ad52

File tree

2 files changed

+97
-11
lines changed

2 files changed

+97
-11
lines changed

‎internal/devpkg/flakeref.go

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,6 @@ type FlakeInstallable struct {
441441
Ref FlakeRef
442442
AttrPath string
443443

444-
raw string
445444
Outputs string
446445
}
447446

@@ -455,7 +454,7 @@ func ParseFlakeInstallable(raw string) (FlakeInstallable, error) {
455454

456455
// The output spec must be parsed and removed first, otherwise it will
457456
// be parsed as part of the flakeref's URL fragment.
458-
install := FlakeInstallable{raw: raw}
457+
install := FlakeInstallable{}
459458
raw, install.Outputs = splitOutputSpec(raw)
460459
install.Outputs = strings.Join(install.SplitOutputs(), ",") // clean the outputs
461460

@@ -501,9 +500,34 @@ func (f FlakeInstallable) SplitOutputs() []string {
501500
return split
502501
}
503502

504-
// String returns the raw installable string as given to ParseFlakeInstallable.
503+
// String encodes the installable as a Nix command line argument. It normalizes
504+
// the result such that if two installable values are equal, then their strings
505+
// will also be equal.
506+
//
507+
// String always cleans the outputs spec as described by the Outputs field's
508+
// documentation. The same normalization rules from FlakeRef.String still apply.
505509
func (f FlakeInstallable) String() string {
506-
return f.raw
510+
str := f.Ref.String()
511+
if str == "" {
512+
return ""
513+
}
514+
if f.AttrPath != "" {
515+
url, err := url.Parse(str)
516+
if err != nil {
517+
// This should never happen. Even an empty string is a
518+
// valid URL.
519+
panic("invalid URL from FlakeRef.String: " + str)
520+
}
521+
url.Fragment = f.AttrPath
522+
str = url.String()
523+
}
524+
if f.Outputs != "" {
525+
clean := strings.Join(f.SplitOutputs(), ",")
526+
if clean != "" {
527+
str += "^" + clean
528+
}
529+
}
530+
return str
507531
}
508532

509533
// splitOutputSpec cuts a flake installable around the last instance of ^.

‎internal/devpkg/flakeref_test.go

Lines changed: 69 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"testing"
55

66
"github.com/google/go-cmp/cmp"
7-
"github.com/google/go-cmp/cmp/cmpopts"
87
)
98

109
func TestParseFlakeRef(t *testing.T) {
@@ -264,17 +263,80 @@ func TestParseFlakeInstallable(t *testing.T) {
264263
for installable, want := range cases {
265264
t.Run(installable, func(t *testing.T) {
266265
got, err := ParseFlakeInstallable(installable)
267-
if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(FlakeRef{}, FlakeInstallable{})); diff != "" {
266+
if diff := cmp.Diff(want, got); diff != "" {
268267
if err != nil {
269268
t.Errorf("got error: %s", err)
270269
}
271270
t.Errorf("wrong installable (-want +got):\n%s", diff)
272271
}
273-
if err != nil {
274-
return
275-
}
276-
if installable != got.String() {
277-
t.Errorf("got.String() = %q != %q", got, installable)
272+
})
273+
}
274+
}
275+
276+
func TestFlakeInstallableString(t *testing.T) {
277+
cases := map[FlakeInstallable]string{
278+
{}: "",
279+
280+
// No attribute or outputs.
281+
{Ref: FlakeRef{Type: "path", Path: "."}}: "path:.",
282+
{Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake",
283+
{Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake",
284+
{Ref: FlakeRef{Type: "indirect", ID: "indirect"}}: "flake:indirect",
285+
286+
// Attribute without outputs.
287+
{AttrPath: "app", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#app",
288+
{AttrPath: "my#app", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#my%23app",
289+
{AttrPath: "app", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#app",
290+
{AttrPath: "my#app", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#my%23app",
291+
{AttrPath: "app", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app",
292+
{AttrPath: "my#app", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#my%23app",
293+
{AttrPath: "app", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app",
294+
{AttrPath: "my#app", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#my%23app",
295+
296+
// Attribute with single output.
297+
{AttrPath: "app", Outputs: "out", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#app^out",
298+
{AttrPath: "app", Outputs: "out", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#app^out",
299+
{AttrPath: "app", Outputs: "out", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app^out",
300+
{AttrPath: "app", Outputs: "out", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^out",
301+
302+
// Attribute with multiple outputs.
303+
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#app^lib,out",
304+
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#app^lib,out",
305+
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app^lib,out",
306+
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^lib,out",
307+
308+
// Outputs are cleaned and sorted.
309+
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#app^lib,out",
310+
{AttrPath: "app", Outputs: "lib,out", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#app^lib,out",
311+
{AttrPath: "app", Outputs: "out,,", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app^out",
312+
{AttrPath: "app", Outputs: ",lib,out", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app^lib,out",
313+
{AttrPath: "app", Outputs: ",", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app",
314+
315+
// Wildcard replaces other outputs.
316+
{AttrPath: "app", Outputs: "*", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^*",
317+
{AttrPath: "app", Outputs: "out,*", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^*",
318+
{AttrPath: "app", Outputs: ",*", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^*",
319+
320+
// Outputs are not percent-encoded.
321+
{AttrPath: "app", Outputs: "%", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^%",
322+
{AttrPath: "app", Outputs: "/", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^/",
323+
{AttrPath: "app", Outputs: "%2F", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^%2F",
324+
325+
// Missing or invalid fields.
326+
{AttrPath: "app", Ref: FlakeRef{Type: "file", URL: ""}}: "",
327+
{AttrPath: "app", Ref: FlakeRef{Type: "git", URL: ""}}: "",
328+
{AttrPath: "app", Ref: FlakeRef{Type: "github", Owner: ""}}: "",
329+
{AttrPath: "app", Ref: FlakeRef{Type: "indirect", ID: ""}}: "",
330+
{AttrPath: "app", Ref: FlakeRef{Type: "path", Path: ""}}: "",
331+
{AttrPath: "app", Ref: FlakeRef{Type: "tarball", URL: ""}}: "",
332+
}
333+
334+
for installable, want := range cases {
335+
t.Run(want, func(t *testing.T) {
336+
t.Logf("input = %#v", installable)
337+
got := installable.String()
338+
if got != want {
339+
t.Errorf("got %#q, want %#q", got, want)
278340
}
279341
})
280342
}

0 commit comments

Comments
 (0)