Skip to content

Commit

Permalink
Allow download, download_and_extract, and file in module_ctx
Browse files Browse the repository at this point in the history 
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: 469413473
Change-Id: I8c5b7477e6993a6fb77e07d82fc97260c19674b4
  • Loading branch information
@Wyverald @copybara-github
Wyverald authored and copybara-github committed Sep 6, 2022
1 parent c2ccb28 commit 9edede0
Show file tree
Hide file tree
Showing 15 changed files with 921 additions and 896 deletions.
6 changes: 5 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
+ " argument to the <code>implementation</code> function when you create a module"
+ " extension.")
public class ModuleExtensionContext extends StarlarkBaseExternalContext {
private final ModuleExtensionId extensionId;
private final StarlarkList<StarlarkBazelModule> modules;

protected ModuleExtensionContext(
Expand All @@ -51,6 +52,7 @@ protected ModuleExtensionContext(
@Nullable ProcessWrapper processWrapper,
StarlarkSemantics starlarkSemantics,
@Nullable RepositoryRemoteExecutor remoteExecutor,
ModuleExtensionId extensionId,
StarlarkList<StarlarkBazelModule> modules) {
super(
workingDirectory,
Expand All @@ -61,6 +63,7 @@ protected ModuleExtensionContext(
processWrapper,
starlarkSemantics,
remoteExecutor);
this.extensionId = extensionId;
this.modules = modules;
}

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

@Override
protected String getIdentifyingStringForLogging() {
return "TODO";
return String.format(
"module extension %s in %s", extensionId.getExtensionName(), extensionId.getBzlFileLabel());
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
StarlarkThread thread = new StarlarkThread(mu, starlarkSemantics);
thread.setPrintHandler(Event.makeDebugPrintHandler(env.getListener()));
ModuleExtensionContext moduleContext =
createContext(env, usagesValue, starlarkSemantics, extension);
createContext(env, usagesValue, starlarkSemantics, extensionId, extension);
threadContext.storeInThread(thread);
try {
Starlark.fastcall(
Expand Down Expand Up @@ -232,6 +232,7 @@ private ModuleExtensionContext createContext(
Environment env,
SingleExtensionUsagesValue usagesValue,
StarlarkSemantics starlarkSemantics,
ModuleExtensionId extensionId,
ModuleExtension extension)
throws SingleExtensionEvalFunctionException {
Path workingDirectory =
Expand Down Expand Up @@ -262,6 +263,7 @@ private ModuleExtensionContext createContext(
processWrapper,
starlarkSemantics,
repositoryRemoteExecutor,
extensionId,
StarlarkList.immutableCopyOf(modules));
}

Expand Down
74 changes: 36 additions & 38 deletions src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static WorkspaceRuleEvent newExecuteEvent(
Map<String, String> customEnvironment,
String outputDirectory,
boolean quiet,
String ruleLabel,
String context,
Location location) {

WorkspaceLogProtos.ExecuteEvent.Builder e =
Expand All @@ -71,8 +71,8 @@ public static WorkspaceRuleEvent newExecuteEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}
Expand All @@ -84,7 +84,7 @@ public static WorkspaceRuleEvent newDownloadEvent(
String sha256,
String integrity,
Boolean executable,
String ruleLabel,
String context,
Location location) {
WorkspaceLogProtos.DownloadEvent.Builder e =
WorkspaceLogProtos.DownloadEvent.newBuilder()
Expand All @@ -102,8 +102,8 @@ public static WorkspaceRuleEvent newDownloadEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}
Expand All @@ -114,9 +114,8 @@ public static WorkspaceRuleEvent newExtractEvent(
String output,
String stripPrefix,
Map<String, String> renameFiles,
String ruleLabel,
String context,
Location location) {

ExtractEvent e =
WorkspaceLogProtos.ExtractEvent.newBuilder()
.setArchive(archive)
Expand All @@ -131,8 +130,8 @@ public static WorkspaceRuleEvent newExtractEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}
Expand All @@ -146,7 +145,7 @@ public static WorkspaceRuleEvent newDownloadAndExtractEvent(
String type,
String stripPrefix,
Map<String, String> renameFiles,
String ruleLabel,
String context,
Location location) {
WorkspaceLogProtos.DownloadAndExtractEvent.Builder e =
WorkspaceLogProtos.DownloadAndExtractEvent.newBuilder()
Expand All @@ -166,15 +165,15 @@ public static WorkspaceRuleEvent newDownloadAndExtractEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}

/** Creates a new WorkspaceRuleEvent for a file event. */
public static WorkspaceRuleEvent newFileEvent(
String path, String content, boolean executable, String ruleLabel, Location location) {
String path, String content, boolean executable, String context, Location location) {
FileEvent e =
WorkspaceLogProtos.FileEvent.newBuilder()
.setPath(path)
Expand All @@ -188,14 +187,14 @@ public static WorkspaceRuleEvent newFileEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}

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

Expand All @@ -205,15 +204,14 @@ public static WorkspaceRuleEvent newReadEvent(String path, String ruleLabel, Loc
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}

/** Creates a new WorkspaceRuleEvent for a file read event. */
public static WorkspaceRuleEvent newDeleteEvent(
String path, String ruleLabel, Location location) {
public static WorkspaceRuleEvent newDeleteEvent(String path, String context, Location location) {
WorkspaceLogProtos.DeleteEvent e =
WorkspaceLogProtos.DeleteEvent.newBuilder().setPath(path).build();

Expand All @@ -223,15 +221,15 @@ public static WorkspaceRuleEvent newDeleteEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}

/** Creates a new WorkspaceRuleEvent for a patch event. */
public static WorkspaceRuleEvent newPatchEvent(
String patchFile, int strip, String ruleLabel, Location location) {
String patchFile, int strip, String context, Location location) {
WorkspaceLogProtos.PatchEvent e =
WorkspaceLogProtos.PatchEvent.newBuilder().setPatchFile(patchFile).setStrip(strip).build();

Expand All @@ -241,14 +239,14 @@ public static WorkspaceRuleEvent newPatchEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}

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

WorkspaceLogProtos.WorkspaceEvent.Builder result =
Expand All @@ -257,15 +255,15 @@ public static WorkspaceRuleEvent newOsEvent(String ruleLabel, Location location)
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}

/** Creates a new WorkspaceRuleEvent for a symlink event. */
public static WorkspaceRuleEvent newSymlinkEvent(
String from, String to, String ruleLabel, Location location) {
String from, String to, String context, Location location) {
SymlinkEvent e =
WorkspaceLogProtos.SymlinkEvent.newBuilder().setTarget(from).setPath(to).build();

Expand All @@ -275,8 +273,8 @@ public static WorkspaceRuleEvent newSymlinkEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}
Expand All @@ -287,7 +285,7 @@ public static WorkspaceRuleEvent newTemplateEvent(
String template,
Map<String, String> substitutions,
boolean executable,
String ruleLabel,
String context,
Location location) {
TemplateEvent e =
WorkspaceLogProtos.TemplateEvent.newBuilder()
Expand All @@ -303,15 +301,15 @@ public static WorkspaceRuleEvent newTemplateEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}

/** Creates a new WorkspaceRuleEvent for a which event. */
public static WorkspaceRuleEvent newWhichEvent(
String program, String ruleLabel, Location location) {
String program, String context, Location location) {
WhichEvent e = WorkspaceLogProtos.WhichEvent.newBuilder().setProgram(program).build();

WorkspaceLogProtos.WorkspaceEvent.Builder result =
Expand All @@ -320,8 +318,8 @@ public static WorkspaceRuleEvent newWhichEvent(
if (location != null) {
result = result.setLocation(location.toString());
}
if (ruleLabel != null) {
result = result.setRule(ruleLabel);
if (context != null) {
result = result.setContext(context);
}
return new WorkspaceRuleEvent(result.build());
}
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,9 @@ message WorkspaceEvent {
// Location in the code (.bzl file) where the event originates.
string location = 1;

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

oneof event {
ExecuteEvent execute_event = 3;
Expand Down
26 changes: 13 additions & 13 deletions src/main/java/com/google/devtools/build/lib/bazel/repository/CacheHitReportingModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,39 +30,39 @@
/** Module reporting about cache hits in external repositories in case of failures */
public final class CacheHitReportingModule extends BlazeModule {
private Reporter reporter;
private Map<String, Set<Pair<String, URL>>> cacheHitsByRepo;
private Map<String, Set<Pair<String, URL>>> cacheHitsByContext;

@Override
public void beforeCommand(CommandEnvironment env) {
env.getEventBus().register(this);
this.reporter = env.getReporter();
this.cacheHitsByRepo = new HashMap<String, Set<Pair<String, URL>>>();
this.cacheHitsByContext = new HashMap<>();
}

@Override
public void afterCommand() {
this.reporter = null;
this.cacheHitsByRepo = null;
this.cacheHitsByContext = null;
}

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

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

info.append("Repository '")
.append(repo)
info.append(context)
.append(
"' used the following cache hits instead of downloading the corresponding file.\n");
for (Pair<String, URL> hit : cacheHits) {
Expand All @@ -73,7 +73,7 @@ public void failed(RepositoryFailedEvent event) {
.append("\n");
}
info.append("If the definition of '")
.append(repo)
.append(context)
.append("' was updated, verify that the hashes were also updated.");
reporter.handle(Event.info(info.toString()));
}
Expand Down
11 changes: 5 additions & 6 deletions ...in/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheHitEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,23 @@

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

import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import java.net.URL;

/** Event reporting about cache hits for download requests. */
public final class RepositoryCacheHitEvent implements Postable {
private final RepositoryName repo;
private final String context;
private final String hash;
private final URL url;

public RepositoryCacheHitEvent(RepositoryName repo, String hash, URL url) {
this.repo = repo;
public RepositoryCacheHitEvent(String context, String hash, URL url) {
this.context = context;
this.hash = hash;
this.url = url;
}

public RepositoryName getRepo() {
return repo;
public String getContext() {
return context;
}

public URL getUrl() {
Expand Down
Loading

0 comments on commit 9edede0

Please sign in to comment.