//cuttlefish/package:package_files overhaul

[Databean]

• base_dir is recreated on every rule evaluation, fixing an outstanding issue where files would persist in the output directory beyond being deleted in the build target
• Most of the implementation is moved to a C++ executable
• package_executable is separated from package_files. this allows referencing 0 to many executables from a package, rather than always exactly one.
• When possible, hard links are created instead of copies.

Bug: b/429012834

[jmacnak]

• Most of the implementation is moved to a C++ executable

Sorry if I am missing something. Is there a motivation for this? Couldn't _package_files_impl be upated to perform the rm -rf ctx.attr.base_dir as the first step?

[Databean]

Should have mentioned, one of the issues I ran into is https://www.github.com/bazelbuild/bazel/issues/21782 : if the entire directory is considered the output with declare_directory, then bazel will reject any declare_file members inside of it. Semantically, the directory is the output, rather than a list of known files inside of it, as we care about stale files present inside of it.

I think #2998 is the strategy you're proposing, for running rm -r first. Without being able to declare_directory the entire directory, it uses a placeholder file next to it as a substitute. However, I think bazel sandboxing prevents it from observing the directory, because it's not a declared output:

ERROR: /mnt/build/standalone/android-cuttlefish/base/cvd/cuttlefish/package/BUILD.bazel:8:14: ClearOutputDirectory cuttlefish/package/cuttlefish-common.placeholder failed: (Exit 1): bash failed: error executing ClearOutputDirectory command (from target //cuttlefish/package:common) /bin/bash -c 'rm -r bazel-out/k8-fastbuild/bin/cuttlefish/package/cuttlefish-common && touch bazel-out/k8-fastbuild/bin/cuttlefish/package/cuttlefish-common.placeholder'

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
rm: cannot remove 'bazel-out/k8-fastbuild/bin/cuttlefish/package/cuttlefish-common': No such file or directory
Target //cuttlefish/package:common failed to build

This can be worked around by disabling the sandbox using --spawn_strategy=local, which is not ideal. The implementation declares the placeholder file as an input to every generated file to try to force it to run first.

Using declare_directory pushes us into using a single run or run_shell which has to produce the entire directory.

[jmacnak]

Ahh makes sense. Could you add a comment either around _package_executable_impl or in packager.cc‎ explaining the bazel limitation?

[Databean]

Added a comment, and pushed some more fixes to the debian rules script.

Also added a workaround for Bazel chmod-ing all output files ( https://www.github.com/bazelbuild/bazel/issues/5588 ) which can propagate back to any source files hard linked in.

[Databean]

Thanks for the reviews!

Also added a fix to package_executable:

-    ctx.actions.symlink(
-        output = executable_link,
-        target_path = base_dir.basename + "/" + ctx.attr.executable,
+    ctx.actions.run_shell(
+        mnemonic = "OutputSymlink",
+        outputs = [executable_link],
+        inputs = ctx.attr.package[DefaultInfo].files,
+        command = "ln -s " + base_dir.basename + "/" + ctx.attr.executable + " " + executable_link.path,
     )

With ctx.actions.symlink, running bazel run //cuttlefish/package:cvd did not check if //cuttlefish/package:common needed to be rebuilt for source file changes. Using ctx.actions.run_shell has an inputs attribute which reports the dependency edge correctly.