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 bazelbuild#16144

PiperOrigin-RevId: 472682924
Change-Id: I8c5b7477e6993a6fb77e07d82fc97260c19674b4
  • Loading branch information
Wyverald authored and copybara-github committed Sep 7, 2022
1 parent f84a679 commit 93725da
Show file tree
Hide file tree
Showing 15 changed files with 923 additions and 896 deletions.
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
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
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
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
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
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 93725da

Please sign in to comment.