Skip to content

Commit

Permalink
devpkg: use struct field values in FlakeInstallable.String (jetify-co…
Browse files Browse the repository at this point in the history
…m#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.
  • Loading branch information
gcurtis authored Nov 7, 2023
1 parent d89a373 commit 4a0ad52
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 11 deletions.
32 changes: 28 additions & 4 deletions internal/devpkg/flakeref.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,6 @@ type FlakeInstallable struct {
Ref FlakeRef
AttrPath string

raw string
Outputs string
}

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

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

Expand Down Expand Up @@ -501,9 +500,34 @@ func (f FlakeInstallable) SplitOutputs() []string {
return split
}

// String returns the raw installable string as given to ParseFlakeInstallable.
// String encodes the installable as a Nix command line argument. It normalizes
// the result such that if two installable values are equal, then their strings
// will also be equal.
//
// String always cleans the outputs spec as described by the Outputs field's
// documentation. The same normalization rules from FlakeRef.String still apply.
func (f FlakeInstallable) String() string {
return f.raw
str := f.Ref.String()
if str == "" {
return ""
}
if f.AttrPath != "" {
url, err := url.Parse(str)
if err != nil {
// This should never happen. Even an empty string is a
// valid URL.
panic("invalid URL from FlakeRef.String: " + str)
}
url.Fragment = f.AttrPath
str = url.String()
}
if f.Outputs != "" {
clean := strings.Join(f.SplitOutputs(), ",")
if clean != "" {
str += "^" + clean
}
}
return str
}

// splitOutputSpec cuts a flake installable around the last instance of ^.
Expand Down
76 changes: 69 additions & 7 deletions internal/devpkg/flakeref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
)

func TestParseFlakeRef(t *testing.T) {
Expand Down Expand Up @@ -264,17 +263,80 @@ func TestParseFlakeInstallable(t *testing.T) {
for installable, want := range cases {
t.Run(installable, func(t *testing.T) {
got, err := ParseFlakeInstallable(installable)
if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(FlakeRef{}, FlakeInstallable{})); diff != "" {
if diff := cmp.Diff(want, got); diff != "" {
if err != nil {
t.Errorf("got error: %s", err)
}
t.Errorf("wrong installable (-want +got):\n%s", diff)
}
if err != nil {
return
}
if installable != got.String() {
t.Errorf("got.String() = %q != %q", got, installable)
})
}
}

func TestFlakeInstallableString(t *testing.T) {
cases := map[FlakeInstallable]string{
{}: "",

// No attribute or outputs.
{Ref: FlakeRef{Type: "path", Path: "."}}: "path:.",
{Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake",
{Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake",
{Ref: FlakeRef{Type: "indirect", ID: "indirect"}}: "flake:indirect",

// Attribute without outputs.
{AttrPath: "app", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#app",
{AttrPath: "my#app", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#my%23app",
{AttrPath: "app", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#app",
{AttrPath: "my#app", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#my%23app",
{AttrPath: "app", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app",
{AttrPath: "my#app", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#my%23app",
{AttrPath: "app", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app",
{AttrPath: "my#app", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#my%23app",

// Attribute with single output.
{AttrPath: "app", Outputs: "out", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#app^out",
{AttrPath: "app", Outputs: "out", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#app^out",
{AttrPath: "app", Outputs: "out", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app^out",
{AttrPath: "app", Outputs: "out", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^out",

// Attribute with multiple outputs.
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#app^lib,out",
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#app^lib,out",
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app^lib,out",
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^lib,out",

// Outputs are cleaned and sorted.
{AttrPath: "app", Outputs: "out,lib", Ref: FlakeRef{Type: "path", Path: "."}}: "path:.#app^lib,out",
{AttrPath: "app", Outputs: "lib,out", Ref: FlakeRef{Type: "path", Path: "./flake"}}: "path:flake#app^lib,out",
{AttrPath: "app", Outputs: "out,,", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app^out",
{AttrPath: "app", Outputs: ",lib,out", Ref: FlakeRef{Type: "path", Path: "/flake"}}: "path:/flake#app^lib,out",
{AttrPath: "app", Outputs: ",", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app",

// Wildcard replaces other outputs.
{AttrPath: "app", Outputs: "*", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^*",
{AttrPath: "app", Outputs: "out,*", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^*",
{AttrPath: "app", Outputs: ",*", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^*",

// Outputs are not percent-encoded.
{AttrPath: "app", Outputs: "%", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^%",
{AttrPath: "app", Outputs: "/", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^/",
{AttrPath: "app", Outputs: "%2F", Ref: FlakeRef{Type: "indirect", ID: "nixpkgs"}}: "flake:nixpkgs#app^%2F",

// Missing or invalid fields.
{AttrPath: "app", Ref: FlakeRef{Type: "file", URL: ""}}: "",
{AttrPath: "app", Ref: FlakeRef{Type: "git", URL: ""}}: "",
{AttrPath: "app", Ref: FlakeRef{Type: "github", Owner: ""}}: "",
{AttrPath: "app", Ref: FlakeRef{Type: "indirect", ID: ""}}: "",
{AttrPath: "app", Ref: FlakeRef{Type: "path", Path: ""}}: "",
{AttrPath: "app", Ref: FlakeRef{Type: "tarball", URL: ""}}: "",
}

for installable, want := range cases {
t.Run(want, func(t *testing.T) {
t.Logf("input = %#v", installable)
got := installable.String()
if got != want {
t.Errorf("got %#q, want %#q", got, want)
}
})
}
Expand Down

0 comments on commit 4a0ad52

Please sign in to comment.