From ccd88d5a63a91254277c4eaddfb4c428560ad56a Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 3 Sep 2022 01:52:54 +0000 Subject: [PATCH 1/4] Updated staleness test and amalgamator to work cross-repo. --- BUILD | 1 + bazel/amalgamate.py | 104 ++++++++++++++++++++++--------------------- bazel/build_defs.bzl | 11 +---- cmake/BUILD.bazel | 1 + cmake/build_defs.bzl | 4 +- 5 files changed, 59 insertions(+), 62 deletions(-) diff --git a/BUILD b/BUILD index a49327b287..7aaf58a78f 100644 --- a/BUILD +++ b/BUILD @@ -942,6 +942,7 @@ upb_amalgamation( ], prefix = "ruby-", strip_import_prefix = ["src"], + visibility = ["@com_google_protobuf//ruby:__pkg__"], ) cc_library( diff --git a/bazel/amalgamate.py b/bazel/amalgamate.py index 70399085ca..d60cba5d83 100755 --- a/bazel/amalgamate.py +++ b/bazel/amalgamate.py @@ -35,43 +35,42 @@ def parse_include(line): match = INCLUDE_RE.match(line) return match.groups()[0] if match else None +def is_core_upb_header(fname): + if not fname.endswith(".h"): + return False + return fname.startswith("upb") or fname.startswith("google") + class Amalgamator: - def __init__(self, output_path, prefix): + def __init__(self, h_out, c_out): self.include_paths = ["."] - self.included = set(["upb/port_def.inc", "upb/port_undef.inc"]) - self.output_h = open(output_path + prefix + "upb.h", "w") - self.output_c = open(output_path + prefix + "upb.c", "w") + self.included = set() + self.output_h = open(h_out, "w") + self.output_c = open(c_out, "w") + self.h_out = h_out.split("/")[-1] + def amalgamate(self, h_files, c_files): + self.h_files = set(h_files) self.output_c.write("/* Amalgamated source file */\n") - self.output_c.write('#include "%supb.h"\n' % (prefix)) - if prefix == "ruby-": + self.output_c.write('#include "%s"\n' % (self.h_out)) + if self.h_out == "ruby-upb.h": self.output_h.write("// Ruby is still using proto3 enum semantics for proto2\n") self.output_h.write("#define UPB_DISABLE_PROTO2_ENUM_CHECKING\n") - self.output_c.write(open("upb/port_def.inc").read()) self.output_h.write("/* Amalgamated source file */\n") - self.output_h.write(open("upb/port_def.inc").read()) - def add_include_path(self, path): - self.include_paths.append(path) + port_def = self._find_include_file("upb/port_def.inc") + port_undef = self._find_include_file("upb/port_undef.inc") + self._process_file(port_def, self.output_h) + self._process_file(port_def, self.output_c) + + for file in c_files: + self._process_file(file, self.output_c) - def finish(self): - self._add_header("upb/port_undef.inc") - self.add_src("upb/port_undef.inc") + self._process_file(port_undef, self.output_h) + self._process_file(port_undef, self.output_c) def _process_file(self, infile_name, outfile): - file = None - for path in self.include_paths: - try: - full_path = os.path.join(path, infile_name) - file = open(full_path) - break - except IOError: - pass - if not file: - raise RuntimeError("Couldn't open file " + infile_name) - - lines = file.readlines() + lines = open(infile_name).readlines() has_copyright = lines[1].startswith(" * Copyright") if has_copyright: @@ -79,51 +78,54 @@ class Amalgamator: lines.pop(0) lines.pop(0) - lines.insert(0, "\n/** " + infile_name + " " + ("*" * 60) +"/"); - for line in lines: - if not self._process_include(line, outfile): + if not self._process_include(line): outfile.write(line) - def _process_include(self, line, outfile): + def _find_include_file(self, name): + for h_file in self.h_files: + if h_file.endswith(name): + return h_file + + def _process_include(self, line): include = parse_include(line) if not include: return False if not (include.startswith("upb") or include.startswith("google")): return False + if include and (include.endswith("port_def.inc") or include.endswith("port_undef.inc")): + # Skip, we handle this separately + return True if include.endswith("hpp"): # Skip, we don't support the amalgamation from C++. return True + elif include in self.included: + return True else: # Include this upb header inline. - if include not in self.included: + h_file = self._find_include_file(include) + if h_file: + self.h_files.remove(h_file) self.included.add(include) - self._add_header(include) - return True - - def _add_header(self, filename): - self._process_file(filename, self.output_h) - - def add_src(self, filename): - self._process_file(filename, self.output_c) + self._process_file(h_file, self.output_h) + return True + raise RuntimeError("Couldn't find include: " + include + ", h_files=" + repr(self.h_files)) # ---- main ---- -output_path = sys.argv[1] -prefix = sys.argv[2] -amalgamator = Amalgamator(output_path, prefix) -files = [] +c_out = sys.argv[1] +h_out = sys.argv[2] +print(c_out) +print(h_out) +amalgamator = Amalgamator(h_out, c_out) +c_files = [] +h_files = [] for arg in sys.argv[3:]: arg = arg.strip() - if arg.startswith("-I"): - amalgamator.add_include_path(arg[2:]) - elif arg.endswith(".h") or arg.endswith(".inc"): - pass + if arg.endswith(".h") or arg.endswith(".inc"): + h_files.append(arg) else: - files.append(arg) - -for filename in files: - amalgamator.add_src(filename) + c_files.append(arg) -amalgamator.finish() +amalgamator.amalgamate(h_files, c_files) diff --git a/bazel/build_defs.bzl b/bazel/build_defs.bzl index e5c270d057..224bc46c85 100644 --- a/bazel/build_defs.bzl +++ b/bazel/build_defs.bzl @@ -97,13 +97,6 @@ def _get_real_roots(files): roots[real_root] = True return roots.keys() -def _get_includes(files, strip_import_prefix): - roots = _get_real_roots(files) - includes = ["-I" + root for root in roots] - for include in strip_import_prefix: - includes += ["-I" + paths.join(root, include) for root in roots] - return includes - def make_shell_script(name, contents, out): contents = contents.replace("$", "$$") native.genrule( @@ -142,11 +135,11 @@ def _upb_amalgamation(ctx): inputs = [] for lib in ctx.attr.libs: inputs += lib[SrcList].srcs - srcs = [src for src in inputs if src.path.endswith("c")] + srcs = [src for src in inputs if not src.path.endswith("hpp")] ctx.actions.run( inputs = inputs, outputs = ctx.outputs.outs, - arguments = [ctx.bin_dir.path + "/", ctx.attr.prefix] + [f.path for f in srcs] + _get_includes(inputs, ctx.attr.strip_import_prefix), + arguments = [f.path for f in ctx.outputs.outs] + [f.path for f in srcs], progress_message = "Making amalgamation", executable = ctx.executable._amalgamator, ) diff --git a/cmake/BUILD.bazel b/cmake/BUILD.bazel index 10572f052c..48dad3c1b2 100644 --- a/cmake/BUILD.bazel +++ b/cmake/BUILD.bazel @@ -40,6 +40,7 @@ py_library( name = "staleness_test_lib", testonly = 1, srcs = ["staleness_test_lib.py"], + visibility = ["//visibility:public"], ) py_binary( diff --git a/cmake/build_defs.bzl b/cmake/build_defs.bzl index 9e30af952a..dd31230584 100644 --- a/cmake/build_defs.bzl +++ b/cmake/build_defs.bzl @@ -42,7 +42,7 @@ def generated_file_staleness_test(name, outs, generated_pattern, **kwargs): """ script_name = name + ".py" - script_src = ":staleness_test.py" + script_src = "@upb//cmake:staleness_test.py" # Filter out non-existing rules so Blaze doesn't error out before we even # run the test. @@ -67,7 +67,7 @@ def generated_file_staleness_test(name, outs, generated_pattern, **kwargs): data = existing_outs + [generated_pattern % file for file in outs], python_version = "PY3", deps = [ - ":staleness_test_lib", + "@upb//cmake:staleness_test_lib", ], **kwargs ) From 5cb177e09a0e84dc5bcf6ec3b860365131a033a7 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 3 Sep 2022 13:01:01 +0000 Subject: [PATCH 2/4] Removed unused code. --- bazel/amalgamate.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/bazel/amalgamate.py b/bazel/amalgamate.py index d60cba5d83..e7121c622d 100755 --- a/bazel/amalgamate.py +++ b/bazel/amalgamate.py @@ -35,11 +35,6 @@ def parse_include(line): match = INCLUDE_RE.match(line) return match.groups()[0] if match else None -def is_core_upb_header(fname): - if not fname.endswith(".h"): - return False - return fname.startswith("upb") or fname.startswith("google") - class Amalgamator: def __init__(self, h_out, c_out): self.include_paths = ["."] @@ -115,8 +110,6 @@ class Amalgamator: c_out = sys.argv[1] h_out = sys.argv[2] -print(c_out) -print(h_out) amalgamator = Amalgamator(h_out, c_out) c_files = [] h_files = [] From 81e716d459af2886402359302de023fe5e5b87c1 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Fri, 30 Sep 2022 17:41:33 +0000 Subject: [PATCH 3/4] Fixed to add missing table targets. --- BUILD | 2 ++ 1 file changed, 2 insertions(+) diff --git a/BUILD b/BUILD index 925696a339..b50c54a29b 100644 --- a/BUILD +++ b/BUILD @@ -985,6 +985,7 @@ upb_amalgamation( ":mini_table", ":port", ":reflection", + ":table_internal", ":upb", ], prefix = "php-", @@ -1019,6 +1020,7 @@ upb_amalgamation( ":mini_table", ":port", ":reflection", + ":table_internal", ":upb", ], prefix = "ruby-", From 18446cf64f00069638052c1bec30e1b6d3133fd9 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Fri, 30 Sep 2022 18:37:19 +0000 Subject: [PATCH 4/4] Removed unused "paths" import. --- bazel/build_defs.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/bazel/build_defs.bzl b/bazel/build_defs.bzl index 7add80a884..15be25c97e 100644 --- a/bazel/build_defs.bzl +++ b/bazel/build_defs.bzl @@ -25,7 +25,6 @@ """Internal rules for building upb.""" -load("@bazel_skylib//lib:paths.bzl", "paths") load(":upb_proto_library.bzl", "GeneratedSrcsInfo") _DEFAULT_CPPOPTS = []