diff --git a/src/core/package.go b/src/core/package.go index 93c9a85c6..2ba00a911 100644 --- a/src/core/package.go +++ b/src/core/package.go @@ -240,6 +240,9 @@ func FindOwningPackage(state *BuildState, file string) BuildLabel { // suggestTargets suggests the targets in the given package that might be misspellings of // the requested one. func suggestTargets(pkg *Package, label, dependent BuildLabel) string { + if pkg == nil { + return "" + } // The initial haystack only contains target names haystack := []string{} for _, t := range pkg.AllTargets() { diff --git a/src/core/state.go b/src/core/state.go index f98b22881..69d149026 100644 --- a/src/core/state.go +++ b/src/core/state.go @@ -1017,7 +1017,9 @@ func (state *BuildState) ActivateTarget(pkg *Package, label, dependent BuildLabe // Bazel allows some things that look like build targets but aren't - notably the syntax // to load(). It suits us to treat that as though it is one, but we now have to // implicitly make it available. - exportFile(state, pkg, label) + if pkg != nil { + exportFile(state, pkg, label) + } } else { msg := fmt.Sprintf("Parsed build file %s but it doesn't contain target %s", pkg.Filename, label.Name) if dependent != OriginalTarget { @@ -1029,6 +1031,9 @@ func (state *BuildState) ActivateTarget(pkg *Package, label, dependent BuildLabe if state.ParsePackageOnly && !mode.IsForSubinclude() { return nil // Some kinds of query don't need a full recursive parse. } else if label.IsAllTargets() { + if pkg == nil { + return fmt.Errorf("Cannot use :all in this context") + } if dependent == OriginalTarget { for _, target := range pkg.AllTargets() { // Don't activate targets that were added in a post-build function; that causes a race condition diff --git a/src/parse/asp/builtins.go b/src/parse/asp/builtins.go index c8bd6253b..acef0b934 100644 --- a/src/parse/asp/builtins.go +++ b/src/parse/asp/builtins.go @@ -392,7 +392,7 @@ func subincludeTarget(s *scope, l core.BuildLabel) *core.BuildTarget { // If the subinclude is local to this package, it must already exist in the graph. If it already exists in the graph // but isn't activated, we should activate it otherwise WaitForSubincludedTarget might block. This can happen when // another package also subincludes this target, and queues it first. - if isLocal && s.pkg != nil { + if isLocal { t := s.state.Graph.Target(l) if t == nil { s.Error("Target :%s is not defined in this package; it has to be defined before the subinclude() call", l.Name) diff --git a/test/double_local_subinclude/BUILD b/test/double_local_subinclude/BUILD new file mode 100644 index 000000000..290c88e2a --- /dev/null +++ b/test/double_local_subinclude/BUILD @@ -0,0 +1,8 @@ +subinclude("//test/build_defs") + +please_repo_e2e_test( + name = "double_local_subinclude_test", + # This isn't 100% deterministic but 20 clean runs should be reasonably sufficient + plz_command = "for i in `seq 1 20`; do plz build -p -v 2 //...; done", + repo = "test_repo", +) diff --git a/test/double_local_subinclude/test_repo/.plzconfig b/test/double_local_subinclude/test_repo/.plzconfig new file mode 100644 index 000000000..e69de29bb diff --git a/test/double_local_subinclude/test_repo/a/1.build_defs b/test/double_local_subinclude/test_repo/a/1.build_defs new file mode 100644 index 000000000..500549eef --- /dev/null +++ b/test/double_local_subinclude/test_repo/a/1.build_defs @@ -0,0 +1 @@ +THE_QUESTION = "what is the answer to life, the universe and everything?" diff --git a/test/double_local_subinclude/test_repo/a/2.build_defs b/test/double_local_subinclude/test_repo/a/2.build_defs new file mode 100644 index 000000000..eec3aabb4 --- /dev/null +++ b/test/double_local_subinclude/test_repo/a/2.build_defs @@ -0,0 +1,3 @@ +subinclude("//a:defs_1") + +THE_ANSWER = 42 diff --git a/test/double_local_subinclude/test_repo/a/BUILD_FILE b/test/double_local_subinclude/test_repo/a/BUILD_FILE new file mode 100644 index 000000000..d6b75525c --- /dev/null +++ b/test/double_local_subinclude/test_repo/a/BUILD_FILE @@ -0,0 +1,16 @@ +filegroup( + name = "defs_1", + srcs = ["1.build_defs"], + visibility = ["PUBLIC"], +) + +filegroup( + name = "defs_2", + srcs = ["2.build_defs"], + visibility = ["PUBLIC"], +) + +subinclude(":defs_2") + +assert THE_QUESTION.endswith("?"), "not a question" +assert THE_ANSWER == 42, "wrong answer to life, the universe and everything" diff --git a/test/double_local_subinclude/test_repo/b/BUILD_FILE b/test/double_local_subinclude/test_repo/b/BUILD_FILE new file mode 100644 index 000000000..ff4038bbb --- /dev/null +++ b/test/double_local_subinclude/test_repo/b/BUILD_FILE @@ -0,0 +1,3 @@ +subinclude("//a:defs_2") + +assert THE_ANSWER == 42, "wrong answer to life, the universe and everything" diff --git a/test/double_local_subinclude/test_repo/c/BUILD_FILE b/test/double_local_subinclude/test_repo/c/BUILD_FILE new file mode 100644 index 000000000..d95ba18cd --- /dev/null +++ b/test/double_local_subinclude/test_repo/c/BUILD_FILE @@ -0,0 +1,3 @@ +subinclude("//a:defs_1") + +assert THE_QUESTION.endswith("?"), "not a question"