Skip to content

Commit 4e35c02

Browse files
ShreeM01fmeum
ShreeM01
and
fmeum
authored
Prettify labels in action progress messages with Bzlmod (bazelbuild#17278)
`UiStateTracker` is provided with the repository mapping of the main repository after the loading phase has been completed and uses this mapping to "unmap" canonical labels back to the apparent name used for them by the main repository. Fixes bazelbuild#17130 Closes bazelbuild#17045. PiperOrigin-RevId: 501256229 Change-Id: I796585da07773ee3a589bb83948b9e0667b2e799 Co-authored-by: Fabian Meumertzheim <[email protected]>
1 parent 71c3dae commit 4e35c02

File tree

8 files changed

+96
-10
lines changed

8 files changed

+96
-10
lines changed

‎src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
2828
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
2929
import com.google.devtools.build.lib.cmdline.Label;
30+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
3031
import com.google.devtools.build.lib.collect.nestedset.Depset;
3132
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
3233
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -363,14 +364,47 @@ public boolean showsOutputUnconditionally() {
363364
@Nullable
364365
@Override
365366
public final String getProgressMessage() {
367+
return getProgressMessageChecked(null);
368+
}
369+
370+
@Nullable
371+
@Override
372+
public final String getProgressMessage(RepositoryMapping mainRepositoryMapping) {
373+
Preconditions.checkNotNull(mainRepositoryMapping);
374+
return getProgressMessageChecked(mainRepositoryMapping);
375+
}
376+
377+
private String getProgressMessageChecked(@Nullable RepositoryMapping mainRepositoryMapping) {
366378
String message = getRawProgressMessage();
367379
if (message == null) {
368380
return null;
369381
}
382+
message = replaceProgressMessagePlaceholders(message, mainRepositoryMapping);
370383
String additionalInfo = getOwner().getAdditionalProgressInfo();
371384
return additionalInfo == null ? message : message + " [" + additionalInfo + "]";
372385
}
373386

387+
private String replaceProgressMessagePlaceholders(
388+
String progressMessage, @Nullable RepositoryMapping mainRepositoryMapping) {
389+
if (progressMessage.contains("%{label}") && getOwner().getLabel() != null) {
390+
String labelString;
391+
if (mainRepositoryMapping != null) {
392+
labelString = getOwner().getLabel().getDisplayForm(mainRepositoryMapping);
393+
} else {
394+
labelString = getOwner().getLabel().toString();
395+
}
396+
progressMessage = progressMessage.replace("%{label}", labelString);
397+
}
398+
if (progressMessage.contains("%{output}") && getPrimaryOutput() != null) {
399+
progressMessage =
400+
progressMessage.replace("%{output}", getPrimaryOutput().getExecPathString());
401+
}
402+
if (progressMessage.contains("%{input}") && getPrimaryInput() != null) {
403+
progressMessage = progressMessage.replace("%{input}", getPrimaryInput().getExecPathString());
404+
}
405+
return progressMessage;
406+
}
407+
374408
/**
375409
* Returns a progress message string that is specific for this action. This is then annotated with
376410
* additional information, currently the string '[for tool]' for actions in the tool

‎src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.actions;
1515

16+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
1617
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
1718
import javax.annotation.Nullable;
1819

@@ -33,6 +34,19 @@ public interface ActionExecutionMetadata extends ActionAnalysisMetadata {
3334
@Nullable
3435
String getProgressMessage();
3536

37+
/**
38+
* A variant of {@link #getProgressMessage} that additionally takes the {@link RepositoryMapping}
39+
* of the main repository, which can be used by the implementation to emit labels with apparent
40+
* instead of canonical repository names. A return value of {@code null} indicates no message
41+
* should be reported.
42+
*
43+
* <p>The default implementation simply returns the result of {@link #getProgressMessage}.
44+
*/
45+
@Nullable
46+
default String getProgressMessage(RepositoryMapping mainRepositoryMapping) {
47+
return getProgressMessage();
48+
}
49+
3650
/**
3751
* Returns a human-readable description of the inputs to {@link #getKey(ActionKeyContext)}. Used
3852
* in the output from '--explain', and in error messages for '--check_up_to_date' and

‎src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseCompleteEvent.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import com.google.common.base.Preconditions;
1717
import com.google.common.collect.ImmutableSet;
1818
import com.google.devtools.build.lib.cmdline.Label;
19+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
1920
import com.google.devtools.build.lib.events.ExtendedEventHandler;
2021

2122
/**
@@ -24,6 +25,7 @@
2425
public final class LoadingPhaseCompleteEvent implements ExtendedEventHandler.Postable {
2526
private final ImmutableSet<Label> labels;
2627
private final ImmutableSet<Label> filteredLabels;
28+
private final RepositoryMapping mainRepositoryMapping;
2729

2830
/**
2931
* Construct the event.
@@ -33,9 +35,11 @@ public final class LoadingPhaseCompleteEvent implements ExtendedEventHandler.Pos
3335
*/
3436
public LoadingPhaseCompleteEvent(
3537
ImmutableSet<Label> labels,
36-
ImmutableSet<Label> filteredLabels) {
38+
ImmutableSet<Label> filteredLabels,
39+
RepositoryMapping mainRepositoryMapping) {
3740
this.labels = Preconditions.checkNotNull(labels);
3841
this.filteredLabels = Preconditions.checkNotNull(filteredLabels);
42+
this.mainRepositoryMapping = Preconditions.checkNotNull(mainRepositoryMapping);
3943
}
4044

4145
/**
@@ -53,6 +57,13 @@ public ImmutableSet<Label> getFilteredLabels() {
5357
return filteredLabels;
5458
}
5559

60+
/**
61+
* @return The repository mapping of the main repository.
62+
*/
63+
public RepositoryMapping getMainRepositoryMapping() {
64+
return mainRepositoryMapping;
65+
}
66+
5667
@Override
5768
public boolean storeForReplay() {
5869
return true;

‎src/main/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTracker.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ void loadingComplete(LoadingPhaseCompleteEvent event) {
171171
} else {
172172
additionalMessage = labelsCount + " targets";
173173
}
174+
mainRepositoryMapping = event.getMainRepositoryMapping();
174175
}
175176

176177
@Override

‎src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
import com.google.devtools.build.lib.buildtool.buildevent.TestFilteringCompleteEvent;
4343
import com.google.devtools.build.lib.clock.Clock;
4444
import com.google.devtools.build.lib.cmdline.Label;
45+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
46+
import com.google.devtools.build.lib.events.Event;
4547
import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress;
4648
import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
4749
import com.google.devtools.build.lib.skyframe.ConfigurationPhaseStartedEvent;
@@ -69,7 +71,7 @@
6971
import java.util.concurrent.ConcurrentHashMap;
7072
import java.util.concurrent.atomic.AtomicInteger;
7173
import javax.annotation.Nullable;
72-
import javax.annotation.concurrent.GuardedBy;
74+
import javax.annotation.concurrent.GuardedBy;
7375
import javax.annotation.concurrent.ThreadSafe;
7476

7577
/** Tracks state for the UI. */
@@ -89,6 +91,8 @@ class UiStateTracker {
8991

9092
private String status;
9193
protected String additionalMessage;
94+
// Not null after the loading phase has completed.
95+
protected RepositoryMapping mainRepositoryMapping;
9296

9397
protected final Clock clock;
9498

@@ -427,6 +431,7 @@ void loadingComplete(LoadingPhaseCompleteEvent event) {
427431
} else {
428432
additionalMessage = count + " targets";
429433
}
434+
mainRepositoryMapping = event.getMainRepositoryMapping();
430435
}
431436

432437
/**
@@ -641,11 +646,11 @@ static String suffix(String s, int len) {
641646
* If possible come up with a human-readable description of the label that fits within the given
642647
* width; a non-positive width indicates not no restriction at all.
643648
*/
644-
private static String shortenedLabelString(Label label, int width) {
649+
private String shortenedLabelString(Label label, int width) {
645650
if (width <= 0) {
646-
return label.toString();
651+
return label.getDisplayForm(mainRepositoryMapping);
647652
}
648-
String name = label.toString();
653+
String name = label.getDisplayForm(mainRepositoryMapping);
649654
if (name.length() <= width) {
650655
return name;
651656
}
@@ -787,7 +792,7 @@ protected String describeAction(
787792
postfix += " " + strategy;
788793
}
789794

790-
String message = action.getProgressMessage();
795+
String message = action.getProgressMessage(mainRepositoryMapping);
791796
if (message == null) {
792797
message = action.prettyPrint();
793798
}

‎src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,11 @@ public TargetPatternPhaseValue compute(SkyKey key, Environment env) throws Inter
238238
mapOriginalPatternsToLabels(expandedPatterns, targets.getTargets()),
239239
testSuiteExpansions.buildOrThrow()));
240240
env.getListener()
241-
.post(new LoadingPhaseCompleteEvent(result.getTargetLabels(), removedTargetLabels));
241+
.post(
242+
new LoadingPhaseCompleteEvent(
243+
result.getTargetLabels(),
244+
removedTargetLabels,
245+
repositoryMappingValue.getRepositoryMapping()));
242246
return result;
243247
}
244248

‎src/test/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTrackerTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.devtools.build.lib.buildtool.ExecutionProgressReceiver;
2525
import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
2626
import com.google.devtools.build.lib.buildtool.buildevent.ExecutionProgressReceiverAvailableEvent;
27+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
2728
import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
2829
import com.google.devtools.build.lib.runtime.SkymeldUiStateTracker.BuildStatus;
2930
import com.google.devtools.build.lib.skyframe.ConfigurationPhaseStartedEvent;
@@ -74,7 +75,8 @@ public void loadingComplete_stateChanges() {
7475
uiStateTracker.buildStatus = BuildStatus.TARGET_PATTERN_PARSING;
7576

7677
uiStateTracker.loadingComplete(
77-
new LoadingPhaseCompleteEvent(ImmutableSet.of(), ImmutableSet.of()));
78+
new LoadingPhaseCompleteEvent(
79+
ImmutableSet.of(), ImmutableSet.of(), RepositoryMapping.ALWAYS_FALLBACK));
7880

7981
assertThat(uiStateTracker.buildStatus).isEqualTo(BuildStatus.LOADING_COMPLETE);
8082
}

