add test to ensure subincluding subrepo target before subrepo is defi…

…ned errors (#2988)

Co-authored-by: rgodden <rgodden@thoughtmachine.net>

src/core/build_label.go

@@ -47,7 +47,7 @@ func (label BuildLabel) String() string {
 	zero := BuildLabel{} //nolint:ifshort
 	if label == zero {
 		return ""
-	} else if label == OriginalTarget {
+	} else if label.IsOriginalTarget() {
 		return "command-line targets"
 	}
 	s := "//" + label.PackageName
@@ -63,6 +63,10 @@ func (label BuildLabel) String() string {
 	return s + ":" + label.Name
 }
 
+func (label BuildLabel) IsOriginalTarget() bool {
+	return label == OriginalTarget
+}
+
 // ShortString returns a string representation of this build label, abbreviated if
 // possible, and relative to the given label.
 func (label BuildLabel) ShortString(context BuildLabel) string {

src/parse/parse_step.go

@@ -90,7 +90,7 @@ func parse(state *core.BuildState, label, dependent core.BuildLabel, mode core.P
 }
 
 func inSamePackage(label, dependent core.BuildLabel) bool {
-	return label.Subrepo == dependent.Subrepo && label.PackageName == dependent.PackageName
+	return !dependent.IsOriginalTarget() && label.Subrepo == dependent.Subrepo && label.PackageName == dependent.PackageName
 }
 
 // checkSubrepo checks if the label we're parsing is within a subrepo, returning that subrepo, if present in the label.
@@ -108,17 +108,17 @@ func checkSubrepo(state *core.BuildState, label, dependent core.BuildLabel, mode
 		return subrepo, nil
 	}
 
+	// SubrepoLabel returns the expected build label for the subrepo's target. Parsing its package should give us the
+	// subrepo we're looking for.
+	sl := label.SubrepoLabel(state)
+
 	// This can happen when we subinclude() a target in a subrepo from the same package the subrepo is defined in. In,
 	// this case, the subrepo must be registered by now. We shouldn't continue to try and parse the subrepo package, as
 	// it's the current package we're parsing, which would result in a lockup.
-	if inSamePackage(label, dependent) {
-		return nil, fmt.Errorf("subrepo %v is not defined yet in this package yet. It must appear before it is used by %v", label.Subrepo, dependent)
+	if inSamePackage(sl, dependent) {
+		return nil, fmt.Errorf("subrepo %v is not defined in this package yet. It must appear before it is used by %v", sl.Subrepo, dependent)
 	}
 
-	// SubrepoLabel returns the expected build label for the subrepo's target. Parsing its package should give us the
-	// subrepo we're looking for.
-	sl := label.SubrepoLabel(state)
-
 	// Try parsing the package in the host repo first.
 	s, err := parseSubrepoPackage(state, sl.PackageName, sl.Subrepo, label, mode)
 	if err != nil || s != nil {

test/subrepo/BUILD

@@ -0,0 +1,11 @@
+subinclude("//test/build_defs")
+
+please_repo_e2e_test(
+    name = "same_package_error",
+    expect_output_contains = {
+        "output": "is not defined in this package yet",
+    },
+    expected_failure = True,
+    plz_command = "plz build //:all 2>output",
+    repo = "test_repo",
+)

test/subrepo/test_repo/BUILD_FILE

@@ -0,0 +1,6 @@
+subinclude("///subrepo//:foo")
+
+local_repository(
+    name = "subrepo",
+    path = "subrepo",
+)

test/subrepo/test_repo/subrepo/BUILD_FILE

@@ -0,0 +1,4 @@
+genrule(
+    name = "foo",
+    cmd = "touch foo.txt",
+)