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

[AOSP-pick] [querysync] Do not scan Bazel system directories for BUILD files #7247

Merged
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
14 changes: 8 additions & 6 deletions base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,14 @@ public QuerySyncProject loadProject(BlazeContext context) throws BuildException
.collect(ImmutableSet.toImmutableSet());

ProjectDefinition latestProjectDef =
ProjectDefinition.create(
importRoots.rootPaths(),
importRoots.excludePaths(),
LanguageClasses.toQuerySync(workspaceLanguageSettings.getActiveLanguages()),
testSourceGlobs,
importRoots.systemExcludes());
ProjectDefinition.builder()
.setProjectIncludes(importRoots.rootPaths())
.setProjectExcludes(importRoots.excludePaths())
.setLanguageClasses(
LanguageClasses.toQuerySync(workspaceLanguageSettings.getActiveLanguages()))
.setTestSources(testSourceGlobs)
.setSystemExcludes(importRoots.systemExcludes())
.build();

Path snapshotFilePath = getSnapshotFilePath(importSettings);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,12 +484,14 @@ public boolean isDefinitionCurrent(BlazeContext context) throws BuildException {
.map(Glob::toString)
.collect(ImmutableSet.toImmutableSet());
ProjectDefinition projectDefinition =
ProjectDefinition.create(
importRoots.rootPaths(),
importRoots.excludePaths(),
LanguageClasses.toQuerySync(workspaceLanguageSettings.getActiveLanguages()),
testSourceGlobs,
importRoots.systemExcludes());
ProjectDefinition.builder()
.setProjectIncludes(importRoots.rootPaths())
.setProjectExcludes(importRoots.excludePaths())
.setLanguageClasses(
LanguageClasses.toQuerySync(workspaceLanguageSettings.getActiveLanguages()))
.setTestSources(testSourceGlobs)
.setSystemExcludes(importRoots.systemExcludes())
.build();

return this.projectDefinition.equals(projectDefinition);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,21 +138,35 @@ public Builder exclude(WorkspacePath entry) {
return this;
}

public ImmutableSet<String> systemExcludes(WorkspaceRoot workspaceRoot) {
var buildArtifactDirectories = BuildSystemProvider
.getBuildSystemProvider(BuildSystemName.Bazel)
.buildArtifactDirectories(workspaceRoot);
var projectDataSubdirectory = new WorkspacePath(BlazeDataStorage.PROJECT_DATA_SUBDIRECTORY).toString();
var systemExcludedDirectories = ImmutableSet.<String>builder().addAll(buildArtifactDirectories).add(projectDataSubdirectory).build();
return systemExcludedDirectories;

/**
* For Bazel projects if the root directories include the workspace root, exclude bazel-bin,
* bazel-out, ... directories to avoid scanning them for Build files by {@link
* com.google.idea.blaze.qsync.project.ProjectDefinition#deriveQuerySpec}
*/
private ImmutableSet<WorkspacePath> getBuildSystemExcludes(
ImmutableCollection<WorkspacePath> rootDirectories) {
if (buildSystemName == BuildSystemName.Bazel && hasWorkspaceRoot(rootDirectories)) {
ImmutableSet<WorkspacePath> buildArtifactDirectories =
BuildSystemProvider.getBuildSystemProvider(buildSystemName)
.buildArtifactDirectories(workspaceRoot)
.stream()
.map(p -> new WorkspacePath(p))
.collect(toImmutableSet());
WorkspacePath projectDataSubdirectory =
new WorkspacePath(BlazeDataStorage.PROJECT_DATA_SUBDIRECTORY);
return ImmutableSet.<WorkspacePath>builder()
.addAll(buildArtifactDirectories)
.add(projectDataSubdirectory)
.build();
}
return ImmutableSet.of();
}

public ImportRoots build() {
if (viewProjectRoot) {
rootDirectoriesBuilder.add(workspaceRoot.workspacePathFor(workspaceRoot.directory()));
}

ImmutableCollection<WorkspacePath> rootDirectories = rootDirectoriesBuilder.build();
if (buildSystemName == BuildSystemName.Bazel) {
if (hasWorkspaceRoot(rootDirectories)) {
Expand All @@ -177,7 +191,7 @@ public ImportRoots build() {
ImmutableSet<WorkspacePath> excludePathsForBazelQuery = Sets.difference(minimalExcludes, bazelIgnorePathsBuilder.build()).immutableCopy();


ImmutableSet<String> systemExcludes = systemExcludes(workspaceRoot);
ImmutableSet<WorkspacePath> systemExcludes = getBuildSystemExcludes(rootDirectories);

ProjectDirectoriesHelper directories =
new ProjectDirectoriesHelper(minimalRootDirectories, minimalExcludes, excludePathsForBazelQuery, systemExcludes);
Expand All @@ -188,7 +202,7 @@ public ImportRoots build() {
projectTargets.build(), directories)
: TargetExpressionList.create(projectTargets.build());

return new ImportRoots(directories, targets);
return new ImportRoots(directories, targets, systemExcludes);
}

private @NotNull List<WorkspacePath> selectExcludes(ImmutableCollection<WorkspacePath> rootDirectories) {
Expand Down Expand Up @@ -236,6 +250,7 @@ private static boolean hasWorkspaceRoot(ImmutableCollection<WorkspacePath> rootD

private final ProjectDirectoriesHelper projectDirectories;
private final TargetExpressionList projectTargets;
private final ImmutableSet<WorkspacePath> buildSystemExcludes;

public static Builder builder(Project project) {
return new Builder(WorkspaceRoot.fromProject(project), Blaze.getBuildSystemName(project));
Expand All @@ -246,9 +261,12 @@ public static Builder builder(WorkspaceRoot workspaceRoot, BuildSystemName build
}

private ImportRoots(
ProjectDirectoriesHelper projectDirectories, TargetExpressionList projectTargets) {
ProjectDirectoriesHelper projectDirectories,
TargetExpressionList projectTargets,
ImmutableSet<WorkspacePath> buildSystemExcludes) {
this.projectDirectories = projectDirectories;
this.projectTargets = projectTargets;
this.buildSystemExcludes = buildSystemExcludes;
}

public Collection<WorkspacePath> rootDirectories() {
Expand All @@ -262,8 +280,9 @@ public ImmutableSet<Path> rootPaths() {
.collect(toImmutableSet());
}

public ImmutableSet<String> systemExcludes() {
return projectDirectories.systemExcludes;
/** Returns the system excluded directories. */
public ImmutableSet<Path> systemExcludes() {
return buildSystemExcludes.stream().map(WorkspacePath::asPath).collect(toImmutableSet());
}

public Set<WorkspacePath> excludeDirectories() {
Expand Down Expand Up @@ -309,11 +328,14 @@ static class ProjectDirectoriesHelper {
private final ImmutableSet<WorkspacePath> rootDirectories;
private final ImmutableSet<WorkspacePath> excludeDirectories;
private final ImmutableSet<WorkspacePath> excludePathsForBazelQuery;
private final ImmutableSet<String> systemExcludes;
private final ImmutableSet<WorkspacePath> systemExcludes;

@VisibleForTesting
ProjectDirectoriesHelper(
Collection<WorkspacePath> rootDirectories, Collection<WorkspacePath> excludeDirectories, Collection<WorkspacePath> excludePathsForBazelQuery, ImmutableSet<String> systemExcludes) {
Collection<WorkspacePath> rootDirectories,
Collection<WorkspacePath> excludeDirectories,
Collection<WorkspacePath> excludePathsForBazelQuery,
ImmutableSet<WorkspacePath> systemExcludes) {
this.rootDirectories = ImmutableSet.copyOf(rootDirectories);
this.excludeDirectories = ImmutableSet.copyOf(excludeDirectories);
this.excludePathsForBazelQuery = ImmutableSet.copyOf(excludePathsForBazelQuery);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,7 @@ public abstract class PostQuerySyncData {
@VisibleForTesting
public static final PostQuerySyncData EMPTY =
builder()
.setProjectDefinition(
ProjectDefinition.create(
/* includes= */ ImmutableSet.of(),
/* excludes= */ ImmutableSet.of(),
/* languageClasses= */ ImmutableSet.of(),
/* testSources= */ ImmutableSet.of(),
/* systemExcludes= */ ImmutableSet.of()))
.setProjectDefinition(ProjectDefinition.EMPTY)
.setVcsState(Optional.empty())
.setBazelVersion(Optional.empty())
.setQuerySummary(QuerySummary.EMPTY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,13 @@
public abstract class ProjectDefinition {

public static final ProjectDefinition EMPTY =
create(
/* includes= */ ImmutableSet.of(),
/* excludes= */ ImmutableSet.of(),
/* languageClasses= */ ImmutableSet.of(),
/* testSources= */ ImmutableSet.of(),
/* systemExcludes = */ ImmutableSet.of());
new AutoValue_ProjectDefinition.Builder()
.setProjectIncludes(ImmutableSet.of())
.setProjectExcludes(ImmutableSet.of())
.setLanguageClasses(ImmutableSet.of())
.setTestSources(ImmutableSet.of())
.setSystemExcludes(ImmutableSet.of())
.build();

/**
* Project includes, also know as root directories. Taken from the users {@code .blazeproject}
Expand All @@ -66,35 +67,52 @@ public abstract class ProjectDefinition {
*/
public abstract ImmutableSet<String> testSources();

public abstract ImmutableSet<String> systemExcludes();
/**
* System Excludes. Only available for Bazel projects to avoid scanning the system directories
* like bazel-bin, bazel-out, ... for BUILD files before ignoring them in the query invocation.
*/
public abstract ImmutableSet<Path> systemExcludes();

public static ProjectDefinition create(
ImmutableSet<Path> includes,
ImmutableSet<Path> excludes,
ImmutableSet<QuerySyncLanguage> languageClasses,
ImmutableSet<String> testSources,
ImmutableSet<String> systemExcludes
) {
return new AutoValue_ProjectDefinition(includes, excludes, languageClasses, testSources, systemExcludes);
public static Builder builder() {
return EMPTY.toBuilder();
}

public abstract Builder toBuilder();

/** Builder for {@link ProjectDefinition}. */
@AutoValue.Builder
public abstract static class Builder {
public abstract Builder setProjectIncludes(ImmutableSet<Path> projectIncludes);

public abstract Builder setProjectExcludes(ImmutableSet<Path> projectExcludes);

public abstract Builder setLanguageClasses(ImmutableSet<QuerySyncLanguage> languageClasses);

public abstract Builder setTestSources(ImmutableSet<String> testSources);

public abstract Builder setSystemExcludes(ImmutableSet<Path> systemExcludes);

public abstract ProjectDefinition build();
}

/**
* Constructs a query spec from a sync spec. Filters the import roots to those that can be safely
* queried.
*/
public QuerySpec.Builder deriveQuerySpec(Context<?> context, QuerySpec.QueryStrategy queryStrategy, Path workspaceRoot) throws IOException {
public QuerySpec.Builder deriveQuerySpec(
Context<?> context, QuerySpec.QueryStrategy queryStrategy, Path workspaceRoot)
throws IOException {
QuerySpec.Builder result = QuerySpec.builder(queryStrategy).workspaceRoot(workspaceRoot);
for (Path include : projectIncludes()) {
if (isValidPathForQuery(context, workspaceRoot.resolve(include))) {
result.includePath(include);
}
}
for (Path exclude : projectExcludes()) {
if(systemExcludes().contains(exclude.toString())){
// We don't have to check if these dirs are valid for queries, because they also don't fall into //... wildcard
continue;
}

if (systemExcludes().contains(exclude)) {
// We don't have to check if these directories are valid for queries
continue;
}
if (isValidPathForQuery(context, workspaceRoot.resolve(exclude))) {
result.excludePath(exclude);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,16 @@ public PostQuerySyncData getSyncData() {

private void visitProjectDefinition(SnapshotProto.ProjectDefinition proto) {
snapshot.setProjectDefinition(
ProjectDefinition.create(
proto.getIncludePathsList().stream().map(Path::of).collect(toImmutableSet()),
proto.getExcludePathsList().stream().map(Path::of).collect(toImmutableSet()),
QuerySyncLanguage.fromProtoList(proto.getLanguageClassesList()),
ImmutableSet.copyOf(proto.getTestSourcesList()),
ImmutableSet.copyOf(proto.getSystemExcludesList())));
ProjectDefinition.builder()
.setProjectIncludes(
proto.getIncludePathsList().stream().map(Path::of).collect(toImmutableSet()))
.setProjectExcludes(
proto.getExcludePathsList().stream().map(Path::of).collect(toImmutableSet()))
.setLanguageClasses(QuerySyncLanguage.fromProtoList(proto.getLanguageClassesList()))
.setTestSources(ImmutableSet.copyOf(proto.getTestSourcesList()))
.setSystemExcludes(
proto.getSystemExcludesList().stream().map(Path::of).collect(toImmutableSet()))
.build());
}

private void visitVcsState(SnapshotProto.VcsState proto) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ message ProjectDefinition {
repeated string exclude_paths = 2;
repeated LanguageClass language_classes = 3;
repeated string test_sources = 4;
// TODO(b/382333575): improve excluding system folders
repeated string system_excludes = 5;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ abstract class GraphToProjectConverters {

public abstract ImmutableSet<String> testSources();

public abstract ImmutableSet<String> systemExcludes();
public abstract ImmutableSet<Path> systemExcludes();

static Builder builder() {
return new AutoValue_GraphToProjectConverters.Builder()
Expand All @@ -70,7 +70,7 @@ abstract static class Builder {

abstract Builder setTestSources(ImmutableSet<String> value);

abstract Builder setSystemExcludes(ImmutableSet<String> value);
abstract Builder setSystemExcludes(ImmutableSet<Path> value);

abstract GraphToProjectConverters autoBuild();

Expand All @@ -80,12 +80,13 @@ public GraphToProjectConverter build() {
info.packageReader(),
info.fileExistenceCheck(),
NOOP_CONTEXT,
ProjectDefinition.create(
info.projectIncludes(),
info.projectExcludes(),
info.languageClasses(),
info.testSources(),
info.systemExcludes()),
ProjectDefinition.builder()
.setProjectIncludes(info.projectIncludes())
.setProjectExcludes(info.projectExcludes())
.setLanguageClasses(info.languageClasses())
.setTestSources(info.testSources())
.setSystemExcludes(info.systemExcludes())
.build(),
newDirectExecutorService());
}
}
Expand Down
Loading
Loading