From d915b98376295bd010f669e7a60815a3eeca3d0c Mon Sep 17 00:00:00 2001
From: Googler <jingwen@google.com>
Date: Thu, 25 Jul 2024 09:05:34 -0700
Subject: [PATCH] skyfocus: when using PROJECT.scl file, don't attempt to infer
 using the top level packages.

This makes the working set derivation less confusing by not keeping files that
aren't specified in the PROJECT.scl file.

PiperOrigin-RevId: 655974491
Change-Id: I89c73e77bdc8e07be77570cfb8c18d5aed8ad9f4
---
 .../build/lib/skyframe/SkyfocusExecutor.java  | 23 ++++++------
 .../buildtool/SkyfocusIntegrationTest.java    | 36 +++++++++++++++++++
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyfocusExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyfocusExecutor.java
index 2d3411c4781de0..ae45e0ba3369a3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyfocusExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyfocusExecutor.java
@@ -111,21 +111,24 @@ public static Optional<SkyfocusState> prepareWorkingSet(
                       return;
                     }
 
-                    // Check if the file belongs to a project directory (defined in PROJECT.scl)
-                    //
-                    // If this end up being costly, we could represent projectDirectories as a trie
-                    // and iterate with PathFragment#segments.
-                    for (PathFragment projectDirectory : projectDirectories) {
-                      if (fileStateKey
-                          .argument()
-                          .getRootRelativePath()
-                          .startsWith(projectDirectory)) {
+                    if (!projectDirectories.isEmpty()) {
+                      // Check if the file belongs to a project directory (defined in PROJECT.scl)
+                      //
+                      // If this ends up being costly, we could represent projectDirectories as a
+                      // trie and iterate with PathFragment#segments.
+                      for (PathFragment projectDirectory : projectDirectories) {
+                        PathFragment pathFragment = fileStateKey.argument().getRootRelativePath();
+                        if (!pathFragment.startsWith(projectDirectory)) {
+                          continue;
+                        }
                         newWorkingSet.add(fileStateKey.argument());
                         return;
                       }
+                      return;
                     }
 
-                    // Check if the file belongs to the package of a top level target being built.
+                    // If project directories are not defined, check if the file belongs to the
+                    // package of a top level target being built.
                     PathFragment currPath = fileStateKey.argument().getRootRelativePath();
                     while (currPath != null) {
                       try {
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SkyfocusIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SkyfocusIntegrationTest.java
index 0884d6b1037769..5054d5ede709e2 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/SkyfocusIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/SkyfocusIntegrationTest.java
@@ -175,6 +175,42 @@ public void workingSet_canBeAutomaticallyDerivedUsingProjectFile() throws Except
             "somewhere/else/file.txt");
   }
 
+  @Test
+  public void workingSet_ignoresTopLevelPackageDirectoriesWhenUsingProjectFile() throws Exception {
+    addOptions("--experimental_enable_scl_dialect");
+
+    write("hello/x.txt", "x");
+    write(
+        "hello/BUILD",
+        """
+        genrule(
+            name = "target",
+            srcs = ["x.txt", "//somewhere/else:files"],
+            outs = ["out"],
+            cmd = "cat $(SRCS) > $@",
+        )
+        """);
+
+    // Files under //somewhere/else will be included because of this PROJECT.scl file.
+    write(
+        "hello/PROJECT.scl",
+        """
+        owned_code_paths = ["somewhere/else"]
+        """);
+
+    write("somewhere/else/file.txt", "some content");
+    write(
+        "somewhere/else/BUILD",
+        """
+        filegroup(name = "files", srcs = ["file.txt"])
+        """);
+
+    buildTarget("//hello:target");
+    assertContainsEvent("automatically deriving working set");
+    assertThat(getSkyframeExecutor().getSkyfocusState().workingSetStrings())
+        .containsExactly("somewhere/else", "somewhere/else/BUILD", "somewhere/else/file.txt");
+  }
+
   @Test
   public void workingSet_skyfocusDoesNotRunIfDerivedWorkingSetIsUnchanged() throws Exception {
     write("hello/x.txt", "x");