From e1fbc8349850145a12136d2488e0ca70e46fc8e9 Mon Sep 17 00:00:00 2001 From: "bazel.build machine account" <15028808+bazel-io@users.noreply.github.com> Date: Mon, 13 Nov 2023 08:43:13 -0500 Subject: [PATCH] [7.0.0] [Skymeld] Don't plant the symlinks to the paths specified in (#20168) Bazel in skymeld mode eagerly plants the symlinks from the execroot to every entries under the source tree. This leads to some pain points for certain groups of users (https://github.com/bazelbuild/bazel/issues/19581). This CL offers a workaround for that issue. The users can now list the paths that they don't want Bazel to symlink to in the `.bazelignore` file in their project. Commit https://github.com/bazelbuild/bazel/commit/caf1702ce9fa060169548181afb9e19956826f0b PiperOrigin-RevId: 581919915 Change-Id: I329365b0655fc0bcb2db1eeea91ef74c775c72ae Co-authored-by: Googler --- .../build/lib/buildtool/ExecutionTool.java | 1 + .../build/lib/buildtool/SymlinkForest.java | 24 ++++++++++++--- .../lib/skyframe/IncrementalPackageRoots.java | 24 ++++++++++----- .../build/lib/skyframe/SkyframeExecutor.java | 8 ++++- .../SkymeldBuildIntegrationTest.java | 30 +++++++++++++++++++ 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index 5b60cf8dd97cf3..3b4e6a638d6765 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java @@ -275,6 +275,7 @@ public void prepareForExecution(Stopwatch executionTimer) singleSourceRoot, env.getEventBus(), env.getDirectories().getProductName() + "-", + skyframeExecutor.getIgnoredPaths(), request.getOptions(BuildLanguageOptions.class).experimentalSiblingRepositoryLayout, runtime.getWorkspace().doesAllowExternalRepositories()); if (shouldSymlinksBePlanted) { diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java index 8fba8147933ce7..f8870c021cea9e 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java @@ -140,7 +140,7 @@ private void plantSymlinkForestWithFullMainRepository( for (Path target : mainRepoRoot.getDirectoryEntries()) { String baseName = target.getBaseName(); Path execPath = execroot.getRelative(baseName); - if (symlinkShouldBePlanted(prefix, siblingRepositoryLayout, baseName)) { + if (symlinkShouldBePlanted(prefix, siblingRepositoryLayout, baseName, target)) { execPath.createSymbolicLink(target); plantedSymlinks.add(execPath); // TODO(jingwen-external): is this creating execroot/io_bazel/external? @@ -384,7 +384,11 @@ private static void deleteSiblingRepositorySymlinks( * @return a set of potentially conflicting baseNames, all in lowercase. */ public static ImmutableSet eagerlyPlantSymlinkForestSinglePackagePath( - Path execroot, Path sourceRoot, String prefix, boolean siblingRepositoryLayout) + Path execroot, + Path sourceRoot, + String prefix, + ImmutableSet ignoredPaths, + boolean siblingRepositoryLayout) throws IOException { deleteTreesBelowNotPrefixed(execroot, prefix); deleteSiblingRepositorySymlinks(siblingRepositoryLayout, execroot); @@ -406,7 +410,8 @@ public static ImmutableSet eagerlyPlantSymlinkForestSinglePackagePath( Path target = Iterables.getOnlyElement(targets); String originalBaseName = target.getBaseName(); Path link = execroot.getRelative(originalBaseName); - if (symlinkShouldBePlanted(prefix, siblingRepositoryLayout, originalBaseName)) { + if (symlinkShouldBePlanted( + prefix, ignoredPaths, siblingRepositoryLayout, originalBaseName, target)) { link.createSymbolicLink(target); } } else { @@ -416,11 +421,22 @@ public static ImmutableSet eagerlyPlantSymlinkForestSinglePackagePath( return ImmutableSet.copyOf(potentiallyConflictingBaseNamesLowercase); } + static boolean symlinkShouldBePlanted( + String prefix, boolean siblingRepositoryLayout, String baseName, Path target) { + return symlinkShouldBePlanted( + prefix, ImmutableSet.of(), siblingRepositoryLayout, baseName, target); + } + public static boolean symlinkShouldBePlanted( - String prefix, boolean siblingRepositoryLayout, String baseName) { + String prefix, + ImmutableSet ignoredPaths, + boolean siblingRepositoryLayout, + String baseName, + Path target) { // Create any links that don't start with bazel-, and ignore external/ directory if // user has it in the source tree because it conflicts with external repository location. return !baseName.startsWith(prefix) + && !ignoredPaths.contains(target) && (siblingRepositoryLayout || !baseName.equals(LabelConstants.EXTERNAL_PATH_PREFIX.getBaseName())); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/IncrementalPackageRoots.java b/src/main/java/com/google/devtools/build/lib/skyframe/IncrementalPackageRoots.java index 848f2ddf483814..50eef9d946e7ce 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/IncrementalPackageRoots.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/IncrementalPackageRoots.java @@ -67,6 +67,8 @@ public class IncrementalPackageRoots implements PackageRoots { private final Path execroot; private final Root singleSourceRoot; private final String prefix; + + private final ImmutableSet ignoredPaths; private final boolean useSiblingRepositoryLayout; private final boolean allowExternalRepositories; @@ -80,12 +82,14 @@ private IncrementalPackageRoots( Root singleSourceRoot, EventBus eventBus, String prefix, + ImmutableSet ignoredPaths, boolean useSiblingRepositoryLayout, boolean allowExternalRepositories) { this.threadSafeExternalRepoPackageRootsMap = Maps.newConcurrentMap(); this.execroot = execroot; this.singleSourceRoot = singleSourceRoot; this.prefix = prefix; + this.ignoredPaths = ignoredPaths; this.eventBus = eventBus; this.useSiblingRepositoryLayout = useSiblingRepositoryLayout; this.allowExternalRepositories = allowExternalRepositories; @@ -96,6 +100,7 @@ public static IncrementalPackageRoots createAndRegisterToEventBus( Root singleSourceRoot, EventBus eventBus, String prefix, + ImmutableSet ignoredPaths, boolean useSiblingRepositoryLayout, boolean allowExternalRepositories) { IncrementalPackageRoots incrementalPackageRoots = @@ -104,6 +109,7 @@ public static IncrementalPackageRoots createAndRegisterToEventBus( singleSourceRoot, eventBus, prefix, + ignoredPaths, useSiblingRepositoryLayout, allowExternalRepositories); eventBus.register(incrementalPackageRoots); @@ -140,7 +146,11 @@ public void eagerlyPlantSymlinksToSingleSourceRoot() throws AbruptExitException try { maybeConflictingBaseNamesLowercase = SymlinkForest.eagerlyPlantSymlinkForestSinglePackagePath( - execroot, singleSourceRoot.asPath(), prefix, useSiblingRepositoryLayout); + execroot, + singleSourceRoot.asPath(), + prefix, + ignoredPaths, + useSiblingRepositoryLayout); } catch (IOException e) { throwAbruptExitException(e); } @@ -212,19 +222,19 @@ private void registerAndPlantMissingSymlinks(NestedSet packages) String originalBaseName = pkgId.getTopLevelDir(); String baseNameLowercase = Ascii.toLowerCase(originalBaseName); + // As Skymeld only supports single package path at the moment, we only seek to symlink to + // the top-level dir i.e. what's directly under the source root. + Path link = execroot.getRelative(originalBaseName); + Path target = singleSourceRoot.getRelative(originalBaseName); + if (originalBaseName.isEmpty() || !maybeConflictingBaseNamesLowercase.contains(baseNameLowercase) || !SymlinkForest.symlinkShouldBePlanted( - prefix, useSiblingRepositoryLayout, originalBaseName)) { + prefix, ignoredPaths, useSiblingRepositoryLayout, originalBaseName, target)) { // We should have already eagerly planted a symlink for this, or there's nothing to do. continue; } - // As Skymeld only supports single package path at the moment, we only seek to symlink to - // the top-level dir i.e. what's directly under the source root. - Path link = execroot.getRelative(originalBaseName); - Path target = singleSourceRoot.getRelative(originalBaseName); - if (lazilyPlantedSymlinksLocalRef.add(link)) { try { link.createSymbolicLink(target); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 12d2edbed166b3..e994c099f5dd0a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -442,6 +442,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { @Nullable private final WorkspaceInfoFromDiffReceiver workspaceInfoFromDiffReceiver; private Set previousClientEnvironment = ImmutableSet.of(); + // Contain the paths in the .bazelignore file. + private ImmutableSet ignoredPaths = ImmutableSet.of(); + Duration sourceDiffCheckingDuration = Duration.ofSeconds(-1L); final class PathResolverFactoryImpl implements PathResolverFactory { @@ -1295,6 +1298,10 @@ public AtomicReference getPackageLocator() { return pkgLocator; } + public ImmutableSet getIgnoredPaths() { + return ignoredPaths; + } + protected Differencer.Diff getDiff( TimestampGranularityMonitor tsgm, ModifiedFileSet modifiedFileSet, @@ -3120,7 +3127,6 @@ protected WorkspaceInfoFromDiff handleDiffs( // Ignored package prefixes are specified relative to the workspace root // by definition of .bazelignore. So, we only use ignored paths when the // package root is equal to the workspace path. - ImmutableSet ignoredPaths = ImmutableSet.of(); if (workspacePath != null && workspacePath.equals(pathEntry.asPath())) { ignoredPaths = ignoredPackagePrefixesValue.getPatterns().stream() diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java index 269f1f44ba2471..e8dae66732877d 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/SkymeldBuildIntegrationTest.java @@ -366,6 +366,36 @@ public void symlinksPlanted() throws Exception { assertThat(execroot.getRelative("unused").resolveSymbolicLinks()).isEqualTo(unusedDir); } + @Test + public void symlinksPlantedExceptProductNamePrefixAndIgnoredPaths() throws Exception { + String productName = getRuntime().getProductName(); + Path execroot = directories.getExecRoot(directories.getWorkspace().getBaseName()); + writeMyRuleBzl(); + Path fooDir = + write( + "foo/BUILD", + "load('//foo:my_rule.bzl', 'my_rule')", + "my_rule(name = 'foo', srcs = ['foo.in'])") + .getParentDirectory(); + write("foo/foo.in"); + Path unusedDir = write("unused/dummy").getParentDirectory(); + write(".bazelignore", "ignored"); + write("ignored/dummy"); + write(productName + "-dir/dummy"); + + // Before the build: no symlink. + assertThat(execroot.getRelative("foo").exists()).isFalse(); + + buildTarget("//foo:foo"); + + // After the build: symlinks to the source directory, even unused packages, except for those + // in the .bazelignore file and those with the bazel- prefix. + assertThat(execroot.getRelative("foo").resolveSymbolicLinks()).isEqualTo(fooDir); + assertThat(execroot.getRelative("unused").resolveSymbolicLinks()).isEqualTo(unusedDir); + assertThat(execroot.getRelative("ignored").exists()).isFalse(); + assertThat(execroot.getRelative(productName + "-dir").exists()).isFalse(); + } + @Test public void symlinksReplantedEachBuild() throws Exception { Path execroot = directories.getExecRoot(directories.getWorkspace().getBaseName());