Skip to content

Commit

Permalink
Cleanups in the wake of 9c6d24f.
Browse files Browse the repository at this point in the history
RELNOTES: None.
PiperOrigin-RevId: 693291109
Change-Id: Ia02b8d7495883e16fc3f021807fbca0015b3ad40
  • Loading branch information
lberki authored and copybara-github committed Nov 5, 2024
1 parent 8ce1661 commit 1f0db43
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,11 @@
import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnFilesystemErrorCodeLoadingBzlFile;
import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;

/** Hardcoded constants describing bazel-on-skyframe behavior. */
public class BazelSkyframeExecutorConstants {
private BazelSkyframeExecutorConstants() {}

/**
* The file .bazelignore can be used to specify directories to be ignored by bazel
*
* <p>This is intended for directories containing non-bazel sources (either generated, or
* versioned sources built by other tools) that happen to contain a file called BUILD.
*
* <p>For the time being, this ignore functionality is limited by the fact that it is applied only
* after pattern expansion. So if a pattern expansion fails (e.g., due to symlink-cycles) and
* therefore fails the build, this ignore functionality currently has no chance to kick in.
*/
public static final SkyFunction IGNORED_PACKAGE_PREFIXES_FUNCTION =
new IgnoredPackagePrefixesFunction(PathFragment.create(".bazelignore"));

public static final CrossRepositoryLabelViolationStrategy
CROSS_REPOSITORY_LABEL_VIOLATION_STRATEGY = CrossRepositoryLabelViolationStrategy.ERROR;

Expand All @@ -63,7 +48,7 @@ private BazelSkyframeExecutorConstants() {}

public static SequencedSkyframeExecutor.Builder newBazelSkyframeExecutorBuilder() {
return SequencedSkyframeExecutor.builder()
.setIgnoredPackagePrefixesFunction(IGNORED_PACKAGE_PREFIXES_FUNCTION)
.setIgnoredPackagePrefixesFunction(IgnoredPackagePrefixesFunction.INSTANCE)
.setActionOnIOExceptionReadingBuildFile(ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE)
.setActionOnFilesystemErrorCodeLoadingBzlFile(
ACTION_ON_FILESYSTEM_ERROR_CODE_LOADING_BZL_FILE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,23 @@
* <p>It is used to compute which directories should be ignored in a package. These either come from
* the {@code .bazelignore} file or from the {@code ignored_directories()} function in {@code
* REPO.bazel}.
*
* <p>This is intended for directories containing non-bazel sources (either generated, or versioned
* sources built by other tools) that happen to contain a file called BUILD.
*
* <p>For the time being, this ignore functionality is limited by the fact that it is applied only
* after pattern expansion. So if a pattern expansion fails (e.g., due to symlink-cycles) and
* therefore fails the build, this ignore functionality currently has no chance to kick in.
*/
public class IgnoredPackagePrefixesFunction implements SkyFunction {
/** Repository-relative path of the bazelignore file. */
public static final PathFragment BAZELIGNORE_REPOSITORY_RELATIVE_PATH =
PathFragment.create(".bazelignore");

/** Singleton instance of this {@link SkyFunction}. */
public static final IgnoredPackagePrefixesFunction INSTANCE =
new IgnoredPackagePrefixesFunction();

/**
* A version of {@link IgnoredPackagePrefixesFunction} that always returns the empty value.
*
Expand All @@ -55,11 +70,7 @@ public class IgnoredPackagePrefixesFunction implements SkyFunction {
*/
public static final SkyFunction NOOP = (skyKey, env) -> IgnoredPackagePrefixesValue.EMPTY;

private final PathFragment ignoredPackagePrefixesFile;

public IgnoredPackagePrefixesFunction(PathFragment ignoredPackagePrefixesFile) {
this.ignoredPackagePrefixesFile = ignoredPackagePrefixesFile;
}
private IgnoredPackagePrefixesFunction() {}

public static void getIgnoredPackagePrefixes(
RootedPath patternFile, ImmutableSet.Builder<PathFragment> ignoredPackagePrefixesBuilder)
Expand Down Expand Up @@ -128,7 +139,7 @@ private ImmutableSet<PathFragment> computeIgnoredPrefixes(
}

RootedPath rootedPatternFile =
RootedPath.toRootedPath(packagePathEntry, ignoredPackagePrefixesFile);
RootedPath.toRootedPath(packagePathEntry, BAZELIGNORE_REPOSITORY_RELATIVE_PATH);
FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile));
if (patternFileValue == null) {
return null;
Expand All @@ -148,7 +159,7 @@ private ImmutableSet<PathFragment> computeIgnoredPrefixes(
if (repositoryValue.repositoryExists()) {
RootedPath rootedPatternFile =
RootedPath.toRootedPath(
Root.fromPath(repositoryValue.getPath()), ignoredPackagePrefixesFile);
Root.fromPath(repositoryValue.getPath()), BAZELIGNORE_REPOSITORY_RELATIVE_PATH);
FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile));
if (patternFileValue == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ public void boundedRdepsWithError() throws Exception {

@Test
public void testIgnoredPackagePrefixIsTBDQuery() throws Exception {
writeFile(helper.getIgnoredPackagePrefixesFile().getPathString(), "a/b");
overwriteFile(helper.getIgnoredPackagePrefixesFile().getPathString(), "a/b");
writeFile("a/BUILD", "filegroup(name = 'a')");
writeFile("a/b/BUILD", "filegroup(name = 'a_b')");
writeFile("a/b/c/BUILD", "filegroup(name = 'a_b_c')");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.runtime.QuiescingExecutorsImpl;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
import com.google.devtools.build.lib.skyframe.IgnoredPackagePrefixesFunction;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyframeTargetPatternEvaluator;
Expand Down Expand Up @@ -116,7 +115,7 @@ public abstract class SkyframeQueryHelper extends AbstractQueryHelper<Target> {
private TargetPatternPreloader targetParser;
protected final ActionKeyContext actionKeyContext = new ActionKeyContext();

private final PathFragment ignoredPackagePrefixesFile = PathFragment.create("ignored");
private final PathFragment ignoredPackagePrefixesFile = PathFragment.create(".bazelignore");
private final DelegatingSyscallCache delegatingSyscallCache = new DelegatingSyscallCache();

@Override
Expand Down Expand Up @@ -424,8 +423,6 @@ protected SkyframeExecutor createSkyframeExecutor(ConfiguredRuleClassProvider ru
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
.setIgnoredPackagePrefixesFunction(
new IgnoredPackagePrefixesFunction(ignoredPackagePrefixesFile))
.setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
.setSyscallCache(delegatingSyscallCache)
.build();
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_state_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:file_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:glob_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:invalid_glob_pattern_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
Expand Down Expand Up @@ -872,7 +873,6 @@ java_test(
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:prepare_deps_of_patterns_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
"//src/main/java/com/google/devtools/build/lib/skyframe:transitive_traversal_value",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,7 @@ private Map<SkyFunctionName, SkyFunction> createFunctionMap() {
new RepoFileFunction(
ruleClassProvider.getBazelStarlarkEnvironment(), directories.getWorkspace()));
skyFunctions.put(
SkyFunctions.IGNORED_PACKAGE_PREFIXES,
BazelSkyframeExecutorConstants.IGNORED_PACKAGE_PREFIXES_FUNCTION);
SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.INSTANCE);
skyFunctions.put(
FileStateKey.FILE_STATE,
new FileStateFunction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase {
private MemoizingEvaluator evaluator;
private RecordingDifferencer differencer;
private Path emptyPackagePath;
private static final String IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING = "config/ignored.txt";

protected abstract CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy();

Expand Down Expand Up @@ -138,9 +137,7 @@ public final void setUp() throws Exception {
new RepoFileFunction(
ruleClassProvider.getBazelStarlarkEnvironment(), directories.getWorkspace()));
skyFunctions.put(
SkyFunctions.IGNORED_PACKAGE_PREFIXES,
new IgnoredPackagePrefixesFunction(
PathFragment.create(IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING)));
SkyFunctions.IGNORED_PACKAGE_PREFIXES, IgnoredPackagePrefixesFunction.INSTANCE);
skyFunctions.put(
WorkspaceFileValue.WORKSPACE_FILE,
new WorkspaceFileFunction(
Expand Down Expand Up @@ -259,7 +256,10 @@ public void testDeletedPackage() throws Exception {
public void testIgnoredPackage() throws Exception {
scratch.file("ignored/subdir/BUILD");
scratch.file("ignored/BUILD");
Path ignored = scratch.overwriteFile(IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING, "ignored");
Path ignored =
scratch.overwriteFile(
IgnoredPackagePrefixesFunction.BAZELIGNORE_REPOSITORY_RELATIVE_PATH.getPathString(),
"ignored");

ImmutableSet<String> pkgs = ImmutableSet.of("ignored/subdir", "ignored");
for (String pkg : pkgs) {
Expand All @@ -269,11 +269,12 @@ public void testIgnoredPackage() throws Exception {
assertThat(packageLookupValue.getErrorMsg()).isNotNull();
}

scratch.overwriteFile(IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING, "not_ignored");
scratch.overwriteFile(
IgnoredPackagePrefixesFunction.BAZELIGNORE_REPOSITORY_RELATIVE_PATH.getPathString(),
"not_ignored");
RootedPath rootedIgnoreFile =
RootedPath.toRootedPath(
Root.fromPath(ignored.getParentDirectory().getParentDirectory()),
PathFragment.create("config/ignored.txt"));
root, IgnoredPackagePrefixesFunction.BAZELIGNORE_REPOSITORY_RELATIVE_PATH);
differencer.invalidate(ImmutableSet.of(FileStateValue.key(rootedIgnoreFile)));
for (String pkg : pkgs) {
PackageLookupValue packageLookupValue = lookupPackage(pkg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@
@RunWith(JUnit4.class)
public class PrepareDepsOfPatternsFunctionSmartNegationTest extends FoundationTestCase {
private SkyframeExecutor skyframeExecutor;
private static final String ADDITIONAL_IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING =
"config/ignored.txt";

private static SkyKey getKeyForLabel(Label label) {
// Note that these tests used to look for TargetMarker SkyKeys before TargetMarker was
Expand Down Expand Up @@ -95,9 +93,6 @@ public void setUp() throws Exception {
.setActionKeyContext(new ActionKeyContext())
.setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
.setSyscallCache(SyscallCache.NO_CACHE)
.setIgnoredPackagePrefixesFunction(
new IgnoredPackagePrefixesFunction(
PathFragment.create(ADDITIONAL_IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING)))
.build();
SkyframeExecutorTestHelper.process(skyframeExecutor);
OptionsParser optionsParser =
Expand All @@ -116,7 +111,7 @@ public void setUp() throws Exception {
new TimestampGranularityMonitor(null));
skyframeExecutor.setActionEnv(ImmutableMap.of());
skyframeExecutor.injectExtraPrecomputedValues(analysisMock.getPrecomputedValues());
scratch.file(ADDITIONAL_IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING);
scratch.file(".bazelignore");
}

@Test
Expand Down Expand Up @@ -161,7 +156,7 @@ public void testIgnoredPatternBlocksPatternEvaluation() throws Exception {
ImmutableList<String> patternSequence = ImmutableList.of("//foo/...");

// and an ignored entry for the malformed package,
scratch.overwriteFile(ADDITIONAL_IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING, "foo/foo");
scratch.overwriteFile(".bazelignore", "foo/foo");

assertSkipsFoo(patternSequence);
}
Expand Down

0 comments on commit 1f0db43

Please sign in to comment.