Handle a few more edge cases for parsing in context

src/core/build_label.go

@@ -3,6 +3,7 @@ package core
 import (
 	"context"
 	"fmt"
+	"github.com/thought-machine/please/src/cli"
 	"os"
 	"path/filepath"
 	"strings"
@@ -154,6 +155,25 @@ func TryParseBuildLabel(target, currentPath, subrepo string) (BuildLabel, error)
 	return BuildLabel{}, fmt.Errorf("Invalid build label: %s", target)
 }
 
+// SplitSubrepoArch splits a subrepo name into the subrepo and architecture parts
+func SplitSubrepoArch(subrepoName string) (string, string) {
+	if idx := strings.LastIndex(subrepoName, "@"); idx != -1 {
+		return subrepoName[:idx], subrepoName[(idx+1):]
+	}
+	return subrepoName, ""
+}
+
+// JoinSubrepoArch joins a subrepo name with an architecture
+func JoinSubrepoArch(subrepoName, arch string) string {
+	if subrepoName == "" {
+		return arch
+	}
+	if arch == "" {
+		return subrepoName
+	}
+	return fmt.Sprintf("%v@%v", subrepoName, arch)
+}
+
 // ParseBuildLabelParts parses a build label into the package & name parts.
 // If valid, the name string will always be populated; the package string might not be if it's a local form.
 func ParseBuildLabelParts(target, currentPath, subrepo string) (string, string, string) {
@@ -415,43 +435,54 @@ func (label BuildLabel) PackageDir() string {
 }
 
 // SubrepoLabel returns a build label corresponding to the subrepo part of this build label.
-func (label BuildLabel) SubrepoLabel(state *BuildState, dependentSubrepo string) BuildLabel {
-	arch := ""
-	if dependentSubrepo != "" {
-		a := state.Graph.Subrepo(dependentSubrepo).Arch
-		if a != state.Arch {
-			arch = a.String()
-		}
+func (label BuildLabel) SubrepoLabel(state *BuildState) BuildLabel {
+	if strings.Contains(label.String(), "golang.org_x_net") {
+		print()
 	}
 
-	if plugin, ok := state.Config.Plugin[label.Subrepo]; ok {
-		if plugin.Target.String() == "" {
-			log.Fatalf("[Plugin \"%v\"] must have Target set in the .plzconfig", label.Subrepo)
-		}
-		return plugin.Target
+	if strings.Contains(label.Subrepo, "@") {
+		print()
 	}
-	pluginName := strings.TrimSuffix(label.Subrepo, fmt.Sprintf("@%v", arch))
-	if plugin, ok := state.Config.Plugin[pluginName]; ok {
-		if plugin.Target.String() == "" {
-			log.Fatalf("[Plugin \"%v\"] must have Target set in the .plzconfig", pluginName)
-		}
-		return plugin.Target
+	pluginName, arch := SplitSubrepoArch(label.Subrepo)
+	if arch == "" && state.Arch != cli.HostArch() {
+		arch = state.Arch.String()
 	}
-	return label.subrepoLabel()
+
+	plugin, ok := state.Config.Plugin[pluginName]
+	if !ok {
+		return label.subrepoLabel(state)
+	}
+
+	if plugin.Target.String() == "" {
+		log.Fatalf("[Plugin \"%v\"] must have Target set in the .plzconfig", pluginName)
+	}
+
+	t := plugin.Target
+	// If the plugin already specifies an architecture, don't override it
+	if strings.Contains(t.Subrepo, "@") {
+		return t
+	}
+
+	// Otherwise we need to set it to match our architecture
+	if t.Subrepo == "" {
+		t.Subrepo = arch
+	} else {
+		t.Subrepo = fmt.Sprintf("%v@%v", t.Subrepo, arch)
+	}
+	return t
 }
 
-func (label BuildLabel) subrepoLabel() BuildLabel {
-	subrepo := ""
-	name := label.Subrepo
-	if idx := strings.LastIndex(label.Subrepo, "@"); idx != -1 {
-		subrepo = label.Subrepo[idx+1:]
-		name = label.Subrepo[:idx]
+func (label BuildLabel) subrepoLabel(state *BuildState) BuildLabel {
+	subrepoName, arch := SplitSubrepoArch(label.Subrepo)
+	if arch == "" && state.Arch != cli.HostArch() {
+		arch = state.Arch.String()
 	}
-	if idx := strings.LastIndexByte(name, '/'); idx != -1 {
-		return BuildLabel{PackageName: name[:idx], Name: name[idx+1:], Subrepo: subrepo}
+
+	if idx := strings.LastIndexByte(subrepoName, '/'); idx != -1 {
+		return BuildLabel{PackageName: subrepoName[:idx], Name: subrepoName[idx+1:], Subrepo: arch}
 	}
 	// This is legit, the subrepo is defined at the root.
-	return BuildLabel{Name: name, Subrepo: subrepo}
+	return BuildLabel{Name: subrepoName, Subrepo: arch}
 }
 
 func hashBuildLabel(l BuildLabel) uint64 {

src/core/build_label_test.go

@@ -109,14 +109,22 @@ func TestCompleteError(t *testing.T) {
 }
 
 func TestSubrepoLabel(t *testing.T) {
+	state := NewDefaultBuildState()
 	label := BuildLabel{Subrepo: "test"}
-	assert.EqualValues(t, BuildLabel{PackageName: "", Name: "test"}, label.subrepoLabel())
+	assert.EqualValues(t, BuildLabel{PackageName: "", Name: "test"}, label.subrepoLabel(state))
 	label.Subrepo = "package/test"
-	assert.EqualValues(t, BuildLabel{PackageName: "package", Name: "test"}, label.subrepoLabel())
+	assert.EqualValues(t, BuildLabel{PackageName: "package", Name: "test"}, label.subrepoLabel(state))
 	// This isn't really valid (the caller shouldn't need to call it in such a case)
 	// but we want to make sure it doesn't panic.
 	label.Subrepo = ""
-	assert.EqualValues(t, BuildLabel{PackageName: "", Name: ""}, label.subrepoLabel())
+	assert.EqualValues(t, BuildLabel{PackageName: "", Name: ""}, label.subrepoLabel(state))
+
+	state.Graph.AddSubrepo(&Subrepo{Name: "foowin_amd64", Arch: cli.NewArch("foowin", "amd64")})
+	state.Graph.AddSubrepo(&Subrepo{Name: "dependant@foowin_amd64", Arch: cli.NewArch("foowin", "amd64")})
+
+	label = BuildLabel{PackageName: "build_defs", Subrepo: "dependee@foowin_amd64"}
+	sl := label.SubrepoLabel(state)
+	assert.Equal(t, BuildLabel{Name: "dependee", Subrepo: "foowin_amd64"}, sl)
 }
 
 func TestPluginSubrepoLabel(t *testing.T) {
@@ -129,11 +137,11 @@ func TestPluginSubrepoLabel(t *testing.T) {
 
 	// Check we get back the plugins target instead
 	label := BuildLabel{Subrepo: "plugin"}
-	assert.Equal(t, subrepoLabel, label.SubrepoLabel(state, ""))
+	assert.Equal(t, subrepoLabel, label.SubrepoLabel(state))
 
 	// Check that we handle architecture variants of the plugin subrepo name
-	label = BuildLabel{Subrepo: "plugin_foowin_amd64"}
-	assert.Equal(t, subrepoLabel, label.SubrepoLabel(state, "foowin_amd64"))
+	label = BuildLabel{Subrepo: "plugin@foowin_amd64"}
+	assert.Equal(t, BuildLabel{PackageName: "foo/bar", Name: "plugin", Subrepo: "foowin_amd64"}, label.SubrepoLabel(state))
 }
 
 func TestParseBuildLabelParts(t *testing.T) {

src/parse/asp/builtins.go

@@ -347,7 +347,7 @@ func subincludeTarget(s *scope, l core.BuildLabel) *core.BuildTarget {
 	//
 	// By parsing the package first, the subrepo package's subinclude will queue the subrepo target to be built before
 	// we call WaitForSubincludedTarget below avoiding the lockup.
-	subrepoLabel := l.SubrepoLabel(s.state, "")
+	subrepoLabel := l.SubrepoLabel(s.state)
 	if l.Subrepo != "" && subrepoLabel.PackageName != pkg.Name && l.Subrepo != pkg.SubrepoName {
 		subrepoPackageLabel := core.BuildLabel{
 			PackageName: subrepoLabel.PackageName,

src/parse/asp/interpreter.go

@@ -305,25 +305,44 @@ func (s *scope) parseAnnotatedLabelInPackage(label string, pkg *core.Package) co
 
 // parseLabelInPackage parses a build label in the scope of the package given the current scope.
 func (s *scope) parseLabelInPackage(label string, pkg *core.Package) core.BuildLabel {
+	if label == "//route" {
+		print()
+	}
 	if p, name, subrepo := core.ParseBuildLabelParts(label, pkg.Name, pkg.SubrepoName); name != "" {
 		arch := cli.HostArch()
-		if pkg.Subrepo != nil {
-			arch = pkg.Subrepo.Arch
+
+		subrepo, subrepoArch := core.SplitSubrepoArch(subrepo)
+
+		// Strip out the host arch as that's the default
+		if subrepo == arch.String() {
+			subrepo = ""
+		}
+
+		// Similarly trim the architecture if it's the host subrepo
+		if subrepoArch == arch.String() {
+			subrepoArch = ""
+		}
+
+		// If the subrepo matches the host repo's plugin name, then strip it out e.g. if we're in the Go plugin repo,
+		// then ///go//build_defs:go should translate to just //build_defs:go
+		if s.state.CurrentSubrepo == "" && subrepo == s.state.Config.PluginDefinition.Name {
+			subrepo = ""
 		}
-		subrepoArch := ""
-		if idx := strings.LastIndex(subrepo, "@"); idx != -1 {
-			subrepoArch = subrepo[idx+1:]
+
+		pkgArch := ""
+		if pkg.Subrepo != nil && pkg.Subrepo.Arch != cli.HostArch() {
+			pkgArch = pkg.Subrepo.Arch.String()
 		}
+
+
 		if subrepo == "" && pkg.SubrepoName != "" && (label[0] != '@' && !strings.HasPrefix(label, "///")) {
 			subrepo = pkg.SubrepoName
-			subrepoArch = arch.String()
-		} else if s.state.CurrentSubrepo == "" && subrepo == s.state.Config.PluginDefinition.Name {
-			subrepo = ""
 		}
-		if subrepoArch == "" && subrepo != arch.String() {
+
+		if subrepoArch == "" && pkgArch != "" && pkgArch != subrepo {
 			subrepo = pkg.SubrepoArchName(subrepo)
 		}
-		return core.BuildLabel{PackageName: p, Name: name, Subrepo: subrepo}
+		return core.BuildLabel{PackageName: p, Name: name, Subrepo: core.JoinSubrepoArch(subrepo, subrepoArch)}
 	}
 	return core.ParseBuildLabel(label, pkg.Name)
 }

src/parse/asp/label_context_test.go

@@ -88,12 +88,20 @@ func TestParseLabelContext(t *testing.T) {
 		},
 		{
 			testName: "Test host arch is stripped from subrepo",
-			label:    fmt.Sprintf("///subrepo_%s//test:target", (&arch).String()),
+			label:    fmt.Sprintf("///subrepo@%s//test:target", (&arch).String()),
 			scope:    newScope("pkg", "", ""),
 			subrepo:  "subrepo",
 			pkg:      "test",
 			name:     "target",
 		},
+		{
+			testName: "Test host arch is stripped from nested subrepo",
+			label:    "///foowin_amd64//test:target",
+			scope:    newScope("pkg", "subrepo2@foowin_amd64", ""),
+			subrepo:  "foowin_amd64",
+			pkg:      "test",
+			name:     "target",
+		},
 		{
 			testName: "Test host plugin is stripped",
 			label:    "///foo//test:target",

src/parse/asp/targets.go

@@ -250,6 +250,9 @@ func decodeCommands(s *scope, obj pyObject) (string, map[string]string) {
 
 // populateTarget sets the assorted attributes on a build target.
 func populateTarget(s *scope, t *core.BuildTarget, args []pyObject) {
+	if t.Label.String() == "///third_party/go/golang.org_x_net@darwin_amd64//:installs" {
+		print()
+	}
 	if t.IsRemoteFile {
 		for _, url := range mustList(args[urlsBuildRuleArgIdx]) {
 			t.AddSource(core.URLLabel(url.(pyString)))

src/parse/parse_step.go

@@ -100,7 +100,7 @@ func checkSubrepo(state *core.BuildState, label, dependent core.BuildLabel, mode
 		return subrepo, nil
 	}
 
-	sl := label.SubrepoLabel(state, dependent.Subrepo)
+	sl := label.SubrepoLabel(state)
 
 	// Local subincludes are when we subinclude from a subrepo defined in the current package
 	localSubinclude := label.Subrepo == dependent.Subrepo && label.PackageName == dependent.PackageName && mode.IsForSubinclude()

src/plz/plz.go

@@ -166,7 +166,7 @@ func findOriginalTask(state *core.BuildState, target core.BuildLabel, addToList
 		dir := target.PackageName
 		prefix := ""
 		if target.Subrepo != "" {
-			subrepoLabel := target.SubrepoLabel(state, "")
+			subrepoLabel := target.SubrepoLabel(state)
 			state.WaitForInitialTargetAndEnsureDownload(subrepoLabel, target)
 			// Targets now get activated during parsing, so can be built before we finish parsing their package.
 			state.WaitForPackage(subrepoLabel, target, core.ParseModeNormal)