[bazel] Replace extract_files_from_skia_wasm_container

The existing implementation uses rules_docker, which we need to move awa
from. It was fairly straightforward to write a script to extract files
from the image tarball(s).

Note: this drops the existing functionality where we avoid pulling the specified image (and pull "@empty_container" instead) when the resulting files are not going to be used. As I understand it, this should not be a problem because when the flag is set we don't depend on the output files anyway, so Bazel shouldn't attempt to build the target.
Bug: b/458681039
Change-Id: Ib02727086e96d712f8e480009ec5e4e0ffaaf422
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/1115076
Reviewed-by: Kaylee Lubick <[email protected]>
Commit-Queue: Eric Boren <[email protected]>

Author: erock2112

bazel/BUILD.bazel

@@ -4,4 +4,7 @@ get_fixup_owners_layers_test_suite(
     name = "get_fixup_owners_layers_test",
 )
 
-exports_files(["strip-package-lock.py"])
+exports_files([
+    "oci-extract.py",
+    "strip-package-lock.py",
+])

bazel/oci-extract.bzl

@@ -0,0 +1,42 @@
+"""
+Provides the oci_extract rule, used for extracting files from an oci_image.
+"""
+
+def oci_extract(name, image, paths, **kwargs):
+    """Extract files from an OCI image.
+
+    Args:
+        name: Name of the target.
+        image: Label of the image to use.
+        paths: Dictionary whose keys are absolute paths within the image and
+            values are extraction destination paths.
+        **kwargs: Additional arguments to pass to the genrule.
+    """
+    python = "@python_3_11//:py3_runtime"
+    script = "//bazel:oci-extract.py"
+    cmd = """
+PYTHON=""
+for path in $(locations {python}); do
+    if [[ $$path = *"/bin/python3" ]]; then
+        PYTHON="$$path"
+    fi
+done
+
+$$PYTHON $(execpath {script}) --input=$(execpath {image}) --dest=$(RULEDIR) {path_flags}
+""".format(
+        python = python,
+        script = script,
+        image = image,
+        path_flags = " ".join(["--path=%s:%s" % (input, output) for input, output in paths.items()]),
+    )
+    native.genrule(
+        name = name,
+        srcs = [image],
+        outs = paths.values(),
+        cmd = cmd,
+        tools = [
+            python,
+            script,
+        ],
+        **kwargs
+    )

bazel/oci-extract.py

@@ -0,0 +1,101 @@
+"""
+Utility script used by the oci_extract rule in //bazel:oci-extract.bzl to
+extract files from an oci_image.
+"""
+
+
+import argparse
+import json
+import os
+import tarfile
+
+
+# Unfortunately, TarFile.extract()'s path parameter doesn't do what we'd expect
+# it to do, which is to extract place the extracted file at the exact specified
+# path. Instead, it treats the path as a directory into which the archive path
+# of the file is nested, eg. extracting "/usr/bin/myexec" with
+# path="/some/dir/myexec" produces "/some/dir/myexec/usr/bin/myexec". To work
+# around this we use an extraction filter which replaces the destination path.
+def make_extraction_filter(dst_path):
+    def filter(tarinfo, path):
+        # Call the default extraction filter first. This helps make
+        # extraction more secure.
+        tarinfo = tarfile.data_filter(tarinfo, path)
+        tarinfo.path = dst_path
+        return tarinfo
+    return filter
+
+
+def extract_members(input_path, members, dest_dir):
+    def path_for_blob(digest):
+        return os.path.join(*[input_path, "blobs"] + digest.split(":"))
+
+    with open(os.path.join(input_path, "index.json")) as f:
+        index = json.load(f)
+    # We iterate in reverse, because later layers override the earlier ones and
+    # we want the "final" version of the file.
+    for manifest_spec in reversed(index["manifests"]):
+        with open(path_for_blob(manifest_spec["digest"])) as f:
+            manifest = json.load(f)
+        for layer in reversed(manifest["layers"]):
+            layer_tar = tarfile.open(path_for_blob(layer["digest"]))
+            for src_path in list(members.keys()):
+                try:
+                    member = layer_tar.getmember(src_path)
+                except KeyError:
+                    continue
+                dst_path = os.path.join(dest_dir, members[src_path])
+                layer_tar.extract(member, filter=make_extraction_filter(dst_path))
+                del members[src_path]
+
+    if len(members) > 0:
+        raise Exception("Failed to find and extract files from %s: %s" % (input_path, ", ".join(members.keys())))
+
+
+# TODO(borenet): Remove once we've switched to oci_image.
+def extract_members_legacy(input_path, members, dest_dir):
+    with tarfile.open(input_path) as input:
+        manifest_tar = input.extractfile("manifest.json")
+        manifest_list = json.load(manifest_tar)
+
+        # We iterate in reverse, because later layers override the earlier ones and
+        # we want the "final" version of the file.
+        for manifest in reversed(manifest_list):
+            for layer in reversed(manifest["Layers"]):
+                layer_tar = tarfile.open(
+                    fileobj=input.extractfile(layer), mode="r")
+                for src_path in list(members.keys()):
+                    try:
+                        member = layer_tar.getmember(src_path)
+                    except KeyError:
+                        continue
+                    dst_path = os.path.join(dest_dir, members[src_path])
+                    layer_tar.extract(member, filter=make_extraction_filter(dst_path))
+                    del members[src_path]
+
+    if len(members) > 0:
+        raise Exception("Failed to find and extract files from %s: %s" % (input_path, ", ".join(members.keys())))
+
+
+
+def main():
+    parser = argparse.ArgumentParser()
+    parser.add_argument("--input")
+    parser.add_argument("--path", action="append")
+    parser.add_argument("--dest", default=".")
+    args = parser.parse_args()
+
+    members = {}
+    for arg in args.path:
+        split = arg.split(":")
+        if len(split) != 2:
+            raise ValueError("Invalid path: %s" % arg)
+        src = split[0].removeprefix("/")  # There's no leading "/" in the tarball.
+        dst = split[1]
+        members[src] = dst
+
+    extract_members_legacy(args.input, members, args.dest)
+
+
+if __name__ == "__main__":
+    main()

