Skip to content

Commit

Permalink
[Skymeld] Don't plant the symlinks to the paths specified in `.bazeli…
Browse files Browse the repository at this point in the history
…gnore`.

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 (#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.

PiperOrigin-RevId: 581919915
Change-Id: I329365b0655fc0bcb2db1eeea91ef74c775c72ae
  • Loading branch information
joeleba authored and copybara-github committed Nov 13, 2023
1 parent 353000e commit caf1702
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -384,7 +384,11 @@ private static void deleteSiblingRepositorySymlinks(
* @return a set of potentially conflicting baseNames, all in lowercase.
*/
public static ImmutableSet<String> eagerlyPlantSymlinkForestSinglePackagePath(
Path execroot, Path sourceRoot, String prefix, boolean siblingRepositoryLayout)
Path execroot,
Path sourceRoot,
String prefix,
ImmutableSet<Path> ignoredPaths,
boolean siblingRepositoryLayout)
throws IOException {
deleteTreesBelowNotPrefixed(execroot, prefix);
deleteSiblingRepositorySymlinks(siblingRepositoryLayout, execroot);
Expand All @@ -406,7 +410,8 @@ public static ImmutableSet<String> 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 {
Expand All @@ -416,11 +421,22 @@ public static ImmutableSet<String> 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<Path> 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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ public class IncrementalPackageRoots implements PackageRoots {
private final Path execroot;
private final Root singleSourceRoot;
private final String prefix;

private final ImmutableSet<Path> ignoredPaths;
private final boolean useSiblingRepositoryLayout;

private final boolean allowExternalRepositories;
Expand All @@ -80,12 +82,14 @@ private IncrementalPackageRoots(
Root singleSourceRoot,
EventBus eventBus,
String prefix,
ImmutableSet<Path> 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;
Expand All @@ -96,6 +100,7 @@ public static IncrementalPackageRoots createAndRegisterToEventBus(
Root singleSourceRoot,
EventBus eventBus,
String prefix,
ImmutableSet<Path> ignoredPaths,
boolean useSiblingRepositoryLayout,
boolean allowExternalRepositories) {
IncrementalPackageRoots incrementalPackageRoots =
Expand All @@ -104,6 +109,7 @@ public static IncrementalPackageRoots createAndRegisterToEventBus(
singleSourceRoot,
eventBus,
prefix,
ignoredPaths,
useSiblingRepositoryLayout,
allowExternalRepositories);
eventBus.register(incrementalPackageRoots);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -212,19 +222,19 @@ private void registerAndPlantMissingSymlinks(NestedSet<Package> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
@Nullable private final WorkspaceInfoFromDiffReceiver workspaceInfoFromDiffReceiver;
private Set<String> previousClientEnvironment = ImmutableSet.of();

// Contain the paths in the .bazelignore file.
private ImmutableSet<Path> ignoredPaths = ImmutableSet.of();

Duration sourceDiffCheckingDuration = Duration.ofSeconds(-1L);

final class PathResolverFactoryImpl implements PathResolverFactory {
Expand Down Expand Up @@ -1302,6 +1305,10 @@ public AtomicReference<PathPackageLocator> getPackageLocator() {
return pkgLocator;
}

public ImmutableSet<Path> getIgnoredPaths() {
return ignoredPaths;
}

protected Differencer.Diff getDiff(
TimestampGranularityMonitor tsgm,
ModifiedFileSet modifiedFileSet,
Expand Down Expand Up @@ -3144,7 +3151,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<Path> ignoredPaths = ImmutableSet.of();
if (workspacePath != null && workspacePath.equals(pathEntry.asPath())) {
ignoredPaths =
ignoredPackagePrefixesValue.getPatterns().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down

0 comments on commit caf1702

Please sign in to comment.