Third cl for verbose workspaces (ability to log certain potentially non-hermetic events that happen as part of repository rules).
Adding logging for download and download_and_extract
In the future:
- Add more event types
- Allowing to specify log file rather than dumping to INFO
- Log levels, full or alerts only
RELNOTES: None
PiperOrigin-RevId: 209789459
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java
index 38c9da3..2ff1bb5 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/debug/WorkspaceRuleEvent.java
@@ -16,6 +16,8 @@
import com.google.devtools.build.lib.bazel.debug.proto.WorkspaceLogProtos;
import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
import com.google.devtools.build.lib.events.Location;
+import java.net.URL;
+import java.util.List;
import java.util.Map;
/** An event to record events happening during workspace rule resolution */
@@ -32,8 +34,6 @@
/**
* Creates a new WorkspaceRuleEvent for an execution event.
- *
- * <p>Note: we will add more granular information as needed.
*/
public static WorkspaceRuleEvent newExecuteEvent(
Iterable<Object> args,
@@ -73,6 +73,66 @@
return new WorkspaceRuleEvent(result.build());
}
+ /** Creates a new WorkspaceRuleEvent for a download event. */
+ public static WorkspaceRuleEvent newDownloadEvent(
+ List<URL> urls,
+ String output,
+ String sha256,
+ Boolean executable,
+ String ruleLabel,
+ Location location) {
+ WorkspaceLogProtos.DownloadEvent.Builder e =
+ WorkspaceLogProtos.DownloadEvent.newBuilder()
+ .setOutput(output)
+ .setSha256(sha256)
+ .setExecutable(executable);
+ for (URL u : urls) {
+ e.addUrl(u.toString());
+ }
+
+ WorkspaceLogProtos.WorkspaceEvent.Builder result =
+ WorkspaceLogProtos.WorkspaceEvent.newBuilder();
+ result = result.setDownloadEvent(e.build());
+ if (location != null) {
+ result = result.setLocation(location.print());
+ }
+ if (ruleLabel != null) {
+ result = result.setRule(ruleLabel);
+ }
+ return new WorkspaceRuleEvent(result.build());
+ }
+
+ /** Creates a new WorkspaceRuleEvent for a download event. */
+ public static WorkspaceRuleEvent newDownloadAndExtractEvent(
+ List<URL> urls,
+ String output,
+ String sha256,
+ String type,
+ String stripPrefix,
+ String ruleLabel,
+ Location location) {
+ WorkspaceLogProtos.DownloadAndExtractEvent.Builder e =
+ WorkspaceLogProtos.DownloadAndExtractEvent.newBuilder()
+ .setOutput(output)
+ .setSha256(sha256)
+ .setType(type)
+ .setStripPrefix(stripPrefix);
+ for (URL u : urls) {
+ e.addUrl(u.toString());
+ }
+
+ WorkspaceLogProtos.WorkspaceEvent.Builder result =
+ WorkspaceLogProtos.WorkspaceEvent.newBuilder();
+ result = result.setDownloadAndExtractEvent(e.build());
+ if (location != null) {
+ result = result.setLocation(location.print());
+ }
+ if (ruleLabel != null) {
+ result = result.setRule(ruleLabel);
+ }
+ return new WorkspaceRuleEvent(result.build());
+ }
+
/*
* @return a message to log for this event
*/
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto b/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto
index 1dea56a..edab4fa 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto
+++ b/src/main/java/com/google/devtools/build/lib/bazel/debug/workspace_log.proto
@@ -38,6 +38,31 @@
string output_directory = 6;
}
+// Information on "Download" event in repository_ctx.
+message DownloadEvent {
+ // Url to download from. If multiple, treated as mirrors
+ repeated string url = 1;
+ // Output file
+ string output = 2;
+ // sha256, if speficied
+ string sha256 = 3;
+ // whether to make the resulting file executable
+ bool executable = 4;
+}
+
+message DownloadAndExtractEvent {
+ // Url(s) to download from
+ repeated string url = 1;
+ // Output file
+ string output = 2;
+ // sha256, if specified
+ string sha256 = 3;
+ // Archive type, if specified. Otherwise, inferred from URL.
+ string type = 4;
+ // A directory prefix to strip from extracted files.
+ string strip_prefix = 5;
+}
+
message WorkspaceEvent {
// Location in the code (.bzl file) where the event originates.
string location = 1;
@@ -47,5 +72,7 @@
oneof event {
ExecuteEvent execute_event = 3;
+ DownloadEvent download_event = 4;
+ DownloadAndExtractEvent download_and_extract_event = 5;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
index a618eae..0aaebe1 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -298,11 +298,16 @@
}
@Override
- public void download(Object url, Object output, String sha256, Boolean executable)
+ public void download(
+ Object url, Object output, String sha256, Boolean executable, Location location)
throws RepositoryFunctionException, EvalException, InterruptedException {
validateSha256(sha256);
List<URL> urls = getUrls(url);
SkylarkPath outputPath = getPath("download()", output);
+ WorkspaceRuleEvent w =
+ WorkspaceRuleEvent.newDownloadEvent(
+ urls, output.toString(), sha256, executable, rule.getLabel().toString(), location);
+ env.getListener().post(w);
try {
checkInOutputDirectory(outputPath);
makeDirectories(outputPath.getPath());
@@ -326,11 +331,22 @@
@Override
public void downloadAndExtract(
- Object url, Object output, String sha256, String type, String stripPrefix)
+ Object url, Object output, String sha256, String type, String stripPrefix, Location location)
throws RepositoryFunctionException, InterruptedException, EvalException {
validateSha256(sha256);
List<URL> urls = getUrls(url);
+ WorkspaceRuleEvent w =
+ WorkspaceRuleEvent.newDownloadAndExtractEvent(
+ urls,
+ output.toString(),
+ sha256,
+ type,
+ stripPrefix,
+ rule.getLabel().toString(),
+ location);
+ env.getListener().post(w);
+
// Download to outputDirectory and delete it after extraction
SkylarkPath outputPath = getPath("download_and_extract()", output);
checkInOutputDirectory(outputPath);
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
index 698f307..900fe0a 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
@@ -251,120 +251,114 @@
public RepositoryPathApi<?> which(String program) throws EvalException;
@SkylarkCallable(
- name = "download",
- doc = "Download a file to the output path for the provided url.",
- parameters = {
- @Param(
- name = "url",
- allowedTypes = {
- @ParamType(type = String.class),
- @ParamType(type = Iterable.class, generic1 = String.class),
- },
- named = true,
- doc = "List of mirror URLs referencing the same file."
- ),
- @Param(
- name = "output",
- allowedTypes = {
- @ParamType(type = String.class),
- @ParamType(type = Label.class),
- @ParamType(type = RepositoryPathApi.class)
- },
- defaultValue = "''",
- named = true,
- doc = "path to the output file, relative to the repository directory."
- ),
- @Param(
- name = "sha256",
- type = String.class,
- defaultValue = "''",
- named = true,
- doc =
- "the expected SHA-256 hash of the file downloaded."
- + " This must match the SHA-256 hash of the file downloaded. It is a security risk"
- + " to omit the SHA-256 as remote files can change. At best omitting this field"
- + " will make your build non-hermetic. It is optional to make development easier"
- + " but should be set before shipping."
- ),
- @Param(
- name = "executable",
- type = Boolean.class,
- defaultValue = "False",
- named = true,
- doc = "set the executable flag on the created file, false by default."
- ),
- }
- )
- public void download(Object url, Object output, String sha256, Boolean executable)
+ name = "download",
+ doc = "Download a file to the output path for the provided url.",
+ useLocation = true,
+ parameters = {
+ @Param(
+ name = "url",
+ allowedTypes = {
+ @ParamType(type = String.class),
+ @ParamType(type = Iterable.class, generic1 = String.class),
+ },
+ named = true,
+ doc = "List of mirror URLs referencing the same file."),
+ @Param(
+ name = "output",
+ allowedTypes = {
+ @ParamType(type = String.class),
+ @ParamType(type = Label.class),
+ @ParamType(type = RepositoryPathApi.class)
+ },
+ defaultValue = "''",
+ named = true,
+ doc = "path to the output file, relative to the repository directory."),
+ @Param(
+ name = "sha256",
+ type = String.class,
+ defaultValue = "''",
+ named = true,
+ doc =
+ "the expected SHA-256 hash of the file downloaded."
+ + " This must match the SHA-256 hash of the file downloaded. It is a security"
+ + " risk to omit the SHA-256 as remote files can change. At best omitting this"
+ + " field will make your build non-hermetic. It is optional to make"
+ + " development easier but should be set before shipping."),
+ @Param(
+ name = "executable",
+ type = Boolean.class,
+ defaultValue = "False",
+ named = true,
+ doc = "set the executable flag on the created file, false by default."),
+ })
+ public void download(
+ Object url, Object output, String sha256, Boolean executable, Location location)
throws RepositoryFunctionExceptionT, EvalException, InterruptedException;
@SkylarkCallable(
- name = "download_and_extract",
- doc = "Download a file to the output path for the provided url, and extract it.",
- parameters = {
- @Param(
- name = "url",
- allowedTypes = {
- @ParamType(type = String.class),
- @ParamType(type = Iterable.class, generic1 = String.class),
- },
- named = true,
- doc = "List of mirror URLs referencing the same file."
- ),
- @Param(
- name = "output",
- allowedTypes = {
- @ParamType(type = String.class),
- @ParamType(type = Label.class),
- @ParamType(type = RepositoryPathApi.class)
- },
- defaultValue = "''",
- named = true,
- doc =
- "path to the directory where the archive will be unpacked,"
- + " relative to the repository directory."
- ),
- @Param(
- name = "sha256",
- type = String.class,
- defaultValue = "''",
- named = true,
- doc =
- "the expected SHA-256 hash of the file downloaded."
- + " This must match the SHA-256 hash of the file downloaded. It is a security risk"
- + " to omit the SHA-256 as remote files can change. At best omitting this field"
- + " will make your build non-hermetic. It is optional to make development easier"
- + " but should be set before shipping."
- + " If provided, the repository cache will first be checked for a file with the"
- + " given hash; a download will only be attempted, if the file was not found in the"
- + " cache. After a successful download, the file will be added to the cache."
- ),
- @Param(
- name = "type",
- type = String.class,
- defaultValue = "''",
- named = true,
- doc =
- "the archive type of the downloaded file."
- + " By default, the archive type is determined from the file extension of the URL."
- + " If the file has no extension, you can explicitly specify either \"zip\","
- + " \"jar\", \"war\", \"tar.gz\", \"tgz\", \"tar.bz2\", or \"tar.xz\" here."
- ),
- @Param(
- name = "stripPrefix",
- type = String.class,
- defaultValue = "''",
- named = true,
- doc =
- "a directory prefix to strip from the extracted files."
- + "\nMany archives contain a top-level directory that contains all files in the"
- + " archive. Instead of needing to specify this prefix over and over in the"
- + " <code>build_file</code>, this field can be used to strip it from extracted"
- + " files."
- ),
- }
- )
+ name = "download_and_extract",
+ doc = "Download a file to the output path for the provided url, and extract it.",
+ useLocation = true,
+ parameters = {
+ @Param(
+ name = "url",
+ allowedTypes = {
+ @ParamType(type = String.class),
+ @ParamType(type = Iterable.class, generic1 = String.class),
+ },
+ named = true,
+ doc = "List of mirror URLs referencing the same file."),
+ @Param(
+ name = "output",
+ allowedTypes = {
+ @ParamType(type = String.class),
+ @ParamType(type = Label.class),
+ @ParamType(type = RepositoryPathApi.class)
+ },
+ defaultValue = "''",
+ named = true,
+ doc =
+ "path to the directory where the archive will be unpacked,"
+ + " relative to the repository directory."),
+ @Param(
+ name = "sha256",
+ type = String.class,
+ defaultValue = "''",
+ named = true,
+ doc =
+ "the expected SHA-256 hash of the file downloaded."
+ + " This must match the SHA-256 hash of the file downloaded. It is a security"
+ + " risk to omit the SHA-256 as remote files can change. At best omitting this"
+ + " field will make your build non-hermetic. It is optional to make"
+ + " development easier but should be set before shipping."
+ + " If provided, the repository cache will first be checked for a file with the"
+ + " given hash; a download will only be attempted, if the file was not found"
+ + " in the cache. After a successful download, the file will be added to the"
+ + " cache."),
+ @Param(
+ name = "type",
+ type = String.class,
+ defaultValue = "''",
+ named = true,
+ doc =
+ "the archive type of the downloaded file."
+ + " By default, the archive type is determined from the file extension of the"
+ + " URL. If the file has no extension, you can explicitly specify either"
+ + " \"zip\", \"jar\", \"war\", \"tar.gz\", \"tgz\", \"tar.bz2\", or \"tar.xz\""
+ + " here."),
+ @Param(
+ name = "stripPrefix",
+ type = String.class,
+ defaultValue = "''",
+ named = true,
+ doc =
+ "a directory prefix to strip from the extracted files."
+ + "\nMany archives contain a top-level directory that contains all files in the"
+ + " archive. Instead of needing to specify this prefix over and over in the"
+ + " <code>build_file</code>, this field can be used to strip it from extracted"
+ + " files."),
+ })
public void downloadAndExtract(
- Object url, Object output, String sha256, String type, String stripPrefix)
+ Object url, Object output, String sha256, String type, String stripPrefix, Location location)
throws RepositoryFunctionExceptionT, InterruptedException, EvalException;
}
diff --git a/src/test/shell/bazel/bazel_workspaces_test.sh b/src/test/shell/bazel/bazel_workspaces_test.sh
index 75ebc78..efad56e 100755
--- a/src/test/shell/bazel/bazel_workspaces_test.sh
+++ b/src/test/shell/bazel/bazel_workspaces_test.sh
@@ -20,15 +20,17 @@
source "${CURRENT_DIR}/../integration_test_setup.sh" \
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+source "${CURRENT_DIR}/remote_helpers.sh" \
+ || { echo "remote_helpers.sh not found!" >&2; exit 1; }
function test_execute() {
create_new_workspace
- cat > BUILD <<EOF
+ cat > BUILD <<'EOF'
genrule(
name="test",
srcs=["@repo//:t.txt"],
outs=["out.txt"],
- cmd="echo Result > \$(location out.txt)"
+ cmd="echo Result > $(location out.txt)"
)
EOF
cat >> repos.bzl <<EOF
@@ -58,7 +60,7 @@
# Cached executions are not replayed
bazel build //:test --experimental_workspace_rules_logging=yes &> output || fail "could not build //:test"
- cat output &> $TEST_log
+ cat output >> $TEST_log
executes=`grep "location: .*repos.bzl:2:3" output | wc -l`
if [ "$executes" -ne "0" ]
then
@@ -68,12 +70,12 @@
function test_reexecute() {
create_new_workspace
- cat > BUILD <<EOF
+ cat > BUILD <<'EOF'
genrule(
name="test",
srcs=["@repo//:t.txt"],
outs=["out.txt"],
- cmd="echo Result > \$(location out.txt)"
+ cmd="echo Result > $(location out.txt)"
)
EOF
cat >> repos.bzl <<EOF
@@ -114,4 +116,131 @@
fi
}
+# Sets up a workspace with the given commands inserted into the repository rule
+# that will be executed when doing bazel build //:test
+function set_workspace_command() {
+ create_new_workspace
+ cat > BUILD <<'EOF'
+genrule(
+ name="test",
+ srcs=["@repo//:t.txt"],
+ outs=["out.txt"],
+ cmd="echo Result > $(location out.txt)"
+)
+EOF
+ cat >> repos.bzl <<EOF
+def _executeMe(repository_ctx):
+ $1
+ build_contents = "package(default_visibility = ['//visibility:public'])\n\n"
+ build_contents += "exports_files([\"t.txt\"])\n"
+ repository_ctx.file("BUILD", build_contents, False)
+ repository_ctx.file("t.txt", "HELLO!\n", False)
+
+ex_repo = repository_rule(
+ implementation = _executeMe,
+ local = True,
+)
+EOF
+ cat >> WORKSPACE <<EOF
+load("//:repos.bzl", "ex_repo")
+ex_repo(name = "repo")
+EOF
+}
+
+# Ensure details of the specific functions are present
+function test_execute2() {
+ set_workspace_command 'repository_ctx.execute(["echo", "test_contents"], 21, {"Arg1": "Val1"}, True)'
+
+ bazel build //:test --experimental_workspace_rules_logging=yes &> ${TEST_log} || fail "could not build //:test\n"
+ expect_log "location: .*repos.bzl:2:3"
+ expect_log "arguments: \"echo\""
+ expect_log "arguments: \"test_contents\""
+ expect_log "timeout_seconds: 21"
+ expect_log "quiet: true"
+ expect_log "key: \"Arg1\""
+ expect_log "value: \"Val1\""
+ expect_log "rule: \"//external:repo\""
+}
+
+
+function test_download() {
+ # Prepare HTTP server with Python
+ local server_dir="${TEST_TMPDIR}/server_dir"
+ mkdir -p "${server_dir}"
+ local file="${server_dir}/file.txt"
+ echo "file contents here" > "${file}"
+ file_sha256="$(sha256sum "${file}" | head -c 64)"
+
+ # Start HTTP server with Python
+ startup_server "${server_dir}"
+
+ set_workspace_command "repository_ctx.download(\"http://localhost:${fileserver_port}/file.txt\", \"file.txt\", \"${file_sha256}\")"
+
+ bazel build //:test --experimental_workspace_rules_logging=yes &> ${TEST_log} && shutdown_server || fail "could not build //:test\n"
+ expect_log "location: .*repos.bzl:2:3"
+ expect_log "rule: \"//external:repo\""
+ expect_log "download_event"
+ expect_log "url: \"http://localhost:${fileserver_port}/file.txt\""
+ expect_log "output: \"file.txt\""
+ expect_log "sha256: \"${file_sha256}\""
+}
+
+function test_download_multiple() {
+ # Prepare HTTP server with Python
+ local server_dir="${TEST_TMPDIR}/server_dir"
+ mkdir -p "${server_dir}"
+ local file2="${server_dir}/file2.txt"
+ echo "second contents here" > "${file2}"
+
+ # Start HTTP server with Python
+ startup_server "${server_dir}"
+
+ set_workspace_command "repository_ctx.download([\"http://localhost:${fileserver_port}/file1.txt\",\"http://localhost:${fileserver_port}/file2.txt\"], \"out_for_list.txt\")"
+
+ bazel build //:test --experimental_workspace_rules_logging=yes &> $TEST_log && shutdown_server || fail "could not build //:test\n"
+ expect_log "location: .*repos.bzl:2:3"
+ expect_log "rule: \"//external:repo\""
+ expect_log "download_event"
+ expect_log "url: \"http://localhost:${fileserver_port}/file1.txt\""
+ expect_log "url: \"http://localhost:${fileserver_port}/file2.txt\""
+ expect_log "output: \"out_for_list.txt\""
+}
+
+function test_download_and_extract() {
+ # Prepare HTTP server with Python
+ local server_dir="${TEST_TMPDIR}/server_dir"
+ mkdir -p "${server_dir}"
+ local file_prefix="${server_dir}/download_and_extract"
+
+ pushd ${TEST_TMPDIR}
+ echo "This is one file" > server_dir/download_and_extract.txt
+ zip -r server_dir/download_and_extract.zip server_dir
+ file_sha256="$(sha256sum server_dir/download_and_extract.zip | head -c 64)"
+ popd
+
+ # Start HTTP server with Python
+ startup_server "${server_dir}"
+
+ set_workspace_command "repository_ctx.download_and_extract(\"http://localhost:${fileserver_port}/download_and_extract.zip\", \"out_dir\", \"${file_sha256}\", \"zip\", \"server_dir/\")"
+
+ bazel build //:test --experimental_workspace_rules_logging=yes &> ${TEST_log} && shutdown_server || fail "could not build //:test\n"
+
+ expect_log "location: .*repos.bzl:2:3"
+ expect_log "rule: \"//external:repo\""
+ expect_log "download_and_extract_event"
+ expect_log "url: \"http://localhost:${fileserver_port}/download_and_extract.zip\""
+ expect_log "output: \"out_dir\""
+ expect_log "sha256: \"${file_sha256}\""
+ expect_log "type: \"zip\""
+ expect_log "strip_prefix: \"server_dir/\""
+}
+
+function tear_down() {
+ shutdown_server
+ if [ -d "${TEST_TMPDIR}/server_dir" ]; then
+ rm -fr "${TEST_TMPDIR}/server_dir"
+ fi
+ true
+}
+
run_suite "workspaces_tests"
