Fix fuchsia_prebuilt_package() rule
This fixes the implementation of the rule in several ways:
- Use ctx.actions.declare_directory() to declare an output directory
where all input .far archive content is extracted (using `pm expand`).
This allows Bazel to track all these files properly, even if their
names cannot be known in advance. In particular, if the command is
run in a sandbox, all files are properly copied into the final
output base location, instead of being removed at the end of the
command.
This also ensures that any Bazel target that depends on the
`rebased_package_manifest.json` file will also see the content files
it points to (see associated bug). Note however that this does not
translate to GN unfortunately at the moment!
- Simplify the _COMPONENT_VALIDATION_SCRIPT to directly read the
meta.far file that was extracted by a previous action, instead of
re-extracting the full prebuilt package archive (!) for any component
or driver item to be validated.
- Improve the same script to detect components using the
`meta/${COMPONENT}.cm` fgrep pattern, to avoid false positives,
which could happen trivially if the component name passed to
fuchsia_prebuilt_package() was shorter than the ones found in the
package (e.g. 'foo' would validate 'meta/foobar.cm' even though
these do not correspond to the same component name).
Bug: 120613
Change-Id: Icca4503d4ae8c74172ea3cbe437e88301950ed04
Reviewed-on: https://fuchsia-review.googlesource.com/c/sdk-integration/+/791984
Commit-Queue: David Turner <digit@google.com>
Reviewed-by: Jiaming Li <lijiaming@google.com>
diff --git a/bazel_rules_fuchsia/fuchsia/private/assembly/fuchsia_prebuilt_package.bzl b/bazel_rules_fuchsia/fuchsia/private/assembly/fuchsia_prebuilt_package.bzl
index 25f1dce..0226e79 100644
--- a/bazel_rules_fuchsia/fuchsia/private/assembly/fuchsia_prebuilt_package.bzl
+++ b/bazel_rules_fuchsia/fuchsia/private/assembly/fuchsia_prebuilt_package.bzl
@@ -2,13 +2,14 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+"""Implement fuchsia_prebuilt_package() rule."""
+
load("//fuchsia/private:providers.bzl", "FuchsiaPackageInfo")
load("//fuchsia/private:package_publishing.bzl", "package_repo_path_from_label", "publish_package")
load("//fuchsia/private/workflows:fuchsia_task_publish.bzl", "fuchsia_task_publish")
_COMPONENT_VALIDATION_SCRIPT = """
-$FAR extract --archive=$FAR_ARCHIVE --output=$OUTDIR
-components=$($FAR list --archive=$OUTDIR/meta.far | grep $COMPONENT | wc -l)
+components=$($FAR list --archive=$OUTPUT_DIR/meta.far | grep -F meta/$COMPONENT.cm | wc -l)
if [ $components -eq 0 ]; then
echo
@@ -17,26 +18,31 @@
exit 1
fi
touch $STAMP
-
"""
def _relative_file_name(ctx, filename):
return ctx.label.name + "_expanded/" + filename
-def _validate_components_and_drivers(ctx, outdir, far_archive):
+def _validate_components_and_drivers(ctx, outdir):
far = ctx.toolchains["@rules_fuchsia//fuchsia:toolchain"].far
deps = []
for component in ctx.attr.components + ctx.attr.drivers:
stamp_file = ctx.actions.declare_file(_relative_file_name(ctx, component + "_stamp"))
+
+ # NOTE: outdir is a ctx.actions.declare_directory() path, so declare
+ # it as an input, even though only the `meta.far` inside it is used.
+ # (There is no way to create a File() instance from it).
+ #
+ # This ensures the action that populates the directory is always run
+ # properly before the run_shell() one below.
ctx.actions.run_shell(
- inputs = [far_archive, far],
+ inputs = [outdir, far],
outputs = [stamp_file],
command = _COMPONENT_VALIDATION_SCRIPT,
env = {
"FAR": far.path,
"COMPONENT": component,
- "OUTDIR": outdir,
- "FAR_ARCHIVE": far_archive.path,
+ "OUTPUT_DIR": outdir.path,
"STAMP": stamp_file.path,
},
progress_message = "Validating the component %s" % component,
@@ -47,16 +53,66 @@
def _fuchsia_prebuilt_package_impl(ctx):
sdk = ctx.toolchains["@rules_fuchsia//fuchsia:toolchain"]
far_archive = ctx.files.archive[0]
- package_manifest_json = ctx.actions.declare_file(_relative_file_name(ctx, "package_manifest.json"))
- output_files = []
- output_files += _validate_components_and_drivers(ctx, package_manifest_json.dirname, far_archive)
+
+ # Technical note: `pm expand` below will populate its output directory
+ # with multiple files whose name are content hashes and cannot be known in
+ # advance, so use ctx.actions.declare_directory() to declare an output
+ # directory for the expansion. This tells Bazel that all files present
+ # in the directory after the command execution are outputs, and should
+ # be copied from the sandbox to the corresponding final output location
+ # in the output_base directory (otherwise, they would disappear once the
+ # sandbox is destroyed).
+ #
+ # The top-level directory for this target will be computed from
+ # `${label}_expanded/`, which, for a label like `//package/foo:bar`
+ # will expand to something like this (relative to the sandbox execroot):
+ #
+ # `bazel-out/aarch64-opt/bin/package/foo/bar/bar_expanded`
+ #
+ # Inside this TARGET_OUT_DIR, the following is generated:
+ #
+ # $TARGET_OUT_DIR/
+ # content/
+ # A directory that contains the expanded content from the
+ # prebuilt package, as well as a `package_manifest.json` file
+ # that lists all entries, where source paths appear relative
+ # to the execroot too, e.g.:
+ #
+ # "blobs": [
+ # {
+ # "source_path": "bazel-out/aarch64-opt/bin/package/foo/bar/bar_expanded/_content/meta.far",
+ # "path": "meta/",
+ # "merkle": "d0d73e04d89e393b71f2280831421ebe279e247265e25714c71fdc8961928822",
+ # "size": 94288,
+ # },
+ # ...
+ #
+ # rebased_package_manifest.json
+ # A version of _content/package_manifest.json that contains
+ # source paths that are relative to an alternative `artifacts_base_path`
+ # value. However, since the default value for this argument is just '.',
+ # it will have the same content as `_content/package_manifest.json` in
+ # most cases.
+ #
+ # Note that this file _cannot_ be inside `_content`, as Bazel
+ # would complain otherwise, as it is generated by a different action
+ # than the one that generated `_content/`.
+ #
+ # <component>_stamp
+ # For each component listed in ctx.attr.components or
+ # ctx.attr.drivers, a stamp file generated by the action that
+ # verifies it belongs to the package.
+ #
+ output_dir = ctx.actions.declare_directory(_relative_file_name(ctx, "content"))
+ output_files = [output_dir]
+ output_files += _validate_components_and_drivers(ctx, output_dir)
# extract the package
ctx.actions.run(
executable = sdk.pm,
arguments = [
"-o",
- package_manifest_json.dirname,
+ output_dir.path,
"-r",
"fuchsia.com",
"expand",
@@ -64,21 +120,21 @@
],
inputs = [far_archive],
outputs = [
- package_manifest_json,
+ output_dir,
],
mnemonic = "FuchsiaPmExpand",
progress_message = "expanding package for %{label}",
)
# rebase paths in package manifest
- rebased_package_manifest_json = ctx.actions.declare_file(_relative_file_name(ctx, "_rebased_package_manifest.json"))
+ rebased_package_manifest_json = ctx.actions.declare_file(_relative_file_name(ctx, "rebased_package_manifest.json"))
ctx.actions.run(
outputs = [rebased_package_manifest_json],
- inputs = [package_manifest_json],
+ inputs = [output_dir],
executable = ctx.executable._rebase_package_manifest,
arguments = [
"--package-manifest",
- package_manifest_json.path,
+ output_dir.path + "/package_manifest.json",
"--updated-package-manifest",
rebased_package_manifest_json.path,
"--relative-base",
diff --git a/tests/fuchsia/assembly/test_data/product_config_add_package_golden_test.json b/tests/fuchsia/assembly/test_data/product_config_add_package_golden_test.json
index 7c36978..a5d4f67 100644
--- a/tests/fuchsia/assembly/test_data/product_config_add_package_golden_test.json
+++ b/tests/fuchsia/assembly/test_data/product_config_add_package_golden_test.json
@@ -7,7 +7,7 @@
"packages": {
"base": [
{
- "manifest": "../prebuilt_ams_light_expanded/_rebased_package_manifest.json",
+ "manifest": "../prebuilt_ams_light_expanded/rebased_package_manifest.json",
"config_data": [
{
"destination": "config_data.json",
@@ -18,7 +18,7 @@
],
"cache": [
{
- "manifest": "../../../bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_ams_light_expanded/_rebased_package_manifest.json"
+ "manifest": "../../../bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_ams_light_expanded/rebased_package_manifest.json"
}
]
},
diff --git a/tests/fuchsia/assembly/test_data/product_config_golden_file.json b/tests/fuchsia/assembly/test_data/product_config_golden_file.json
index 9174cc8..c4bf783 100644
--- a/tests/fuchsia/assembly/test_data/product_config_golden_file.json
+++ b/tests/fuchsia/assembly/test_data/product_config_golden_file.json
@@ -47,7 +47,7 @@
"components": [
"meta/backlight_driver.cm"
],
- "package": "bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_driver_expanded/_rebased_package_manifest.json"
+ "package": "bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_driver_expanded/rebased_package_manifest.json"
}
],
"packages": {
@@ -59,12 +59,12 @@
"source": "fuchsia/assembly/test_data/config_data.json"
}
],
- "manifest": "bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_ams_light_expanded/_rebased_package_manifest.json"
+ "manifest": "bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_ams_light_expanded/rebased_package_manifest.json"
}
],
"cache": [
{
- "manifest": "bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_ams_light_expanded/_rebased_package_manifest.json"
+ "manifest": "bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_ams_light_expanded/rebased_package_manifest.json"
}
]
},
diff --git a/tests/fuchsia/assembly/test_data/product_config_no_identity_config_golden_file.json b/tests/fuchsia/assembly/test_data/product_config_no_identity_config_golden_file.json
index ec9a593..23eee16 100644
--- a/tests/fuchsia/assembly/test_data/product_config_no_identity_config_golden_file.json
+++ b/tests/fuchsia/assembly/test_data/product_config_no_identity_config_golden_file.json
@@ -8,7 +8,7 @@
"base": [],
"cache": [
{
- "manifest": "../../../bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_ams_light_expanded/_rebased_package_manifest.json"
+ "manifest": "../../../bazel-out/k8-fastbuild/bin/fuchsia/assembly/prebuilt_ams_light_expanded/rebased_package_manifest.json"
}
]
},