debugger-app/wasm_libs/BUILD.bazel

@@ -1,5 +1,6 @@
 load("@aspect_rules_js//js:defs.bzl", "js_library")
-load("//infra-sk:index.bzl", "bool_flag", "extract_files_from_skia_wasm_container")
+load("//bazel:oci-extract.bzl", "oci_extract")
+load("//infra-sk:index.bzl", "bool_flag")
 
 bool_flag(
     default = True,
@@ -9,16 +10,18 @@ bool_flag(
 # This target makes the wasm binaries and js files available for tests. This is not the same
 # as ../build because we want to be able to control which binaries get used when we deploy a
 # container, but we are less stringent when running tests on the infra CI.
-extract_files_from_skia_wasm_container(
+oci_extract(
     name = "fetch_debugger_wasm",
     testonly = True,
-    container = "@pinned_debugger",
-    container_files = {
+    # TODO(borenet): Remove //image when we move to oci_image.
+    image = "@pinned_debugger//image",
+    paths = {
         "/usr/local/share/debugger-app/index.d.ts": "from_container/canvaskit.d.ts",
         "/usr/local/share/debugger-app/canvaskit.js": "from_container/canvaskit.js",
         "/usr/local/share/debugger-app/canvaskit.wasm": "from_container/canvaskit.wasm",
     },
-    enabled_flag = ":use_debugger_from_container",
+    tags = ["manual"],  # Exclude it from wildcard queries, e.g. "bazel build //...".
+    visibility = ["//jsfiddle:__subpackages__"],
 )
 
 filegroup(

infra-sk/index.bzl

@@ -4,7 +4,6 @@ load("@aspect_rules_js//js:defs.bzl", "js_binary")
 
 # https://github.com/bazelbuild/bazel-skylib/blob/main/rules/common_settings.bzl
 load("@bazel_skylib//rules:common_settings.bzl", skylib_bool_flag = "bool_flag")
-load("@io_bazel_rules_docker//container:flatten.bzl", "container_flatten")
 load("@npm//:mocha/package_json.bzl", _mocha_bin = "bin")
 load("//bazel/test_on_env:test_on_env.bzl", "test_on_env")
 load("//infra-sk/esbuild:esbuild.bzl", "esbuild_dev_bundle", "esbuild_node_bundle", "esbuild_prod_bundle")
@@ -623,109 +622,6 @@ def sk_page(
         visibility = ["//visibility:public"],
     )
 
-def extract_files_from_skia_wasm_container(name, container, container_files, enabled_flag, **kwargs):
-    """Extracts files from the Skia WASM container image (gcr.io/skia-public/skia-wasm-release).
-
-    This macro takes as inputs a list of paths inside the Docker container (container_files
-    argument), and a list of the same length with the destination paths for each of the files to
-    extract (outs argument), relative to the directory where the macro is instantiated.
-
-    This image will be pulled if the enabled_flag is true, so users who need to set that flag to
-    true (e.g. infra folks pushing a clean image) should be granted permissions and have Docker
-    authentication set up. Users who are working with a local build should set the flag to false
-    and not need Docker authentication.
-
-    Args:
-      name: Name of the target.
-      container: The name of the docker container (defined in //WORKSPACE) which contains the files.
-      container_files: Dictionary of absolute paths inside the Docker container to extract mapped to
-          the path they should be extracted to.
-      enabled_flag: Label. If set, should be the name of a bool_flag. If the bool_flag
-          is True, the real skia_wasm_container image will be pulled and the images loaded as per
-          usual. If the bool_flag is false, the container will not be loaded, but any outs will be
-          created as empty files to make dependent rules happy. It is up to the caller to use that
-          same flag to properly ignore those empty files if the flag is false (e.g. via a select).
-      **kwargs: Any flags that should be forwarded to the generated rule
-    """
-
-    # Generates a .tar file with the contents of the image's filesystem (and a .json metadata file
-    # which we ignore).
-    #
-    # See the rule implementation here:
-    # https://github.com/bazelbuild/rules_docker/blob/02ad0a48fac9afb644908a634e8b2139c5e84670/container/flatten.bzl#L48
-    #
-    # Notes:
-    #  - It is unclear whether container_flatten is part of the public API because it is not
-    #    documented. But the fact that container_flatten is re-exported along with other, well
-    #    documented rules in rules_docker suggests that it might indeed be part of the public API
-    #    (see [1] and [2]).
-    #  - If they ever make breaking changes to container_flatten, then it is probably best to fork
-    #    it. The rule itself is relatively small; it is just a wrapper around a Go program that does
-    #    all the heavy lifting.
-    #  - This rule was chosen because most other rules in the rules_docker repository produce .tar
-    #    files with layered outputs, which means we would have to do the flattening ourselves.
-    #
-    # [1] https://github.com/bazelbuild/rules_docker/blob/6c29619903b6bc533ad91967f41f2a3448758e6f/container/container.bzl#L28
-    # [2] https://github.com/bazelbuild/rules_docker/blob/e290d0975ab19f9811933d2c93be275b6daf7427/container/BUILD#L158
-
-    # We expect enabled_flag to be a bool_flag, which means we should have two labels defined
-    # based on the passed in flag name, one with a _true suffix and one with a _false suffix. If
-    # the flag is set to false, we pull from a public image that has no files in it.
-    container_flatten(
-        name = name + "_skia_wasm_container_filesystem",
-        image = select({
-            enabled_flag + "_true": container + "//image",
-            enabled_flag + "_false": "@empty_container//image",
-        }),
-    )
-
-    # Name of the .tar file produced by the container_flatten target.
-    skia_wasm_filesystem_tar = name + "_skia_wasm_container_filesystem.tar"
-
-    # Shell command that returns the directory of the BUILD file where this macro was instantiated.
-    #
-    # This works because:
-    #  - The $< variable[1] is expanded by the below genrule to the path of its only input file.
-    #  - The only input file to the genrule is the .tar file produced by the above container_flatten
-    #    target (see the genrule's srcs attribute).
-    #  - Said .tar file is produced in the directory of the BUILD file where this macro was
-    #    instantiated.
-    #
-    # [1] https://bazel.build/reference/be/make-variables
-    output_dir = "$$(dirname $<)"
-
-    # Directory where we will untar the .tar file produced by the container_flatten target.
-    skia_wasm_filesystem_dir = output_dir + "/" + skia_wasm_filesystem_tar + "_untarred"
-
-    # Untar the .tar file produced by the container_flatten rule.
-    cmd = ("mkdir -p " + skia_wasm_filesystem_dir +
-           " && tar xf $< -C " + skia_wasm_filesystem_dir)
-
-    outs = []
-
-    # Copy each requested file from the container filesystem to its desired destination.
-    for src in container_files:
-        dst = container_files[src]
-        copy = " && (cp %s/%s %s/%s" % (skia_wasm_filesystem_dir, src, output_dir, dst)
-
-        # If the enabled_flag is False, we will be loading from an empty image and thus the
-        # files will not exist. As such, we need to create them or Bazel will fail because the
-        # expected generated files will not be created. It is up to the dependent rules to properly
-        # ignore these files. We cannot easily have the genrule change its behavior depending on
-        # how the flag is set, so we always touch the file as a backup. We write stderr to
-        # /dev/null to squash any errors about files not existing.
-        copy += " 2>/dev/null || touch %s/%s) " % (output_dir, dst)
-        cmd += copy
-        outs.append(dst)
-
-    native.genrule(
-        name = name,
-        srcs = [skia_wasm_filesystem_tar],
-        outs = outs,
-        cmd = cmd,
-        **kwargs
-    )
-
 def bool_flag(flag_name, default = True, name = ""):  # buildifier: disable=unused-variable
     """Create a boolean flag and corresponding config_settings.
 

jsfiddle/wasm_libs/BUILD.bazel

@@ -1,4 +1,5 @@
-load("//infra-sk:index.bzl", "bool_flag", "extract_files_from_skia_wasm_container")
+load("//bazel:oci-extract.bzl", "oci_extract")
+load("//infra-sk:index.bzl", "bool_flag")
 
 bool_flag(
     default = True,
@@ -8,16 +9,17 @@ bool_flag(
 # This target makes the wasm binaries and js files available for tests. This is not the same
 # as ../build because we want to be able to control which binaries get used when we deploy a
 # container, but we are less stringent when running tests on the infra CI.
-extract_files_from_skia_wasm_container(
+oci_extract(
     name = "fetch_canvaskit_wasm",
     testonly = True,
-    container = "@pinned_jsfiddle",
-    container_files = {
+    # TODO(borenet): Remove //image when we move to oci_image.
+    image = "@pinned_jsfiddle//image",
+    paths = {
         "/usr/local/share/jsfiddle/dist/index.d.ts": "from_container/canvaskit.d.ts",
         "/usr/local/share/jsfiddle/dist/canvaskit.js": "from_container/canvaskit.js",
         "/usr/local/share/jsfiddle/dist/canvaskit.wasm": "from_container/canvaskit.wasm",
     },
-    enabled_flag = ":use_libraries_from_container",
+    tags = ["manual"],  # Exclude it from wildcard queries, e.g. "bazel build //...".
     visibility = ["//jsfiddle:__subpackages__"],
 )
 

shaders/wasm_libs/BUILD.bazel

@@ -1,5 +1,6 @@
 load("@aspect_rules_js//js:defs.bzl", "js_library")
-load("//infra-sk:index.bzl", "bool_flag", "extract_files_from_skia_wasm_container")
+load("//bazel:oci-extract.bzl", "oci_extract")
+load("//infra-sk:index.bzl", "bool_flag")
 
 bool_flag(
     default = True,
@@ -9,16 +10,17 @@ bool_flag(
 # This target makes the canvaskit js and wasm binaries available for tests. This is not the same
 # as ../build because we want to be able to control which binaries get used when we deploy a
 # container, but we are less stringent when running tests on the infra CI.
-extract_files_from_skia_wasm_container(
+oci_extract(
     name = "fetch_canvaskit_wasm",
     testonly = True,
-    container = "@pinned_jsfiddle",
-    container_files = {
+    # TODO(borenet): Remove //image when we move to oci_image.
+    image = "@pinned_jsfiddle//image",
+    paths = {
         "/usr/local/share/jsfiddle/dist/index.d.ts": "from_container/canvaskit.d.ts",
         "/usr/local/share/jsfiddle/dist/canvaskit.js": "from_container/canvaskit.js",
         "/usr/local/share/jsfiddle/dist/canvaskit.wasm": "from_container/canvaskit.wasm",
     },
-    enabled_flag = ":use_canvaskit_from_container",
+    tags = ["manual"],  # Exclude it from wildcard queries, e.g. "bazel build //...".
     visibility = ["//shaders:__subpackages__"],
 )
 

skottie/wasm_libs/BUILD.bazel

@@ -1,4 +1,5 @@
-load("//infra-sk:index.bzl", "bool_flag", "extract_files_from_skia_wasm_container")
+load("//bazel:oci-extract.bzl", "oci_extract")
+load("//infra-sk:index.bzl", "bool_flag")
 
 bool_flag(
     default = True,
@@ -8,17 +9,18 @@ bool_flag(
 # This target makes the canvaskit js and wasm binaries available for tests. This is not the same
 # as ../build because we want to be able to control which binaries get used when we deploy a
 # container, but we are less stringent when running tests on the infra CI.
-extract_files_from_skia_wasm_container(
+oci_extract(
     name = "fetch_canvaskit_wasm",
     testonly = True,
-    container = "@pinned_jsfiddle",
-    container_files = {
+    # TODO(borenet): Remove //image when we move to oci_image.
+    image = "@pinned_jsfiddle//image",
+    paths = {
         "/usr/local/share/jsfiddle/dist/index.d.ts": "from_container/canvaskit.d.ts",
         "/usr/local/share/jsfiddle/dist/canvaskit.js": "from_container/canvaskit.js",
         "/usr/local/share/jsfiddle/dist/canvaskit.wasm": "from_container/canvaskit.wasm",
     },
-    enabled_flag = ":use_canvaskit_from_container",
-    visibility = ["//skottie:__subpackages__"],
+    tags = ["manual"],  # Exclude it from wildcard queries, e.g. "bazel build //...".
+    visibility = ["//jsfiddle:__subpackages__"],
 )
 
 filegroup(