From d5d572e46ed3929fa3e67f6174192893943cf724 Mon Sep 17 00:00:00 2001 From: Quentin Khan Date: Thu, 10 Oct 2024 08:42:40 -0700 Subject: [PATCH] Fix build identifier generation on Windows. Windows builds fail on the generate_build_identifier genrule because the command line is too long when expanding the source file paths to feed to the Python generation script. To work around this, we write those files to a single file that we. This is what the new `xnnpack_source_list_file` rule does. It ONLY exposes the list file as an output. When the python script gets executed, the files it has access to are limited to what is explicitly requested in the `srcs` attribute. We add the "list" and the source paths to the sources to make them available to the Python script and only expand the list file path to keep the command line short enough to avoid having Windows break down. I have not found a way to transitively pull the source files from the "list" target and make them available from the Python script without adding them to the `srcs` attribute (making them expanded in the "$(SRCS)" variable), thus the repetition. PiperOrigin-RevId: 684457906 --- BUILD.bazel | 32 ++++++++++++++++++++++++++-- build_defs.bzl | 17 ++++++++++++++- scripts/generate-build-identifier.py | 10 ++++++--- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 170e035eef5..e27250168cf 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -17,6 +17,7 @@ load( "xnnpack_min_size_copts", "xnnpack_slinky_deps", "xnnpack_slinky_srcs", + "xnnpack_source_list_file", "xnnpack_transitive_source_list", "xnnpack_visibility", ) @@ -818,19 +819,46 @@ py_binary( }), ) +# This target gathers the transitive sources from its deps and adds them to the +# srcs attribute. xnnpack_transitive_source_list( name = "build_identifier_ukernel_srcs", + srcs = ["src/packing.cc"], deps = [":prod_microkernels"], ) +# Windows builds fail on the generate_build_identifier genrule because the +# command line is too long when expanding the source file paths to feed to the +# Python generation script. To work around this, we write those files to a +# single file that we. +# +# This is what the below command does. It ONLY exposes the list file as an output. +# +# See the next genrule comment for the rest of the explanation. +xnnpack_source_list_file( + name = "build_identifier_ukernel_srcs_list", + srcs = [":build_identifier_ukernel_srcs"], + compatible_with = [], +) + +# When the python script gets executed, the files it has access to are limited +# to what is explicitely requested in the `srcs` attribute. We add the "list" +# and the source paths to the sources to make them available to the Python +# script and only expand the list file path to keep the command line short +# enough to avoid having Windows break down. +# +# I have not found a way to transitively pull the source files from the "list" +# target and make them available from the Python script without adding them to +# the `srcs` attribute (making them expanded in the "$(SRCS)" variable), thus +# the repetition. genrule( name = "generate_build_identifier", srcs = [ - "src/packing.cc", + ":build_identifier_ukernel_srcs_list", ":build_identifier_ukernel_srcs", ], outs = ["src/build_identifier.c"], - cmd = "$(location generate_build_identifier_py) --output $@ $(SRCS)", + cmd = "$(location generate_build_identifier_py) --output $@ --input_file_list $(location :build_identifier_ukernel_srcs_list)", tools = [":generate_build_identifier_py"], ) diff --git a/build_defs.bzl b/build_defs.bzl index 79e665acf79..c4ad91206a7 100644 --- a/build_defs.bzl +++ b/build_defs.bzl @@ -439,12 +439,27 @@ source_list_aspect = aspect( def _transitive_source_list_rule_impl(ctx): get_repo_name = lambda x: getattr(x, "repo_name", getattr(x, "workspace_name")) files = [p for dep in ctx.attr.deps for p in dep[SrcListInfo].srcs.to_list() if get_repo_name(p.owner) == get_repo_name(ctx.label) and p.owner.package.startswith(ctx.label.package)] - return [DefaultInfo(files = depset(files))] + return [DefaultInfo(files = depset(files + ctx.files.srcs))] xnnpack_transitive_source_list = rule( implementation = _transitive_source_list_rule_impl, attrs = { "deps": attr.label_list(aspects = [source_list_aspect]), + "srcs": attr.label_list(allow_files = True), }, ) +def _source_list_file_rule_impl(ctx): + output_file = ctx.actions.declare_file(ctx.label.name + ".list") + ctx.actions.write( + output = output_file, + content = "\n".join([s.path for s in ctx.files.srcs]), + ) + return [DefaultInfo(files = depset([output_file]))] + +xnnpack_source_list_file = rule( + implementation = _source_list_file_rule_impl, + attrs = { + "srcs": attr.label_list(allow_files = True), + }, +) diff --git a/scripts/generate-build-identifier.py b/scripts/generate-build-identifier.py index f4ae4dfeada..80838c8994c 100755 --- a/scripts/generate-build-identifier.py +++ b/scripts/generate-build-identifier.py @@ -79,15 +79,19 @@ def main(args) -> None: m = hashlib.sha256() + inputs = args.inputs if args.input_file_list: with open(args.input_file_list, "r") as f: - args.inputs = f.read().splitlines() - for path in args.inputs: + inputs += f.read().splitlines() + + inputs = sorted(inputs) + + for path in inputs: with open(path, "rb") as f: m.update(f.read()) byte_list = ", ".join(str(b).rjust(3, "x") for b in m.digest()) byte_list = textwrap.indent(textwrap.fill(byte_list, width=40), " ").replace("x", " ") - formatted_input_list = "\n".join("// - " + p for p in args.inputs) + formatted_input_list = "\n".join("// - " + p for p in inputs) with open(args.output, "w") as out: out.write( FILE_TEMPLATE.format(id_data=byte_list, genlist=formatted_input_list)