From fb7e43e9d157a7fce4dffd0af93cc3a5d34afc39 Mon Sep 17 00:00:00 2001 From: Tomasz Pasternak Date: Fri, 4 Oct 2024 08:13:24 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20Query=20Sync:=20don't=20check=20for=20va?= =?UTF-8?q?lidity=20of=20excludes=20that=20are=20system=E2=80=A6=20(#6841)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Query Sync: don't check for validity of excludes that are system-excludes (bazel-bin, .ijwb, etc.) * Fix tests * Fix tests * Fix tests * Fix tests --- .../android/libraries/RenderJarCacheTest.java | 4 ++++ .../idea/blaze/base/qsync/ProjectLoader.java | 5 +++- .../blaze/base/qsync/QuerySyncProject.java | 4 +++- .../base/sync/projectview/ImportRoots.java | 23 +++++++++++++++++-- .../projectview/TargetExpressionListTest.java | 10 +++++--- .../qsync/project/PostQuerySyncData.java | 3 ++- .../qsync/project/ProjectDefinition.java | 16 ++++++++++--- .../qsync/project/SnapshotDeserializer.java | 3 ++- .../idea/blaze/qsync/project/snapshot.proto | 1 + .../blaze/qsync/GraphToProjectConverters.java | 8 ++++++- .../idea/blaze/qsync/TestDataSyncRunner.java | 4 +++- 11 files changed, 67 insertions(+), 14 deletions(-) diff --git a/aswb/tests/unittests/com/google/idea/blaze/android/libraries/RenderJarCacheTest.java b/aswb/tests/unittests/com/google/idea/blaze/android/libraries/RenderJarCacheTest.java index 77a676c5e38..0359f63c42d 100644 --- a/aswb/tests/unittests/com/google/idea/blaze/android/libraries/RenderJarCacheTest.java +++ b/aswb/tests/unittests/com/google/idea/blaze/android/libraries/RenderJarCacheTest.java @@ -26,6 +26,8 @@ import com.google.idea.blaze.base.MockProjectViewManager; import com.google.idea.blaze.base.async.executor.BlazeExecutor; import com.google.idea.blaze.base.async.executor.MockBlazeExecutor; +import com.google.idea.blaze.base.bazel.BazelBuildSystemProvider; +import com.google.idea.blaze.base.bazel.BuildSystemProvider; import com.google.idea.blaze.base.command.buildresult.LocalFileOutputArtifactWithoutDigest; import com.google.idea.blaze.base.filecache.FileCache; import com.google.idea.blaze.base.ideinfo.AndroidIdeInfo; @@ -153,6 +155,8 @@ public File decode(ArtifactLocation artifactLocation) { intellijRule.registerApplicationService(BlazeExecutor.class, new MockBlazeExecutor()); + intellijRule.registerExtensionPoint(BuildSystemProvider.EP_NAME, BuildSystemProvider.class); + intellijRule.registerExtension(BuildSystemProvider.EP_NAME, new BazelBuildSystemProvider()); setupProjectData(); setProjectView( "directories:", diff --git a/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java b/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java index 4faa46f3fb1..9b09bb0aeec 100644 --- a/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java +++ b/base/src/com/google/idea/blaze/base/qsync/ProjectLoader.java @@ -23,6 +23,7 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.idea.blaze.base.bazel.BuildSystem; import com.google.idea.blaze.base.bazel.BuildSystemProvider; +import com.google.idea.blaze.base.model.primitives.WorkspacePath; import com.google.idea.blaze.base.model.primitives.WorkspaceRoot; import com.google.idea.blaze.base.projectview.ProjectViewManager; import com.google.idea.blaze.base.projectview.ProjectViewSet; @@ -34,6 +35,7 @@ import com.google.idea.blaze.base.scope.BlazeContext; import com.google.idea.blaze.base.settings.BlazeImportSettings; import com.google.idea.blaze.base.settings.BlazeImportSettingsManager; +import com.google.idea.blaze.base.settings.BuildSystemName; import com.google.idea.blaze.base.sync.data.BlazeDataStorage; import com.google.idea.blaze.base.sync.projectview.ImportRoots; import com.google.idea.blaze.base.sync.projectview.LanguageSupport; @@ -122,7 +124,8 @@ public QuerySyncProject loadProject(BlazeContext context) throws BuildException importRoots.rootPaths(), importRoots.excludePaths(), LanguageClasses.toQuerySync(workspaceLanguageSettings.getActiveLanguages()), - testSourceGlobs); + testSourceGlobs, + importRoots.systemExcludes()); Path snapshotFilePath = getSnapshotFilePath(importSettings); diff --git a/base/src/com/google/idea/blaze/base/qsync/QuerySyncProject.java b/base/src/com/google/idea/blaze/base/qsync/QuerySyncProject.java index 1b3e35fd581..37bc080e8b3 100644 --- a/base/src/com/google/idea/blaze/base/qsync/QuerySyncProject.java +++ b/base/src/com/google/idea/blaze/base/qsync/QuerySyncProject.java @@ -440,12 +440,14 @@ public boolean isDefinitionCurrent(BlazeContext context) throws BuildException { projectViewSet.listItems(TestSourceSection.KEY).stream() .map(Glob::toString) .collect(ImmutableSet.toImmutableSet()); + ProjectDefinition projectDefinition = ProjectDefinition.create( importRoots.rootPaths(), importRoots.excludePaths(), LanguageClasses.toQuerySync(workspaceLanguageSettings.getActiveLanguages()), - testSourceGlobs); + testSourceGlobs, + importRoots.systemExcludes()); return this.projectDefinition.equals(projectDefinition); } diff --git a/base/src/com/google/idea/blaze/base/sync/projectview/ImportRoots.java b/base/src/com/google/idea/blaze/base/sync/projectview/ImportRoots.java index c8fdef5e933..4dbd8320c3c 100644 --- a/base/src/com/google/idea/blaze/base/sync/projectview/ImportRoots.java +++ b/base/src/com/google/idea/blaze/base/sync/projectview/ImportRoots.java @@ -138,6 +138,16 @@ public Builder exclude(WorkspacePath entry) { return this; } + public ImmutableSet systemExcludes(WorkspaceRoot workspaceRoot) { + var buildArtifactDirectories = BuildSystemProvider + .getBuildSystemProvider(BuildSystemName.Bazel) + .buildArtifactDirectories(workspaceRoot); + var projectDataSubdirectory = new WorkspacePath(BlazeDataStorage.PROJECT_DATA_SUBDIRECTORY).toString(); + var systemExcludedDirectories = ImmutableSet.builder().addAll(buildArtifactDirectories).add(projectDataSubdirectory).build(); + return systemExcludedDirectories; + + } + public ImportRoots build() { if (viewProjectRoot) { rootDirectoriesBuilder.add(workspaceRoot.workspacePathFor(workspaceRoot.directory())); @@ -166,8 +176,11 @@ public ImportRoots build() { //Paths in .bazelignore are already excluded by Bazel, excluding them explicitly in "bazel query" produces a warning ImmutableSet excludePathsForBazelQuery = Sets.difference(minimalExcludes, bazelIgnorePathsBuilder.build()).immutableCopy(); + + ImmutableSet systemExcludes = systemExcludes(workspaceRoot); + ProjectDirectoriesHelper directories = - new ProjectDirectoriesHelper(minimalRootDirectories, minimalExcludes, excludePathsForBazelQuery); + new ProjectDirectoriesHelper(minimalRootDirectories, minimalExcludes, excludePathsForBazelQuery, systemExcludes); TargetExpressionList targets = deriveTargetsFromDirectories @@ -248,6 +261,10 @@ public ImmutableSet rootPaths() { .collect(toImmutableSet()); } + public ImmutableSet systemExcludes() { + return projectDirectories.systemExcludes; + } + public Set excludeDirectories() { return projectDirectories.excludeDirectories; } @@ -291,13 +308,15 @@ static class ProjectDirectoriesHelper { private final ImmutableSet rootDirectories; private final ImmutableSet excludeDirectories; private final ImmutableSet excludePathsForBazelQuery; + private final ImmutableSet systemExcludes; @VisibleForTesting ProjectDirectoriesHelper( - Collection rootDirectories, Collection excludeDirectories, Collection excludePathsForBazelQuery) { + Collection rootDirectories, Collection excludeDirectories, Collection excludePathsForBazelQuery, ImmutableSet systemExcludes) { this.rootDirectories = ImmutableSet.copyOf(rootDirectories); this.excludeDirectories = ImmutableSet.copyOf(excludeDirectories); this.excludePathsForBazelQuery = ImmutableSet.copyOf(excludePathsForBazelQuery); + this.systemExcludes = systemExcludes; } boolean containsWorkspacePath(WorkspacePath workspacePath) { diff --git a/base/tests/unittests/com/google/idea/blaze/base/sync/projectview/TargetExpressionListTest.java b/base/tests/unittests/com/google/idea/blaze/base/sync/projectview/TargetExpressionListTest.java index 243f9546275..8400600f479 100644 --- a/base/tests/unittests/com/google/idea/blaze/base/sync/projectview/TargetExpressionListTest.java +++ b/base/tests/unittests/com/google/idea/blaze/base/sync/projectview/TargetExpressionListTest.java @@ -182,7 +182,9 @@ public void testInferringTargetsFromDirectoriesSimple() throws Exception { new ProjectDirectoriesHelper( /* rootDirectories= */ ImmutableList.of(new WorkspacePath("foo")), /* excludeDirectories= */ ImmutableSet.of(), - /* excludePathsForBazelQuery= */ ImmutableSet.of() + /* excludePathsForBazelQuery= */ ImmutableSet.of(), + ImmutableSet.of() + )); assertThat(helper.includesTarget(Label.create("//foo:target"))).isTrue(); @@ -203,7 +205,8 @@ public void testExplictTargetsOverrideInferredTargets() throws Exception { new ProjectDirectoriesHelper( /* rootDirectories= */ ImmutableList.of(new WorkspacePath("foo")), /* excludeDirectories= */ ImmutableSet.of(new WorkspacePath("bar")), - /* excludePathsForBazelQuery= */ ImmutableSet.of() + /* excludePathsForBazelQuery= */ ImmutableSet.of(), + ImmutableSet.of() )); assertThat(helper.includesTarget(Label.create("//foo:target"))).isFalse(); @@ -224,7 +227,8 @@ public void testInferredTargetsWithExcludedSubdirectories() { new ProjectDirectoriesHelper( /* rootDirectories= */ ImmutableList.of(new WorkspacePath("foo")), /* excludeDirectories= */ ImmutableSet.of(new WorkspacePath("foo/bar")), - /* excludePathsForBazelQuery= */ ImmutableSet.of() + /* excludePathsForBazelQuery= */ ImmutableSet.of(), + ImmutableSet.of() )); assertThat(helper.includesTarget(Label.create("//foo:target"))).isTrue(); diff --git a/querysync/java/com/google/idea/blaze/qsync/project/PostQuerySyncData.java b/querysync/java/com/google/idea/blaze/qsync/project/PostQuerySyncData.java index 711e5d30909..dd51b3be188 100644 --- a/querysync/java/com/google/idea/blaze/qsync/project/PostQuerySyncData.java +++ b/querysync/java/com/google/idea/blaze/qsync/project/PostQuerySyncData.java @@ -42,7 +42,8 @@ public abstract class PostQuerySyncData { /* includes= */ ImmutableSet.of(), /* excludes= */ ImmutableSet.of(), /* languageClasses= */ ImmutableSet.of(), - /* testSources= */ ImmutableSet.of())) + /* testSources= */ ImmutableSet.of(), + /* systemExcludes = */ ImmutableSet.of())) .setVcsState(Optional.empty()) .setBazelVersion(Optional.empty()) .setQuerySummary(QuerySummary.EMPTY) diff --git a/querysync/java/com/google/idea/blaze/qsync/project/ProjectDefinition.java b/querysync/java/com/google/idea/blaze/qsync/project/ProjectDefinition.java index e016ef19388..21542d36703 100644 --- a/querysync/java/com/google/idea/blaze/qsync/project/ProjectDefinition.java +++ b/querysync/java/com/google/idea/blaze/qsync/project/ProjectDefinition.java @@ -42,7 +42,8 @@ public abstract class ProjectDefinition { /* includes= */ ImmutableSet.of(), /* excludes= */ ImmutableSet.of(), /* languageClasses= */ ImmutableSet.of(), - /* testSources= */ ImmutableSet.of()); + /* testSources= */ ImmutableSet.of(), + /* systemExcludes = */ ImmutableSet.of()); /** * Project includes, also know as root directories. Taken from the users {@code .blazeproject} @@ -65,12 +66,16 @@ public abstract class ProjectDefinition { */ public abstract ImmutableSet testSources(); + public abstract ImmutableSet systemExcludes(); + public static ProjectDefinition create( ImmutableSet includes, ImmutableSet excludes, ImmutableSet languageClasses, - ImmutableSet testSources) { - return new AutoValue_ProjectDefinition(includes, excludes, languageClasses, testSources); + ImmutableSet testSources, + ImmutableSet systemExcludes + ) { + return new AutoValue_ProjectDefinition(includes, excludes, languageClasses, testSources, systemExcludes); } /** @@ -85,6 +90,11 @@ public QuerySpec deriveQuerySpec(Context context, Path workspaceRoot) throws } } 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 (isValidPathForQuery(context, workspaceRoot.resolve(exclude))) { result.excludePath(exclude); } diff --git a/querysync/java/com/google/idea/blaze/qsync/project/SnapshotDeserializer.java b/querysync/java/com/google/idea/blaze/qsync/project/SnapshotDeserializer.java index 10379e5f64f..78ae6d7e0e6 100644 --- a/querysync/java/com/google/idea/blaze/qsync/project/SnapshotDeserializer.java +++ b/querysync/java/com/google/idea/blaze/qsync/project/SnapshotDeserializer.java @@ -74,7 +74,8 @@ private void visitProjectDefinition(SnapshotProto.ProjectDefinition proto) { 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.getTestSourcesList()), + ImmutableSet.copyOf(proto.getSystemExcludesList()))); } private void visitVcsState(SnapshotProto.VcsState proto) { diff --git a/querysync/java/com/google/idea/blaze/qsync/project/snapshot.proto b/querysync/java/com/google/idea/blaze/qsync/project/snapshot.proto index fde699ff0f0..d0c40b8a2ed 100644 --- a/querysync/java/com/google/idea/blaze/qsync/project/snapshot.proto +++ b/querysync/java/com/google/idea/blaze/qsync/project/snapshot.proto @@ -22,6 +22,7 @@ message ProjectDefinition { repeated string exclude_paths = 2; repeated LanguageClass language_classes = 3; repeated string test_sources = 4; + repeated string system_excludes = 5; } message VcsState { diff --git a/querysync/javatests/com/google/idea/blaze/qsync/GraphToProjectConverters.java b/querysync/javatests/com/google/idea/blaze/qsync/GraphToProjectConverters.java index acd635ca63f..900737647e6 100644 --- a/querysync/javatests/com/google/idea/blaze/qsync/GraphToProjectConverters.java +++ b/querysync/javatests/com/google/idea/blaze/qsync/GraphToProjectConverters.java @@ -43,6 +43,8 @@ abstract class GraphToProjectConverters { public abstract ImmutableSet testSources(); + public abstract ImmutableSet systemExcludes(); + static Builder builder() { return new AutoValue_GraphToProjectConverters.Builder() .setPackageReader(EMPTY_PACKAGE_READER) @@ -50,6 +52,7 @@ static Builder builder() { .setLanguageClasses(ImmutableSet.of()) .setProjectIncludes(ImmutableSet.of()) .setProjectExcludes(ImmutableSet.of()) + .setSystemExcludes(ImmutableSet.of()) .setTestSources(ImmutableSet.of()); } @@ -67,6 +70,8 @@ abstract static class Builder { abstract Builder setTestSources(ImmutableSet value); + abstract Builder setSystemExcludes(ImmutableSet value); + abstract GraphToProjectConverters autoBuild(); public GraphToProjectConverter build() { @@ -79,7 +84,8 @@ public GraphToProjectConverter build() { info.projectIncludes(), info.projectExcludes(), info.languageClasses(), - info.testSources()), + info.testSources(), + info.systemExcludes()), newDirectExecutorService(), false); } diff --git a/querysync/javatests/com/google/idea/blaze/qsync/TestDataSyncRunner.java b/querysync/javatests/com/google/idea/blaze/qsync/TestDataSyncRunner.java index 4a74415da04..6efb14a4a3d 100644 --- a/querysync/javatests/com/google/idea/blaze/qsync/TestDataSyncRunner.java +++ b/querysync/javatests/com/google/idea/blaze/qsync/TestDataSyncRunner.java @@ -56,7 +56,9 @@ public BlazeProjectSnapshot sync(TestData testProject) throws IOException, Build /* includes= */ ImmutableSet.copyOf(testProject.getRelativeSourcePaths()), /* excludes= */ ImmutableSet.of(), /* languageClasses= */ ImmutableSet.of(), - /* testSources= */ ImmutableSet.of()); + /* testSources= */ ImmutableSet.of(), + /* systemExcludes = */ ImmutableSet.of() +); QuerySummary querySummary = getQuerySummary(testProject); PostQuerySyncData pqsd = PostQuerySyncData.builder()