Only lookup the project FileValue for valid packages.

PiperOrigin-RevId: 619793530 Change-Id: I6df7e6202abf5706946c60e1b6faecca6bf1dfaa
bazelbuild · Mar 28, 2024 · 781e862 · 781e862
1 parent 581e466
commit 781e862
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 15 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
@@ -320,23 +320,22 @@ private PackageLookupValue getPackageLookupValue(
       return null;
     }
 
-    // Check for the existence of the project.scl file.
-    // TODO b/331316530: Temporarily removed to avoid build memory regressions. Re-enable as opt in.
-    /*RootedPath projectFileRootedPath =
-        RootedPath.toRootedPath(
-            packagePathEntry,
-            packageIdentifier.getPackageFragment().getRelative(PROJECT_FILE_NAME));
-    FileValue projectFileValue = getFileValue(projectFileRootedPath, env, packageIdentifier);
-    if (projectFileValue == null) {
-      return null;
-    } */
-
     if (fileValue.isFile()) {
-      // TODO b/331316530: Temporarily removed to avoid build memory regressions. Re-enable as opt
-      // in.
-      /*if (projectFileValue.exists() && !projectFileValue.isFile()) {
+      // Check for the existence of the project.scl file only in directories with a BUILD file.
+      // to avoid creating excessive FileValue nodes.
+      RootedPath projectFileRootedPath =
+          RootedPath.toRootedPath(
+              packagePathEntry,
+              packageIdentifier.getPackageFragment().getRelative(PROJECT_FILE_NAME));
+      FileValue projectFileValue = getFileValue(projectFileRootedPath, env, packageIdentifier);
+      if (projectFileValue == null) {
+        return null;
+      }
+
+      if (projectFileValue.exists() && !projectFileValue.isFile()) {
         return PackageLookupValue.INVALID_PROJECT_VALUE;
-      }*/
+      }
+
       return PackageLookupValue.success(
           buildFileRootedPath.getRoot(), buildFileName, /* hasProjectFile= */ false);
     }

diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/ProjectResolutionTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/ProjectResolutionTest.java
@@ -19,6 +19,7 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
@@ -30,7 +31,9 @@
 import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
+import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.common.options.Options;
+import java.util.Set;
 import java.util.UUID;
 import org.junit.Before;
 import org.junit.Ignore;
@@ -152,4 +155,33 @@ public void twoTargetsDifferentProjectFiles() throws Exception {
                     + "For example:  [foo/%s]: //foo:f [bar/%s]: //bar:g",
                 PROJECT_FILE_NAME, PROJECT_FILE_NAME));
   }
+
+  @Test
+  public void ignoredProjectFileInNonPackages() throws Exception {
+    write("foo/bar/baz/" + PROJECT_FILE_NAME);
+    write("foo/bar/BUILD", "genrule(name='f', cmd = '', srcs=[], outs=['f.out'])");
+    write("foo/bar/" + PROJECT_FILE_NAME);
+    write("foo/" + PROJECT_FILE_NAME);
+
+    assertThat(
+            BuildTool.getProjectFile(
+                ImmutableList.of(Label.parseCanonical("//foo/bar:f")),
+                getSkyframeExecutor(),
+                events.reporter()))
+        .isEqualTo(PathFragment.create("foo/bar/" + PROJECT_FILE_NAME));
+
+    Set<RootedPath> projectRootedPaths = Sets.newConcurrentHashSet();
+    getSkyframeExecutor()
+        .getEvaluator()
+        .getInMemoryGraph()
+        .parallelForEach(
+            k -> {
+              if (k.getKey().argument() instanceof RootedPath rp) {
+                if (rp.getRootRelativePath().getBaseName().equals(PROJECT_FILE_NAME)) {
+                  projectRootedPaths.add(rp);
+                }
+              }
+            });
+    assertThat(projectRootedPaths).hasSize(1);
+  }
 }