Skip to content

Commit

Permalink
Fix build identifier generation on Windows.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
qukhan authored and xnnpack-bot committed Oct 10, 2024
1 parent 63e8881 commit d5d572e
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 6 deletions.
32 changes: 30 additions & 2 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
Expand Down Expand Up @@ -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"],
)

Expand Down
17 changes: 16 additions & 1 deletion build_defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
)
10 changes: 7 additions & 3 deletions scripts/generate-build-identifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit d5d572e

Please sign in to comment.