Skip to content

Commit

Permalink
[7.0.0] [Skymeld] Don't plant the symlinks to the paths specified in (#…
Browse files Browse the repository at this point in the history
…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
(#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
caf1702

PiperOrigin-RevId: 581919915
Change-Id: I329365b0655fc0bcb2db1eeea91ef74c775c72ae

Co-authored-by: Googler <[email protected]>
  • Loading branch information
bazel-io and joeleba authored Nov 13, 2023
1 parent 8b211fb commit e1fbc83
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 @@ -442,6 +442,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 @@ -1295,6 +1298,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 @@ -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<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 e1fbc83

Please sign in to comment.