Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support .bazelignore file for external repo #10261

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ public RepositoryDirectoryValue.Builder fetch(
}

BlacklistedPackagePrefixesValue blacklistedPackagesValue =
(BlacklistedPackagePrefixesValue) env.getValue(BlacklistedPackagePrefixesValue.key());
(BlacklistedPackagePrefixesValue)
env.getValue(
BlacklistedPackagePrefixesValue.key());

if (env.valuesMissing()) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@ public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() {
throw new IllegalStateException();
}

/** Returns the repository name of the target path. */
public abstract RepositoryName getRepository();

/**
* Returns {@code true} iff this pattern has type {@code Type.TARGETS_BELOW_DIRECTORY} or
* {@code Type.TARGETS_IN_PACKAGE} and the target pattern suffix specified it should match
Expand Down Expand Up @@ -339,6 +342,11 @@ public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() {
return directory;
}

@Override
public RepositoryName getRepository() {
return directory.getRepository();
}

@Override
public boolean getRulesOnly() {
return false;
Expand Down Expand Up @@ -418,6 +426,11 @@ public String getPathForPathAsTarget() {
return path;
}

@Override
public RepositoryName getRepository() {
return RepositoryName.MAIN;
}

@Override
public boolean getRulesOnly() {
return false;
Expand Down Expand Up @@ -490,6 +503,11 @@ public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() {
return packageIdentifier;
}

@Override
public RepositoryName getRepository() {
return packageIdentifier.getRepository();
}

@Override
public boolean getRulesOnly() {
return rulesOnly;
Expand Down Expand Up @@ -621,6 +639,11 @@ public PackageIdentifier getDirectoryForTargetsUnderDirectory() {
return directory;
}

@Override
public RepositoryName getRepository() {
return directory.getRepository();
}

@Override
public boolean getRulesOnly() {
return rulesOnly;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
import com.google.common.io.LineProcessor;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.InconsistentFilesystemException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
Expand Down Expand Up @@ -46,10 +48,33 @@ public BlacklistedPackagePrefixesFunction(
this.additionalBlacklistedPackagePrefixesFile = additionalBlacklistedPackagePrefixesFile;
}

private static void getBlacklistedPackagePrefixes(
RootedPath patternFile, ImmutableSet.Builder<PathFragment> blacklistedPackagePrefixesBuilder)
throws BlacklistedPatternsFunctionException {
try (InputStreamReader reader =
new InputStreamReader(patternFile.asPath().getInputStream(), StandardCharsets.UTF_8)) {
blacklistedPackagePrefixesBuilder.addAll(
CharStreams.readLines(reader, new PathFragmentLineProcessor()));
} catch (IOException e) {
String errorMessage = e.getMessage() != null ? "error '" + e.getMessage() + "'" : "an error";
throw new BlacklistedPatternsFunctionException(
new InconsistentFilesystemException(
patternFile.asPath()
+ " is not readable because: "
+ errorMessage
+ ". Was it modified mid-build?"));
}
}

@Nullable
@Override
public SkyValue compute(SkyKey key, Environment env)
throws SkyFunctionException, InterruptedException {
RepositoryName repositoryName = null;
if (key.argument() != null && key.argument() instanceof RepositoryName) {
repositoryName = (RepositoryName) key.argument();
}

ImmutableSet.Builder<PathFragment> blacklistedPackagePrefixesBuilder = ImmutableSet.builder();

blacklistedPackagePrefixesBuilder.addAll(hardcodedBlacklistedPackagePrefixes);
Expand All @@ -60,29 +85,37 @@ public SkyValue compute(SkyKey key, Environment env)
return null;
}

for (Root packagePathEntry : pkgLocator.getPathEntries()) {
RootedPath rootedPatternFile =
RootedPath.toRootedPath(packagePathEntry, additionalBlacklistedPackagePrefixesFile);
FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile));
if (patternFileValue == null) {
if (repositoryName == null || repositoryName.isMain()) {
for (Root packagePathEntry : pkgLocator.getPathEntries()) {
RootedPath rootedPatternFile =
RootedPath.toRootedPath(packagePathEntry, additionalBlacklistedPackagePrefixesFile);
FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile));
if (patternFileValue == null) {
return null;
}
if (patternFileValue.isFile()) {
getBlacklistedPackagePrefixes(rootedPatternFile, blacklistedPackagePrefixesBuilder);
break;
}
}
} else {
// Make sure the repository is fetched.
RepositoryDirectoryValue repositoryValue =
(RepositoryDirectoryValue) env.getValue(RepositoryDirectoryValue.key(repositoryName));
if (repositoryValue == null) {
return null;
}
if (patternFileValue.isFile()) {
try {
try (InputStreamReader reader =
new InputStreamReader(rootedPatternFile.asPath().getInputStream(),
StandardCharsets.UTF_8)) {
blacklistedPackagePrefixesBuilder.addAll(
CharStreams.readLines(reader, new PathFragmentLineProcessor()));
break;
}
} catch (IOException e) {
String errorMessage = e.getMessage() != null
? "error '" + e.getMessage() + "'" : "an error";
throw new BlacklistedPatternsFunctionException(
new InconsistentFilesystemException(
rootedPatternFile.asPath() + " is not readable because: " + errorMessage
+ ". Was it modified mid-build?"));
if (repositoryValue.repositoryExists()) {
RootedPath rootedPatternFile =
RootedPath.toRootedPath(
Root.fromPath(repositoryValue.getPath()),
additionalBlacklistedPackagePrefixesFile);
FileValue patternFileValue = (FileValue) env.getValue(FileValue.key(rootedPatternFile));
if (patternFileValue == null) {
return null;
}
if (patternFileValue.isFile()) {
getBlacklistedPackagePrefixes(rootedPatternFile, blacklistedPackagePrefixesBuilder);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Interner;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.AbstractSkyKey;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;

Expand All @@ -32,10 +37,20 @@ public BlacklistedPackagePrefixesValue(ImmutableSet<PathFragment> patterns) {
this.patterns = Preconditions.checkNotNull(patterns);
}

/** Creates a key from the main repository. */
public static SkyKey key() {
return BLACKLIST_KEY;
}