‎src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
import static org.hamcrest.CoreMatchers.containsString;
1919
import static org.hamcrest.CoreMatchers.not;
2020
import static org.hamcrest.MatcherAssert.assertThat;
21+
import static org.mockito.ArgumentMatchers.eq;
2122
import static org.mockito.Mockito.mock;
23+
import static org.mockito.Mockito.never;
24+
import static org.mockito.Mockito.verify;
2225
import static org.mockito.Mockito.when;
2326

2427
import com.google.common.collect.ImmutableList;
@@ -54,6 +57,8 @@
5457
import com.google.devtools.build.lib.buildtool.buildevent.TestFilteringCompleteEvent;
5558
import com.google.devtools.build.lib.cmdline.Label;
5659
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
60+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
61+
import com.google.devtools.build.lib.cmdline.RepositoryName;
5762
import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress;
5863
import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
5964
import com.google.devtools.build.lib.runtime.SkymeldUiStateTracker.BuildStatus;
@@ -83,12 +88,15 @@
8388
import net.starlark.java.syntax.Location;
8489
import org.junit.Test;
8590
import org.junit.runner.RunWith;
91+
import org.mockito.AdditionalMatchers;
8692

8793
/** Tests {@link UiStateTracker}. */
8894
@RunWith(TestParameterInjector.class)
8995
public class UiStateTrackerTest extends FoundationTestCase {
9096

9197
@TestParameter boolean isSkymeld;
98+
static final RepositoryMapping MOCK_REPO_MAPPING =
99+
RepositoryMapping.createAllowingFallback(ImmutableMap.of("main", RepositoryName.MAIN));
92100

93101
private UiStateTracker getUiStateTracker(ManualClock clock) {
94102
if (isSkymeld) {
@@ -186,8 +194,11 @@ private Action mockAction(String progressMessage, String primaryOutput) {
186194
ActionsTestUtil.createArtifact(ArtifactRoot.asSourceRoot(Root.fromPath(outputBase)), path);
187195

188196
Action action = mock(Action.class);
189-
when(action.getProgressMessage()).thenReturn(progressMessage);
197+
when(action.getProgressMessage(eq(MOCK_REPO_MAPPING))).thenReturn(progressMessage);
190198
when(action.getPrimaryOutput()).thenReturn(artifact);
199+
200+
verify(action, never()).getProgressMessage(AdditionalMatchers.not(eq(MOCK_REPO_MAPPING)));
201+
verify(action, never()).getProgressMessage();
191202
return action;
192203
}
193204

@@ -208,9 +219,13 @@ private ActionOwner dummyActionOwner() throws LabelSyntaxException {
208219
}
209220

210221
private void simulateExecutionPhase(UiStateTracker uiStateTracker) {
222+
uiStateTracker.loadingComplete(
223+
new LoadingPhaseCompleteEvent(ImmutableSet.of(), ImmutableSet.of(), MOCK_REPO_MAPPING));
211224
if (this.isSkymeld) {
212225
// SkymeldUiStateTracker needs to be in the configuration phase before the execution phase.
213226
((SkymeldUiStateTracker) uiStateTracker).buildStatus = BuildStatus.ANALYSIS_COMPLETE;
227+
} else {
228+
String unused = uiStateTracker.analysisComplete();
214229
}
215230
uiStateTracker.progressReceiverAvailable(
216231
new ExecutionProgressReceiverAvailableEvent(dummyExecutionProgressReceiver()));
@@ -254,7 +269,7 @@ public void testLoadingActivity() throws IOException {
254269

255270
// When it is configuring targets.
256271
stateTracker.loadingComplete(
257-
new LoadingPhaseCompleteEvent(ImmutableSet.of(), ImmutableSet.of()));
272+
new LoadingPhaseCompleteEvent(ImmutableSet.of(), ImmutableSet.of(), MOCK_REPO_MAPPING));
258273
String additionalMessage = "5 targets";
259274
stateTracker.additionalMessage = additionalMessage;
260275
String configuredTargetProgressString = "5 targets configured";

0 commit comments

Comments
 (0)