Skip to content

Commit 93725da

Browse files
Wyveraldcopybara-github
authored andcommitted
Allow download, download_and_extract, and file in module_ctx
This involves some changes in logging and reporting, since a "download event" is not necessarily tied to a repository any more, but could be from a module extension impl function. So we generally convert any logging of repo names to a "context", which could either be a repo or a module extension. Fixes #16144 PiperOrigin-RevId: 472682924 Change-Id: I8c5b7477e6993a6fb77e07d82fc97260c19674b4
1 parent f84a679 commit 93725da

File tree

15 files changed

+923
-896
lines changed

15 files changed

+923
-896
lines changed

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
+ " argument to the <code>implementation</code> function when you create a module"
4141
+ " extension.")
4242
public class ModuleExtensionContext extends StarlarkBaseExternalContext {
43+
private final ModuleExtensionId extensionId;
4344
private final StarlarkList<StarlarkBazelModule> modules;
4445

4546
protected ModuleExtensionContext(
@@ -51,6 +52,7 @@ protected ModuleExtensionContext(
5152
@Nullable ProcessWrapper processWrapper,
5253
StarlarkSemantics starlarkSemantics,
5354
@Nullable RepositoryRemoteExecutor remoteExecutor,
55+
ModuleExtensionId extensionId,
5456
StarlarkList<StarlarkBazelModule> modules) {
5557
super(
5658
workingDirectory,
@@ -61,6 +63,7 @@ protected ModuleExtensionContext(
6163
processWrapper,
6264
starlarkSemantics,
6365
remoteExecutor);
66+
this.extensionId = extensionId;
6467
this.modules = modules;
6568
}
6669

@@ -70,7 +73,8 @@ public Path getWorkingDirectory() {
7073

7174
@Override
7275
protected String getIdentifyingStringForLogging() {
73-
return "TODO";
76+
return String.format(
77+
"module extension %s in %s", extensionId.getExtensionName(), extensionId.getBzlFileLabel());
7478
}
7579

7680
@Override

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
171171
StarlarkThread thread = new StarlarkThread(mu, starlarkSemantics);
172172
thread.setPrintHandler(Event.makeDebugPrintHandler(env.getListener()));
173173
ModuleExtensionContext moduleContext =
174-
createContext(env, usagesValue, starlarkSemantics, extension);
174+
createContext(env, usagesValue, starlarkSemantics, extensionId, extension);
175175
threadContext.storeInThread(thread);
176176
try {
177177
Starlark.fastcall(
@@ -232,6 +232,7 @@ private ModuleExtensionContext createContext(
232232
Environment env,
233233
SingleExtensionUsagesValue usagesValue,
234234
StarlarkSemantics starlarkSemantics,
235+
ModuleExtensionId extensionId,
235236
ModuleExtension extension)
236237
throws SingleExtensionEvalFunctionException {
237238
Path workingDirectory =
@@ -262,6 +263,7 @@ private ModuleExtensionContext createContext(
262263
processWrapper,
263264
starlarkSemantics,
264265
repositoryRemoteExecutor,
266+
extensionId,
265267
StarlarkList.immutableCopyOf(modules));
266268
}
267269

src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public static WorkspaceRuleEvent newExecuteEvent(
4646
Map<String, String> customEnvironment,
4747
String outputDirectory,
4848
boolean quiet,
49-
String ruleLabel,
49+
String context,
5050
Location location) {
5151

5252
WorkspaceLogProtos.ExecuteEvent.Builder e =
@@ -71,8 +71,8 @@ public static WorkspaceRuleEvent newExecuteEvent(
7171
if (location != null) {
7272
result = result.setLocation(location.toString());
7373
}
74-
if (ruleLabel != null) {
75-
result = result.setRule(ruleLabel);
74+
if (context != null) {
75+
result = result.setContext(context);
7676
}
7777
return new WorkspaceRuleEvent(result.build());
7878
}
@@ -84,7 +84,7 @@ public static WorkspaceRuleEvent newDownloadEvent(
8484
String sha256,
8585
String integrity,
8686
Boolean executable,
87-
String ruleLabel,
87+
String context,
8888
Location location) {
8989
WorkspaceLogProtos.DownloadEvent.Builder e =
9090
WorkspaceLogProtos.DownloadEvent.newBuilder()
@@ -102,8 +102,8 @@ public static WorkspaceRuleEvent newDownloadEvent(
102102
if (location != null) {
103103
result = result.setLocation(location.toString());
104104
}
105-
if (ruleLabel != null) {
106-
result = result.setRule(ruleLabel);
105+
if (context != null) {
106+
result = result.setContext(context);
107107
}
108108
return new WorkspaceRuleEvent(result.build());
109109
}
@@ -114,9 +114,8 @@ public static WorkspaceRuleEvent newExtractEvent(
114114
String output,
115115
String stripPrefix,
116116
Map<String, String> renameFiles,
117-
String ruleLabel,
117+
String context,
118118
Location location) {
119-
120119
ExtractEvent e =
121120
WorkspaceLogProtos.ExtractEvent.newBuilder()
122121
.setArchive(archive)
@@ -131,8 +130,8 @@ public static WorkspaceRuleEvent newExtractEvent(
131130
if (location != null) {
132131
result = result.setLocation(location.toString());
133132
}
134-
if (ruleLabel != null) {
135-
result = result.setRule(ruleLabel);
133+
if (context != null) {
134+
result = result.setContext(context);
136135
}
137136
return new WorkspaceRuleEvent(result.build());
138137
}
@@ -146,7 +145,7 @@ public static WorkspaceRuleEvent newDownloadAndExtractEvent(
146145
String type,
147146
String stripPrefix,
148147
Map<String, String> renameFiles,
149-
String ruleLabel,
148+
String context,
150149
Location location) {
151150
WorkspaceLogProtos.DownloadAndExtractEvent.Builder e =
152151
WorkspaceLogProtos.DownloadAndExtractEvent.newBuilder()
@@ -166,15 +165,15 @@ public static WorkspaceRuleEvent newDownloadAndExtractEvent(
166165
if (location != null) {
167166
result = result.setLocation(location.toString());
168167
}
169-
if (ruleLabel != null) {
170-
result = result.setRule(ruleLabel);
168+
if (context != null) {
169+
result = result.setContext(context);
171170
}
172171
return new WorkspaceRuleEvent(result.build());
173172
}
174173

175174
/** Creates a new WorkspaceRuleEvent for a file event. */
176175
public static WorkspaceRuleEvent newFileEvent(
177-
String path, String content, boolean executable, String ruleLabel, Location location) {
176+
String path, String content, boolean executable, String context, Location location) {
178177
FileEvent e =
179178
WorkspaceLogProtos.FileEvent.newBuilder()
180179
.setPath(path)
@@ -188,14 +187,14 @@ public static WorkspaceRuleEvent newFileEvent(
188187
if (location != null) {
189188
result = result.setLocation(location.toString());
190189
}
191-
if (ruleLabel != null) {
192-
result = result.setRule(ruleLabel);
190+
if (context != null) {
191+
result = result.setContext(context);
193192
}
194193
return new WorkspaceRuleEvent(result.build());
195194
}
196195

197196
/** Creates a new WorkspaceRuleEvent for a file read event. */
198-
public static WorkspaceRuleEvent newReadEvent(String path, String ruleLabel, Location location) {
197+
public static WorkspaceRuleEvent newReadEvent(String path, String context, Location location) {
199198
WorkspaceLogProtos.ReadEvent e =
200199
WorkspaceLogProtos.ReadEvent.newBuilder().setPath(path).build();
201200

@@ -205,15 +204,14 @@ public static WorkspaceRuleEvent newReadEvent(String path, String ruleLabel, Loc
205204
if (location != null) {
206205
result = result.setLocation(location.toString());
207206
}
208-
if (ruleLabel != null) {
209-
result = result.setRule(ruleLabel);
207+
if (context != null) {
208+
result = result.setContext(context);
210209
}
211210
return new WorkspaceRuleEvent(result.build());
212211
}
213212

214213
/** Creates a new WorkspaceRuleEvent for a file read event. */
215-
public static WorkspaceRuleEvent newDeleteEvent(
216-
String path, String ruleLabel, Location location) {
214+
public static WorkspaceRuleEvent newDeleteEvent(String path, String context, Location location) {
217215
WorkspaceLogProtos.DeleteEvent e =
218216
WorkspaceLogProtos.DeleteEvent.newBuilder().setPath(path).build();
219217

@@ -223,15 +221,15 @@ public static WorkspaceRuleEvent newDeleteEvent(
223221
if (location != null) {
224222
result = result.setLocation(location.toString());
225223
}
226-
if (ruleLabel != null) {
227-
result = result.setRule(ruleLabel);
224+
if (context != null) {
225+
result = result.setContext(context);
228226
}
229227
return new WorkspaceRuleEvent(result.build());
230228
}
231229

232230
/** Creates a new WorkspaceRuleEvent for a patch event. */
233231
public static WorkspaceRuleEvent newPatchEvent(
234-
String patchFile, int strip, String ruleLabel, Location location) {
232+
String patchFile, int strip, String context, Location location) {
235233
WorkspaceLogProtos.PatchEvent e =
236234
WorkspaceLogProtos.PatchEvent.newBuilder().setPatchFile(patchFile).setStrip(strip).build();
237235

@@ -241,14 +239,14 @@ public static WorkspaceRuleEvent newPatchEvent(
241239
if (location != null) {
242240
result = result.setLocation(location.toString());
243241
}
244-
if (ruleLabel != null) {
245-
result = result.setRule(ruleLabel);
242+
if (context != null) {
243+
result = result.setContext(context);
246244
}
247245
return new WorkspaceRuleEvent(result.build());
248246
}
249247

250248
/** Creates a new WorkspaceRuleEvent for an os event. */
251-
public static WorkspaceRuleEvent newOsEvent(String ruleLabel, Location location) {
249+
public static WorkspaceRuleEvent newOsEvent(String context, Location location) {
252250
OsEvent e = WorkspaceLogProtos.OsEvent.getDefaultInstance();
253251

254252
WorkspaceLogProtos.WorkspaceEvent.Builder result =
@@ -257,15 +255,15 @@ public static WorkspaceRuleEvent newOsEvent(String ruleLabel, Location location)
257255
if (location != null) {
258256
result = result.setLocation(location.toString());
259257
}
260-
if (ruleLabel != null) {
261-
result = result.setRule(ruleLabel);
258+
if (context != null) {
259+
result = result.setContext(context);
262260
}
263261
return new WorkspaceRuleEvent(result.build());
264262
}
265263

266264
/** Creates a new WorkspaceRuleEvent for a symlink event. */
267265
public static WorkspaceRuleEvent newSymlinkEvent(
268-
String from, String to, String ruleLabel, Location location) {
266+
String from, String to, String context, Location location) {
269267
SymlinkEvent e =
270268
WorkspaceLogProtos.SymlinkEvent.newBuilder().setTarget(from).setPath(to).build();
271269

@@ -275,8 +273,8 @@ public static WorkspaceRuleEvent newSymlinkEvent(
275273
if (location != null) {
276274
result = result.setLocation(location.toString());
277275
}
278-
if (ruleLabel != null) {
279-
result = result.setRule(ruleLabel);
276+
if (context != null) {
277+
result = result.setContext(context);
280278
}
281279
return new WorkspaceRuleEvent(result.build());
282280
}
@@ -287,7 +285,7 @@ public static WorkspaceRuleEvent newTemplateEvent(
287285
String template,
288286
Map<String, String> substitutions,
289287
boolean executable,
290-
String ruleLabel,
288+
String context,
291289
Location location) {
292290
TemplateEvent e =
293291
WorkspaceLogProtos.TemplateEvent.newBuilder()
@@ -303,15 +301,15 @@ public static WorkspaceRuleEvent newTemplateEvent(
303301
if (location != null) {
304302
result = result.setLocation(location.toString());
305303
}
306-
if (ruleLabel != null) {
307-
result = result.setRule(ruleLabel);
304+
if (context != null) {
305+
result = result.setContext(context);
308306
}
309307
return new WorkspaceRuleEvent(result.build());
310308
}
311309

312310
/** Creates a new WorkspaceRuleEvent for a which event. */
313311
public static WorkspaceRuleEvent newWhichEvent(
314-
String program, String ruleLabel, Location location) {
312+
String program, String context, Location location) {
315313
WhichEvent e = WorkspaceLogProtos.WhichEvent.newBuilder().setProgram(program).build();
316314

317315
WorkspaceLogProtos.WorkspaceEvent.Builder result =
@@ -320,8 +318,8 @@ public static WorkspaceRuleEvent newWhichEvent(
320318
if (location != null) {
321319
result = result.setLocation(location.toString());
322320
}
323-
if (ruleLabel != null) {
324-
result = result.setRule(ruleLabel);
321+
if (context != null) {
322+
result = result.setContext(context);
325323
}
326324
return new WorkspaceRuleEvent(result.build());
327325
}

src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,9 @@ message WorkspaceEvent {
145145
// Location in the code (.bzl file) where the event originates.
146146
string location = 1;
147147

148-
// Label of the rule whose evaluation caused this event.
149-
string rule = 2;
148+
// The context in which this event happened. Can be "repository @foo", or
149+
// "module extension foo in @bar//:quux.bzl".
150+
string context = 2;
150151

151152
oneof event {
152153
ExecuteEvent execute_event = 3;

src/main/java/com/google/devtools/build/lib/bazel/repository/CacheHitReportingModule.java

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,39 +30,39 @@
3030
/** Module reporting about cache hits in external repositories in case of failures */
3131
public final class CacheHitReportingModule extends BlazeModule {
3232
private Reporter reporter;
33-
private Map<String, Set<Pair<String, URL>>> cacheHitsByRepo;
33+
private Map<String, Set<Pair<String, URL>>> cacheHitsByContext;
3434

3535
@Override
3636
public void beforeCommand(CommandEnvironment env) {
3737
env.getEventBus().register(this);
3838
this.reporter = env.getReporter();
39-
this.cacheHitsByRepo = new HashMap<String, Set<Pair<String, URL>>>();
39+
this.cacheHitsByContext = new HashMap<>();
4040
}
4141

4242
@Override
4343
public void afterCommand() {
4444
this.reporter = null;
45-
this.cacheHitsByRepo = null;
45+
this.cacheHitsByContext = null;
4646
}
4747

4848
@Subscribe
4949
public synchronized void cacheHit(RepositoryCacheHitEvent event) {
50-
String repo = event.getRepo().getName();
51-
if (cacheHitsByRepo.get(repo) == null) {
52-
cacheHitsByRepo.put(repo, new HashSet<Pair<String, URL>>());
53-
}
54-
cacheHitsByRepo.get(repo).add(Pair.of(event.getFileHash(), event.getUrl()));
50+
cacheHitsByContext
51+
.computeIfAbsent(event.getContext(), k -> new HashSet<>())
52+
.add(Pair.of(event.getFileHash(), event.getUrl()));
5553
}
5654

5755
@Subscribe
5856
public void failed(RepositoryFailedEvent event) {
59-
String repo = event.getRepo().getName();
60-
Set<Pair<String, URL>> cacheHits = cacheHitsByRepo.get(repo);
57+
// TODO(wyv): figure out where to put this context generation logic (right now it needs to be
58+
// kept in sync with StarlarkRepositoryContext.getIdentifyingStringForLogging), and add an
59+
// event for the failure of a module extension too
60+
String context = "repository " + event.getRepo().getNameWithAt();
61+
Set<Pair<String, URL>> cacheHits = cacheHitsByContext.get(context);
6162
if (cacheHits != null && !cacheHits.isEmpty()) {
6263
StringBuilder info = new StringBuilder();
6364

64-
info.append("Repository '")
65-
.append(repo)
65+
info.append(context)
6666
.append(
6767
"' used the following cache hits instead of downloading the corresponding file.\n");
6868
for (Pair<String, URL> hit : cacheHits) {
@@ -73,7 +73,7 @@ public void failed(RepositoryFailedEvent event) {
7373
.append("\n");
7474
}
7575
info.append("If the definition of '")
76-
.append(repo)
76+
.append(context)
7777
.append("' was updated, verify that the hashes were also updated.");
7878
reporter.handle(Event.info(info.toString()));
7979
}

src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheHitEvent.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,23 @@
1414

1515
package com.google.devtools.build.lib.bazel.repository.cache;
1616

17-
import com.google.devtools.build.lib.cmdline.RepositoryName;
1817
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
1918
import java.net.URL;
2019

2120
/** Event reporting about cache hits for download requests. */
2221
public final class RepositoryCacheHitEvent implements Postable {
23-
private final RepositoryName repo;
22+
private final String context;
2423
private final String hash;
2524
private final URL url;
2625

27-
public RepositoryCacheHitEvent(RepositoryName repo, String hash, URL url) {
28-
this.repo = repo;
26+
public RepositoryCacheHitEvent(String context, String hash, URL url) {
27+
this.context = context;
2928
this.hash = hash;
3029
this.url = url;
3130
}
3231

33-
public RepositoryName getRepo() {
34-
return repo;
32+
public String getContext() {
33+
return context;
3534
}
3635

3736
public URL getUrl() {

0 commit comments

Comments
 (0)