/** Creates a key from the given repository name. */
public static SkyKey key(RepositoryName repository) {
if (repository.isMain()) {
return BLACKLIST_KEY;
} else {
return Key.create(repository);
}
}

public ImmutableSet<PathFragment> getPatterns() {
return patterns;
}
Expand All @@ -53,4 +68,25 @@ public boolean equals(Object obj) {
}
return false;
}

@AutoCodec.VisibleForSerialization
@AutoCodec
static class Key extends AbstractSkyKey<RepositoryName> {
private static final Interner<Key> interner = BlazeInterners.newWeakInterner();

private Key(RepositoryName arg) {
super(arg);
}

@AutoCodec.VisibleForSerialization
@AutoCodec.Instantiator
static Key create(RepositoryName arg) {
return interner.intern(new Key(arg));
}

@Override
public SkyFunctionName functionName() {
return SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.InconsistentFilesystemException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.vfs.Dirent;
Expand Down Expand Up @@ -56,8 +57,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
throws GlobFunctionException, InterruptedException {
GlobDescriptor glob = (GlobDescriptor) skyKey.argument();

RepositoryName repositoryName = glob.getPackageId().getRepository();
BlacklistedPackagePrefixesValue blacklistedPackagePrefixes =
(BlacklistedPackagePrefixesValue) env.getValue(BlacklistedPackagePrefixesValue.key());
(BlacklistedPackagePrefixesValue)
env.getValue(BlacklistedPackagePrefixesValue.key(repositoryName));
if (env.valuesMissing()) {
return null;
}
Expand All @@ -79,7 +82,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
env.getValue(
PackageLookupValue.key(
PackageIdentifier.create(
glob.getPackageId().getRepository(),
repositoryName,
glob.getPackageId().getPackageFragment().getRelative(globSubdir))));
if (globSubdirPkgLookupValue == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,8 @@ public SkyValue compute(SkyKey key, Environment env) throws PackageFunctionExcep
RuleVisibility defaultVisibility = PrecomputedValue.DEFAULT_VISIBILITY.get(env);
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
BlacklistedPackagePrefixesValue blacklistedPackagePrefixes =
(BlacklistedPackagePrefixesValue) env.getValue(BlacklistedPackagePrefixesValue.key());
(BlacklistedPackagePrefixesValue)
env.getValue(BlacklistedPackagePrefixesValue.key(packageId.getRepository()));
if (env.valuesMissing()) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,15 @@ public SkyValue compute(SkyKey skyKey, Environment env)
return computeWorkspacePackageLookupValue(env, pkgLocator.getPathEntries());
}

// Check .bazelignore file under main repository.
BlacklistedPackagePrefixesValue blacklistedPatternsValue =
(BlacklistedPackagePrefixesValue) env.getValue(BlacklistedPackagePrefixesValue.key());
if (blacklistedPatternsValue == null) {
return null;
}

PathFragment buildFileFragment = packageKey.getPackageFragment();
for (PathFragment pattern : blacklistedPatternsValue.getPatterns()) {
if (buildFileFragment.startsWith(pattern)) {
return PackageLookupValue.DELETED_PACKAGE_VALUE;
}
if (isPackageIgnored(packageKey, blacklistedPatternsValue)) {
return PackageLookupValue.DELETED_PACKAGE_VALUE;
}

return findPackageByBuildFile(env, pkgLocator, packageKey);
Expand Down Expand Up @@ -292,6 +290,17 @@ private PackageLookupValue getPackageLookupValue(
return PackageLookupValue.NO_BUILD_FILE_VALUE;
}

private static boolean isPackageIgnored(
PackageIdentifier id, BlacklistedPackagePrefixesValue blacklistedPatternsValue) {
PathFragment packageFragment = id.getPackageFragment();
for (PathFragment pattern : blacklistedPatternsValue.getPatterns()) {
if (packageFragment.startsWith(pattern)) {
return true;
}
}
return false;
}

private PackageLookupValue computeWorkspacePackageLookupValue(
Environment env, ImmutableList<Root> packagePathEntries)
throws PackageLookupFunctionException, InterruptedException {
Expand Down Expand Up @@ -369,6 +378,18 @@ private PackageLookupValue computeExternalPackageLookupValue(
return new PackageLookupValue.NoRepositoryPackageLookupValue(id.getRepository().getName());
}

// Check .bazelignore file after fetching the external repository.
BlacklistedPackagePrefixesValue blacklistedPatternsValue =
(BlacklistedPackagePrefixesValue)
env.getValue(BlacklistedPackagePrefixesValue.key(id.getRepository()));
if (blacklistedPatternsValue == null) {
return null;
}

if (isPackageIgnored(id, blacklistedPatternsValue)) {
return PackageLookupValue.DELETED_PACKAGE_VALUE;
}

// This checks for the build file names in the correct precedence order.
for (BuildFileName buildFileName : buildFilesByPriority) {
PathFragment buildFileFragment =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,20 @@ public SkyValue compute(SkyKey key, Environment env)
TargetPattern parsedPattern = patternKey.getParsedPattern();

BlacklistedPackagePrefixesValue blacklist =
(BlacklistedPackagePrefixesValue) env.getValue(BlacklistedPackagePrefixesValue.key());
(BlacklistedPackagePrefixesValue)
env.getValue(BlacklistedPackagePrefixesValue.key(parsedPattern.getRepository()));
if (blacklist == null) {
return null;
}
ImmutableSet<PathFragment> blacklistedPatterns = blacklist.getPatterns();

// This SkyFunction is used to load the universe, so we want both the blacklisted directories
// from the global blacklist and the excluded directories from the TargetPatternKey itself to be
// embedded in the SkyKeys created and used by the DepsOfPatternPreparer. The
// DepsOfPatternPreparer ignores excludedSubdirectories and embeds blacklistedSubdirectories in
// the SkyKeys it creates and uses.
ImmutableSet<PathFragment> blacklistedSubdirectories =
patternKey.getAllSubdirectoriesToExclude(blacklist.getPatterns());
patternKey.getAllSubdirectoriesToExclude(blacklistedPatterns);
ImmutableSet<PathFragment> excludedSubdirectories = ImmutableSet.of();

DepsOfPatternPreparer preparer =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,9 +505,7 @@ private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions(PackageFactory p
SkyFunctions.COLLECT_PACKAGES_UNDER_DIRECTORY,
newCollectPackagesUnderDirectoryFunction(directories));
map.put(
SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES,
new BlacklistedPackagePrefixesFunction(
hardcodedBlacklistedPackagePrefixes, additionalBlacklistedPackagePrefixesFile));
SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES, newBlacklistedPackagePrefixesFunction());
map.put(SkyFunctions.TESTS_IN_SUITE, new TestExpansionFunction());
map.put(SkyFunctions.TEST_SUITE_EXPANSION, new TestsForTargetPatternFunction());
map.put(SkyFunctions.TARGET_PATTERN_PHASE, new TargetPatternPhaseFunction());
Expand Down Expand Up @@ -636,6 +634,11 @@ protected SkyFunction newGlobFunction() {
return new GlobFunction(/*alwaysUseDirListing=*/ false);
}

protected SkyFunction newBlacklistedPackagePrefixesFunction() {
return new BlacklistedPackagePrefixesFunction(
hardcodedBlacklistedPackagePrefixes, additionalBlacklistedPackagePrefixesFile);
}

protected SkyFunction newCollectPackagesUnderDirectoryFunction(BlazeDirectories directories) {
return new CollectPackagesUnderDirectoryFunction(directories);
}
Expand Down
Loading