Skip to content

Commit

Permalink
Preserve canonical labels as such
Browse files Browse the repository at this point in the history
Previously, parsing and then stringifying a label would turn `@@canonical~name//pkg:target` into `@canonical~name//pkg:target`, which is no longer valid.

The new field on `Label` defaults to `false` if not set, which is preserves the current behavior.
  • Loading branch information
fmeum committed Aug 13, 2024
1 parent 2c65af5 commit dc3789a
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 9 deletions.
25 changes: 18 additions & 7 deletions label/label.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ type Label struct {
// Relative indicates whether the label refers to a target in the current
// package. Relative is true if and only if Repo and Pkg are both omitted.
Relative bool

// Canonical indicates whether the repository name is canonical. If true,
// then the label will be stringified with an extra "@" prefix if it is
// absolute.
// Note: Label does not apply any kind of repo mapping.
Canonical bool
}

// New constructs a new label from components.
Expand Down Expand Up @@ -83,10 +89,11 @@ func Parse(s string) (Label, error) {
origStr := s

relative := true
canonical := false
var repo string
// if target name begins @@ drop the first @
if strings.HasPrefix(s, "@@") {
s = s[len("@"):]
canonical = true
}
if strings.HasPrefix(s, "@") {
relative = false
Expand Down Expand Up @@ -140,10 +147,11 @@ func Parse(s string) (Label, error) {
}

return Label{
Repo: repo,
Pkg: pkg,
Name: name,
Relative: relative,
Repo: repo,
Pkg: pkg,
Name: name,
Relative: relative,
Canonical: canonical,
}, nil
}

Expand All @@ -160,6 +168,9 @@ func (l Label) String() string {
// if l.Repo == "@", the label string will begin with "@//"
repo = l.Repo
}
if l.Canonical && strings.HasPrefix(repo, "@") {
repo = "@" + repo
}

if path.Base(l.Pkg) == l.Name {
return fmt.Sprintf("%s//%s", repo, l.Pkg)
Expand Down Expand Up @@ -213,8 +224,8 @@ func (l Label) Contains(other Label) bool {
}

func (l Label) BzlExpr() bzl.Expr {
return &bzl.StringExpr {
Value: l.String(),
return &bzl.StringExpr{
Value: l.String(),
}
}

Expand Down
38 changes: 36 additions & 2 deletions label/label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ func TestParse(t *testing.T) {
{str: "@a//some/pkg/[someId]:[someId]", want: Label{Repo: "a", Pkg: "some/pkg/[someId]", Name: "[someId]"}},
{str: "@rules_python~0.0.0~pip~name_dep//:_pkg", want: Label{Repo: "rules_python~0.0.0~pip~name_dep", Name: "_pkg"}},
{str: "@rules_python~0.0.0~pip~name//:dep_pkg", want: Label{Repo: "rules_python~0.0.0~pip~name", Name: "dep_pkg"}},
{str: "@@rules_python~0.26.0~python~python_3_10_x86_64-unknown-linux-gnu//:python_runtimes", want: Label{Repo: "rules_python~0.26.0~python~python_3_10_x86_64-unknown-linux-gnu", Name: "python_runtimes"}},
{str: "@@rules_python~0.26.0~python~python_3_10_x86_64-unknown-linux-gnu//:python_runtimes", want: Label{Repo: "rules_python~0.26.0~python~python_3_10_x86_64-unknown-linux-gnu", Name: "python_runtimes", Canonical: true}},
{str: "@rules_python++pip+name_dep//:_pkg", want: Label{Repo: "rules_python++pip+name_dep", Name: "_pkg"}},
{str: "@rules_python++pip+name//:dep_pkg", want: Label{Repo: "rules_python++pip+name", Name: "dep_pkg"}},
{str: "@@rules_python++python+python_3_10_x86_64-unknown-linux-gnu//:python_runtimes", want: Label{Repo: "rules_python++python+python_3_10_x86_64-unknown-linux-gnu", Name: "python_runtimes"}},
{str: "@@rules_python++python+python_3_10_x86_64-unknown-linux-gnu//:python_runtimes", want: Label{Repo: "rules_python++python+python_3_10_x86_64-unknown-linux-gnu", Name: "python_runtimes", Canonical: true}},
} {
got, err := Parse(tc.str)
if err != nil && !tc.wantErr {
Expand All @@ -117,3 +117,37 @@ func TestImportPathToBazelRepoName(t *testing.T) {
}
}
}

func TestAbsRoundtrip(t *testing.T) {
for _, tc := range []struct {
in string
out string
}{
{in: "target", out: ":target"},
{in: ":target"},
{in: "//:target"},
{in: "//pkg:target"},
{in: "@repo//:target"},
{in: "@repo//pkg:target"},
{in: "@repo", out: "@repo//:repo"},
{in: "@//pkg:target"},
{in: "@@canonical~name//:target"},
{in: "@@//:target"},
} {
lbl, err := Parse(tc.in)
if err != nil {
t.Errorf("Parse(%q) failed: %v", tc, err)
continue
}
got := lbl.String()
var want string
if len(tc.out) == 0 {
want = tc.in
} else {
want = tc.out
}
if got != want {
t.Errorf("Parse(%q).String() = %q; want %q", tc.in, got, want)
}
}
}

0 comments on commit dc3789a

Please sign in to comment.