Skip to content

Commit

Permalink
Fix manual targets sync for target explicitly specified (#5458)
Browse files Browse the repository at this point in the history 
* Check to see if we can exclude manual tag based on allow_manual_targets_sync: true flag

The shard_sync: true flag must be set to expand the targets.

* Enable shard sync when we also specify manual targets
  • Loading branch information
@rogerhu
rogerhu authored Oct 18, 2023
1 parent 3f4d317 commit 8532fe6
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 12 deletions.
2 changes: 1 addition & 1 deletion base/src/com/google/idea/blaze/base/sync/SyncProjectTargetsHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ private static boolean shouldDeriveSyncTargetsFromDirectories(ProjectViewSet vie
return viewSet.getScalarValue(AutomaticallyDeriveTargetsSection.KEY).orElse(false);
}

private static boolean shouldSyncManualTargets(ProjectViewSet viewSet) {
public static boolean shouldSyncManualTargets(ProjectViewSet viewSet) {
return viewSet.getScalarValue(SyncManualTargetsSection.KEY).orElse(false);
}

Expand Down
5 changes: 4 additions & 1 deletion base/src/com/google/idea/blaze/base/sync/sharding/BlazeBuildTargetSharder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.idea.blaze.base.projectview.ProjectViewManager;
import com.google.idea.blaze.base.projectview.ProjectViewSet;
import com.google.idea.blaze.base.projectview.section.sections.ShardBlazeBuildsSection;
import com.google.idea.blaze.base.projectview.section.sections.SyncManualTargetsSection;
import com.google.idea.blaze.base.projectview.section.sections.TargetShardSizeSection;
import com.google.idea.blaze.base.scope.BlazeContext;
import com.google.idea.blaze.base.scope.Scope;
Expand Down Expand Up @@ -87,7 +88,9 @@ static boolean shardingRequested(Project project) {
}

private static boolean shardingRequested(ProjectViewSet projectViewSet) {
return projectViewSet.getScalarValue(ShardBlazeBuildsSection.KEY).orElse(false);
// We need to perform expansion of query targets if we are to allow for manual targets to be synced.
return projectViewSet.getScalarValue(ShardBlazeBuildsSection.KEY).orElse(false) ||
projectViewSet.getScalarValue(SyncManualTargetsSection.KEY).orElse(false);
}

/** Number of individual targets per blaze build shard. */
Expand Down
6 changes: 4 additions & 2 deletions base/src/com/google/idea/blaze/base/sync/sharding/WildcardTargetExpander.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.google.idea.blaze.base.scope.scopes.TimingScope;
import com.google.idea.blaze.base.scope.scopes.TimingScope.EventType;
import com.google.idea.blaze.base.settings.Blaze;
import com.google.idea.blaze.base.sync.SyncProjectTargetsHelper;
import com.google.idea.blaze.base.sync.aspects.BuildResult;
import com.google.idea.blaze.base.sync.aspects.BuildResult.Status;
import com.google.idea.blaze.base.sync.projectview.LanguageSupport;
Expand All @@ -64,6 +65,7 @@
/** Expands wildcard target patterns into individual blaze targets. */
public class WildcardTargetExpander {

public static final String MANUAL_EXCLUDE_TAG = "^((?!manual).)*$";
private static final BoolExperiment filterByRuleType =
new BoolExperiment("blaze.build.filter.by.rule.type", true);

Expand Down Expand Up @@ -195,7 +197,7 @@ private static boolean excludeManualTargets(
BlazeCommandName.BUILD,
context,
BlazeInvocationContext.SYNC_CONTEXT)
.contains("--build_manual_tests");
.contains("--build_manual_tests") && !SyncProjectTargetsHelper.shouldSyncManualTargets(projectView);
}

/** Runs a blaze query to expand the input target patterns to individual blaze targets. */
Expand Down Expand Up @@ -272,7 +274,7 @@ private static String queryString(List<TargetExpression> targets, boolean exclud
return targetList;
}
return excludeManualTargets
? String.format("attr('tags', '^((?!manual).)*$', %s)", targetList)
? String.format("attr('tags', '%s', %s)", MANUAL_EXCLUDE_TAG, targetList)
: targetList;
}
}
101 changes: 93 additions & 8 deletions ...tests/unittests/com/google/idea/blaze/base/sync/sharding/BlazeBuildTargetSharderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static junit.framework.TestCase.assertFalse;
import static junit.framework.TestCase.assertTrue;
import static junit.framework.TestCase.fail;

import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -55,6 +57,7 @@
import com.google.idea.blaze.base.projectview.ProjectViewSet;
import com.google.idea.blaze.base.projectview.section.ScalarSection;
import com.google.idea.blaze.base.projectview.section.sections.ShardBlazeBuildsSection;
import com.google.idea.blaze.base.projectview.section.sections.SyncManualTargetsSection;
import com.google.idea.blaze.base.projectview.section.sections.TargetShardSizeSection;
import com.google.idea.blaze.base.scope.BlazeContext;
import com.google.idea.blaze.base.scope.BlazeScope;
Expand Down Expand Up @@ -267,7 +270,7 @@ public void shardTargetsRetainingOrdering_testShardWithOnlyExcludedTargetsIsDrop
public void expandAndShardTargets_shardingApproachPartitionWithoutExpanding() {
List<TargetExpression> targets = ImmutableList.of(target("//java/com/google:foo"));
ShardedTargetsResult result =
expandAndShardTargets(SyncStrategy.SERIAL, ProjectView.builder().build(), targets);
expandAndShardTargets(SyncStrategy.SERIAL, ProjectView.builder().build(), targets, fakeWildCardTargetExpanderBlazeCommandRunner);

assertThat(result.buildResult.exitCode).isEqualTo(0);
assertThat(result.shardedTargets.shardStats.shardingApproach())
Expand All @@ -281,7 +284,7 @@ public void expandAndShardTargets_remoteBuild_buildBatchingServiceIsUsed() {
.setFailToBatchTarget(false);
List<TargetExpression> targets = ImmutableList.of(target("//java/com/google:foo"));
ShardedTargetsResult result =
expandAndShardTargets(SyncStrategy.PARALLEL, ProjectView.builder().build(), targets);
expandAndShardTargets(SyncStrategy.PARALLEL, ProjectView.builder().build(), targets, fakeWildCardTargetExpanderBlazeCommandRunner);

assertThat(result.buildResult.exitCode).isEqualTo(0);
assertThat(result.shardedTargets.shardStats.shardingApproach())
Expand All @@ -301,7 +304,7 @@ public void expandAndShardTargets_localBuild_buildBatchingServiceIsUsed() {
.add(ScalarSection.builder(ShardBlazeBuildsSection.KEY).set(true))
.add(ScalarSection.builder(TargetShardSizeSection.KEY).set(500))
.build(),
targets);
targets, fakeWildCardTargetExpanderBlazeCommandRunner);

assertThat(result.buildResult.exitCode).isEqualTo(0);
ShardStats shardStats = result.shardedTargets.shardStats;
Expand All @@ -323,7 +326,7 @@ public void expandAndShardTargets_failToExpand_shardingApproachError() {
.add(ScalarSection.builder(ShardBlazeBuildsSection.KEY).set(true))
.add(ScalarSection.builder(TargetShardSizeSection.KEY).set(500))
.build(),
targets);
targets, fakeWildCardTargetExpanderBlazeCommandRunner);

assertThat(result.buildResult.exitCode).isEqualTo(BuildResult.FATAL_ERROR.exitCode);
assertThat(result.shardedTargets.shardStats.shardingApproach())
Expand All @@ -344,7 +347,7 @@ public void expandAndShardTargets_failToBatchingTargets_shardingApproachError()
.add(ScalarSection.builder(ShardBlazeBuildsSection.KEY).set(true))
.add(ScalarSection.builder(TargetShardSizeSection.KEY).set(500))
.build(),
targets);
targets, fakeWildCardTargetExpanderBlazeCommandRunner);

assertThat(result.buildResult.exitCode).isEqualTo(0);
assertThat(result.shardedTargets.shardStats.shardingApproach())
Expand Down Expand Up @@ -372,7 +375,7 @@ public void expandAndShardTargets_expandWildcardTargets() {
.add(ScalarSection.builder(ShardBlazeBuildsSection.KEY).set(true))
.add(ScalarSection.builder(TargetShardSizeSection.KEY).set(500))
.build(),
targets);
targets, fakeWildCardTargetExpanderBlazeCommandRunner);

ShardStats shardStats = result.shardedTargets.shardStats;
assertThat(shardStats.suggestedTargetSizePerShard()).isEqualTo(500);
Expand All @@ -381,8 +384,90 @@ public void expandAndShardTargets_expandWildcardTargets() {
.containsExactly(ImmutableList.of(target(expectedLabel1), target(expectedLabel2)));
}

@Test
public void expandAndShardTargets_expandWildcardTargetsNoExcludeManualTag() {
String expectedLabel1 = "//java/com/google:one";
FakeWildCardTargetExpanderBlazeCommandRunner commandRunner = new FakeWildCardTargetExpanderBlazeCommandRunner() {

@Override
public InputStream runQuery(Project project, BlazeCommand.Builder blazeCommandBuilder, BuildResultHelper buildResultHelper, BlazeContext context) throws BuildException {
// We need to confirm within the query runner because there are no public methods currently to
// perform this check downstream.
for (String argument : blazeCommandBuilder.build().toArgumentList()) {
assertFalse(argument.contains(WildcardTargetExpander.MANUAL_EXCLUDE_TAG));
}
return super.runQuery(project, blazeCommandBuilder, buildResultHelper, context);
}
};

fakeWildCardTargetExpanderExternalTaskProvider
.setReturnVal(0)
.setOutputMessage("sh_library rule " + expectedLabel1);
commandRunner.setOutputMessages(
ImmutableList.of("sh_library rule " + expectedLabel1));
fakeBuildBatchingService
.setShardingApproach(ShardingApproach.LEXICOGRAPHIC_TARGET_SHARDER)
.setFailToBatchTarget(false);

List<TargetExpression> targets = ImmutableList.of(target("//java/com/google/..."));
ProjectView projectView = ProjectView.builder()
.add(ScalarSection.builder(SyncManualTargetsSection.KEY).set(true))
.add(ScalarSection.builder(ShardBlazeBuildsSection.KEY).set(true))
.add(ScalarSection.builder(TargetShardSizeSection.KEY).set(500))
.build();

ShardedTargetsResult result =
expandAndShardTargets(
SyncStrategy.PARALLEL,
projectView,
targets, commandRunner);

assertThat(result.shardedTargets.shardedTargets)
.containsExactly(ImmutableList.of(target(expectedLabel1)));
}

@Test
public void expandAndShardTargets_expandWildcardTargetsIncludesManualTag() {
String expectedLabel1 = "//java/com/google:one";
FakeWildCardTargetExpanderBlazeCommandRunner commandRunner = new FakeWildCardTargetExpanderBlazeCommandRunner() {

@Override
public InputStream runQuery(Project project, BlazeCommand.Builder blazeCommandBuilder, BuildResultHelper buildResultHelper, BlazeContext context) throws BuildException {
// We need to confirm within the query runner because there are no public methods currently to
// perform this check downstream.
assertTrue(blazeCommandBuilder.build().toArgumentList().stream().anyMatch(argument -> argument.contains(WildcardTargetExpander.MANUAL_EXCLUDE_TAG)));
return super.runQuery(project, blazeCommandBuilder, buildResultHelper, context);
}
};

fakeWildCardTargetExpanderExternalTaskProvider
.setReturnVal(0)
.setOutputMessage("sh_library rule " + expectedLabel1);
commandRunner.setOutputMessages(
ImmutableList.of("sh_library rule " + expectedLabel1));
fakeBuildBatchingService
.setShardingApproach(ShardingApproach.LEXICOGRAPHIC_TARGET_SHARDER)
.setFailToBatchTarget(false);

List<TargetExpression> targets = ImmutableList.of(target("//java/com/google/..."));
ProjectView projectView = ProjectView.builder()
.add(ScalarSection.builder(SyncManualTargetsSection.KEY).set(false))
.add(ScalarSection.builder(ShardBlazeBuildsSection.KEY).set(true))
.add(ScalarSection.builder(TargetShardSizeSection.KEY).set(500))
.build();

ShardedTargetsResult result =
expandAndShardTargets(
SyncStrategy.PARALLEL,
projectView,
targets, commandRunner);

assertThat(result.shardedTargets.shardedTargets)
.containsExactly(ImmutableList.of(target(expectedLabel1)));
}

private ShardedTargetsResult expandAndShardTargets(
SyncStrategy syncStrategy, ProjectView projectView, List<TargetExpression> targets) {
SyncStrategy syncStrategy, ProjectView projectView, List<TargetExpression> targets, FakeBlazeCommandRunner commandRunner) {
WorkspaceRoot workspaceRoot = new WorkspaceRoot(new File("workspaceRoot"));
return BlazeBuildTargetSharder.expandAndShardTargets(
getProject(),
Expand All @@ -392,7 +477,7 @@ private ShardedTargetsResult expandAndShardTargets(
targets,
FakeBuildInvoker.builder()
.type(BuildBinaryType.BAZEL)
.commandRunner(fakeWildCardTargetExpanderBlazeCommandRunner)
.commandRunner(commandRunner)
.build(),
syncStrategy);
}
Expand Down

0 comments on commit 8532fe6

Please sign in to comment.