Skip to content

Commit ce726b0

Browse files
Fixes that needs to be available in the main branch in order to cherry-pick into Release branch (#49)
* Update WORKSPACE * Update workspace.bzl * Migrate to Protobuf 4.23.4: custom Bazel rule for Python proto generation Protobuf 4.x removed py_proto_library and changed proto_library behavior. This commit: - Creates _py_proto_library_rule: a custom Bazel rule that accepts ProtoInfo or PyInfo, runs protoc to generate Python _pb2.py files, and provides PyInfo for Python deps - Replaces deprecated py_proto_library calls with custom rule implementation - Configures protoc proto_path to include workspace dirs and external dependencies - Adds local any.proto copies to bypass sandbox limitations in proto imports - Applies compatibility patches to TensorFlow and TensorFlow Metadata dependencies - Updates all s2t_proto_library_py calls to work with new implementation Fixes: Protobuf 4.23.4 compatibility for struct2tensor build system * Add compatibility patches for older Abseil and Protobuf 4.x This patch extends tensorflow.patch with changes to support: 1. Abseil backward compatibility: - Add absl_base_prefetch.h compatibility shim for older Abseil versions that lack absl/base/prefetch.h - Update prefetch includes across TensorFlow/TSL to use the shim - Add inline implementation of NullTerminatedMessage for older Abseil versions without StatusMessageAsCStr 2. Protobuf 4.x support: - Refactor cc_proto_library to use native proto_library and cc_proto_library rules instead of custom proto_gen - Implement custom _tsl_py_proto_library_rule to replace the built-in py_proto_library removed in Protobuf 4.x - Update proto library generation to depend on generated cc_proto_library targets 3. Build cleanup: - Remove unused absl/strings:string_view dependency - Update BUILD files to reflect new dependency structure These changes enable building struct2tensor with older Abseil versions while supporting Protobuf 4.x, improving compatibility across different dependency versions. * chore: upgrade Protobuf dependency from 4.23.4 to 4.25.6 * Move TFMD patch to tfmd.patch; Protobuf 4.x compatibility Remove vendored google/protobuf/any.proto and obsolete third_party/README.md * fix: Use native prefix for Bazel rules in macros Prefix cc_binary and cc_library with native. in struct2tensor.bzl * Update Dockerfile * Fix struct2tensor integration in TensorFlow Serving Docker build Apply rules_cc patch before adding struct2tensor local repository to ensure proper Bazel configuration. * Update RELEASE.md to sync with main branch * Update version.py --------- Co-authored-by: Gagandeep Singh <gdp.1807@gmail.com>
1 parent 5ce1eec commit ce726b0

12 files changed

Lines changed: 941 additions & 45 deletions

File tree

WORKSPACE

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ tf_configure(name = "local_config_tf")
4141
# 3. Request the new archive to be mirrored on mirror.bazel.build for more
4242
# reliable downloads.
4343

44-
_TENSORFLOW_GIT_COMMIT = "5bc9d26649cca274750ad3625bd93422617eed4b" # tf 2.16.1
45-
_TENSORFLOW_ARCHIVE_SHA256 = "fe592915c85d1a89c20f3dd89db0772ee22a0fbda78e39aa46a778d638a96abc"
44+
_TENSORFLOW_GIT_COMMIT = "3c92ac03cab816044f7b18a86eb86aa01a294d95" # tf 2.17.1
45+
_TENSORFLOW_ARCHIVE_SHA256 = "317dd95c4830a408b14f3e802698eb68d70d81c7c7cfcd3d28b0ba023fe84a68"
4646

4747
http_archive(
4848
name = "org_tensorflow",
@@ -51,6 +51,8 @@ http_archive(
5151
"https://github.com/tensorflow/tensorflow/archive/%s.tar.gz" % _TENSORFLOW_GIT_COMMIT,
5252
],
5353
strip_prefix = "tensorflow-%s" % _TENSORFLOW_GIT_COMMIT,
54+
patches = ["//third_party:tensorflow.patch"],
55+
patch_args = ["-p1"],
5456
)
5557

5658
load("//third_party:python_configure.bzl", "local_python_configure")
@@ -62,6 +64,21 @@ local_python_configure(name = "local_execution_config_python")
6264
load("//struct2tensor:workspace.bzl", "struct2tensor_workspace")
6365
struct2tensor_workspace()
6466

67+
# ===== Protobuf 4.25.6 dependency =====
68+
# Must be declared BEFORE TensorFlow's workspaces to override the version they pull
69+
http_archive(
70+
name = "com_google_protobuf",
71+
sha256 = "4e6727bc5d23177edefa3ad86fd2f5a92cd324151636212fd1f7f13aef3fd2b7",
72+
strip_prefix = "protobuf-4.25.6",
73+
urls = [
74+
"https://github.com/protocolbuffers/protobuf/archive/v4.25.6.tar.gz",
75+
],
76+
)
77+
78+
# Load Protobuf dependencies
79+
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
80+
protobuf_deps()
81+
6582
# Initialize TensorFlow's external dependencies.
6683
load("@org_tensorflow//tensorflow:workspace3.bzl", "tf_workspace3")
6784
tf_workspace3()

struct2tensor/BUILD

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ s2t_pytype_library(
8484
"path.py",
8585
],
8686
deps = [
87-
"@com_github_tensorflow_metadata//tensorflow_metadata/proto/v0:py_metadata_v0_proto_py",
87+
"@com_github_tensorflow_metadata//tensorflow_metadata/proto/v0:metadata_v0_proto_py_pb2",
8888
"@com_google_protobuf//:protobuf_python",
8989
],
9090
)

struct2tensor/proto/BUILD

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ licenses(["notice"])
99
s2t_proto_library(
1010
name = "query_metadata_proto",
1111
srcs = ["query_metadata.proto"],
12-
deps = ["@com_github_tensorflow_metadata//tensorflow_metadata/proto/v0:cc_metadata_v0_proto_cc"],
12+
deps = ["@com_github_tensorflow_metadata//tensorflow_metadata/proto/v0:metadata_v0_proto"],
1313
)
1414

1515
s2t_proto_library_cc(
@@ -19,10 +19,9 @@ s2t_proto_library_cc(
1919

2020
s2t_proto_library_py(
2121
name = "query_metadata_py_pb2",
22-
srcs = ["query_metadata.proto"],
2322
api_version = 2,
2423
oss_deps = [
25-
"@com_github_tensorflow_metadata//tensorflow_metadata/proto/v0:py_metadata_v0_proto_py",
24+
"@com_github_tensorflow_metadata//tensorflow_metadata/proto/v0:metadata_v0_proto_py_pb2",
2625
],
2726
proto_library = "query_metadata_proto",
2827
)

struct2tensor/struct2tensor.bzl

Lines changed: 246 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,224 @@
1414

1515
"""Bazel macros used in OSS."""
1616

17-
load("@com_google_protobuf//:protobuf.bzl", "cc_proto_library", "py_proto_library")
18-
load("@rules_cc//cc:cc_binary.bzl", "cc_binary")
19-
load("@rules_cc//cc:cc_library.bzl", "cc_library")
17+
def _py_proto_library_impl(ctx):
18+
"""Implementation of py_proto_library rule."""
19+
proto_deps = ctx.attr.deps
20+
21+
# Separate proto and Python dependencies
22+
all_sources = []
23+
py_infos = []
24+
25+
for dep in proto_deps:
26+
if ProtoInfo in dep:
27+
# It's a proto_library - collect proto sources
28+
all_sources.extend(dep[ProtoInfo].direct_sources)
29+
elif PyInfo in dep:
30+
# It's already a py_library - collect its PyInfo for passthrough
31+
py_infos.append(dep[PyInfo])
32+
33+
# Filter to only include sources from the workspace (not external packages)
34+
# We can only declare outputs in our own package
35+
workspace_sources = []
36+
for src in all_sources:
37+
# Filter out external sources (they start with external/ or ..)
38+
if not src.short_path.startswith("external/") and not src.short_path.startswith("../"):
39+
workspace_sources.append(src)
40+
41+
# Generate Python output files from proto sources
42+
py_outputs = []
43+
for proto_src in workspace_sources:
44+
# Use just the basename to avoid path issues
45+
basename = proto_src.basename[:-6] # Remove .proto
46+
py_file = ctx.actions.declare_file(basename + "_pb2.py")
47+
py_outputs.append(py_file)
48+
49+
if py_outputs:
50+
# Build proto_path arguments for protoc
51+
# We need to include paths for workspace root and external dependencies
52+
proto_path_args = []
53+
54+
# Add current directory to find workspace proto files
55+
proto_path_args.append("--proto_path=.")
56+
57+
# Collect proto_path entries from all transitive dependencies
58+
# Use dictionary as a set (Starlark doesn't have set type)
59+
proto_paths = {".": True}
60+
61+
# Also add directories of workspace sources so imports like "any.proto"
62+
# (in the same folder) resolve correctly.
63+
for ws in workspace_sources:
64+
ws_dir = "/".join(ws.short_path.split("/")[:-1])
65+
if ws_dir and ws_dir not in proto_paths:
66+
proto_paths[ws_dir] = True
67+
proto_path_args.append("--proto_path=" + ws_dir)
68+
69+
for dep in proto_deps:
70+
if ProtoInfo in dep:
71+
# Add proto_source_root if available
72+
if hasattr(dep[ProtoInfo], 'proto_source_root'):
73+
root = dep[ProtoInfo].proto_source_root
74+
if root and root not in proto_paths:
75+
proto_paths[root] = True
76+
proto_path_args.append("--proto_path=" + root)
77+
78+
# Also derive from file paths for more coverage
79+
for src in dep[ProtoInfo].transitive_sources.to_list():
80+
# Use the directory containing the proto file's import root
81+
# For external/com_google_protobuf/src/google/protobuf/any.proto,
82+
# we want external/com_google_protobuf/src
83+
if src.path.startswith("external/com_google_protobuf/"):
84+
proto_path = "external/com_google_protobuf/src"
85+
if proto_path not in proto_paths:
86+
proto_paths[proto_path] = True
87+
proto_path_args.append("--proto_path=" + proto_path)
88+
elif src.path.startswith("external/"):
89+
# For other external repos like tensorflow_metadata
90+
# Extract external/repo_name
91+
parts = src.path.split("/")
92+
if len(parts) >= 2:
93+
proto_path = "/".join(parts[:2])
94+
if proto_path not in proto_paths:
95+
proto_paths[proto_path] = True
96+
proto_path_args.append("--proto_path=" + proto_path)
97+
98+
# Also add Bazel root paths
99+
if src.root.path and src.root.path not in proto_paths:
100+
proto_paths[src.root.path] = True
101+
proto_path_args.append("--proto_path=" + src.root.path)
102+
103+
# Build list of proto file paths - only include workspace sources
104+
proto_file_args = []
105+
for src in workspace_sources:
106+
proto_file_args.append(src.short_path)
107+
108+
# Run protoc to generate Python files
109+
# Use ctx.bin_dir.path as the output directory root
110+
output_root = ctx.bin_dir.path
111+
112+
ctx.actions.run(
113+
# Include workspace sources plus all transitive dependencies for imports
114+
inputs = depset(direct = workspace_sources, transitive = [
115+
dep[ProtoInfo].transitive_sources for dep in proto_deps if ProtoInfo in dep
116+
]),
117+
outputs = py_outputs,
118+
executable = ctx.executable._protoc,
119+
arguments = [
120+
"--python_out=" + output_root,
121+
] + proto_path_args + proto_file_args,
122+
mnemonic = "ProtocPython",
123+
)
124+
125+
# Collect transitive sources from both generated files and Python deps
126+
all_transitive_sources = [depset(py_outputs)]
127+
all_imports = [depset([ctx.bin_dir.path])] if py_outputs else []
128+
129+
for py_info in py_infos:
130+
all_transitive_sources.append(py_info.transitive_sources)
131+
if hasattr(py_info, 'imports'):
132+
all_imports.append(py_info.imports)
133+
134+
# Return PyInfo provider so this can be used as a py_library dependency
135+
# Merge proto-generated files with passthrough Python dependencies
136+
return [
137+
DefaultInfo(files = depset(py_outputs)),
138+
PyInfo(
139+
transitive_sources = depset(transitive = all_transitive_sources),
140+
imports = depset(transitive = all_imports),
141+
has_py2_only_sources = False,
142+
has_py3_only_sources = True,
143+
),
144+
]
145+
146+
_py_proto_library_rule = rule(
147+
implementation = _py_proto_library_impl,
148+
attrs = {
149+
"deps": attr.label_list(
150+
providers = [[ProtoInfo], [PyInfo]], # Accept either ProtoInfo OR PyInfo
151+
doc = "Proto library or Python library dependencies",
152+
),
153+
"_protoc": attr.label(
154+
default = "@com_google_protobuf//:protoc",
155+
executable = True,
156+
cfg = "exec",
157+
),
158+
},
159+
provides = [PyInfo],
160+
)
161+
162+
# Wrapper for cc_proto_library to maintain compatibility with old Protobuf 3.x API
163+
def cc_proto_library(
164+
name,
165+
srcs = [],
166+
deps = [],
167+
cc_libs = [],
168+
protoc = None,
169+
default_runtime = None,
170+
use_grpc_plugin = None,
171+
testonly = 0,
172+
visibility = None,
173+
**kwargs):
174+
"""Wrapper for cc_proto_library that works with Protobuf 4.x."""
175+
_ignore = [cc_libs, protoc, default_runtime, use_grpc_plugin, kwargs]
176+
177+
# Create proto_library first
178+
native.proto_library(
179+
name = name + "_proto",
180+
srcs = srcs,
181+
deps = [d + "_proto" if not d.startswith("@") else d for d in deps],
182+
testonly = testonly,
183+
visibility = visibility,
184+
)
185+
186+
# Create cc_proto_library that depends on proto_library
187+
native.cc_proto_library(
188+
name = name,
189+
deps = [":" + name + "_proto"],
190+
testonly = testonly,
191+
visibility = visibility,
192+
)
20193

21194
def s2t_pytype_library(
22195
name,
23196
srcs = [],
24197
deps = [],
25198
srcs_version = "PY3ONLY",
26199
testonly = False):
27-
native.py_library(name = name, srcs = srcs, deps = deps, testonly = testonly)
200+
"""Python library that automatically wraps proto_library deps with PyInfo.
201+
202+
This wrapper wraps all dependencies with our custom py_proto_library_rule.
203+
Dependencies that don't provide ProtoInfo will fail with a clear error.
204+
Dependencies that do provide ProtoInfo (proto_library targets) will get PyInfo.
205+
"""
206+
# Process dependencies to wrap them all with our custom rule
207+
processed_deps = []
208+
for dep in deps:
209+
# Skip protobuf_python - it's already a proper Python library
210+
if dep == "@com_google_protobuf//:protobuf_python":
211+
processed_deps.append(dep)
212+
continue
213+
214+
# Create a safe wrapper name for this dependency
215+
safe_dep_name = dep.replace(":", "_").replace("//", "").replace("/", "_").replace("@", "").replace("-", "_").replace(".", "_")
216+
wrapper_name = name + "_proto_wrapper_" + safe_dep_name
217+
218+
# Wrap all dependencies with our custom py_proto_library rule
219+
# If the dep provides ProtoInfo, this will work and provide PyInfo
220+
# If it doesn't provide ProtoInfo, it will fail with a clear error
221+
_py_proto_library_rule(
222+
name = wrapper_name,
223+
deps = [dep],
224+
testonly = testonly,
225+
)
226+
processed_deps.append(":" + wrapper_name)
227+
228+
native.py_library(
229+
name = name,
230+
srcs = srcs,
231+
deps = processed_deps,
232+
testonly = testonly,
233+
)
234+
28235

29236
def s2t_proto_library(
30237
name,
@@ -52,18 +259,22 @@ def s2t_proto_library(
52259
testonly = testonly,
53260
)
54261

55-
use_grpc_plugin = None
56-
if cc_grpc_version:
57-
use_grpc_plugin = True
262+
# Create a native proto_library for Python generation
263+
# This is needed by s2t_proto_library_py
264+
proto_lib_deps = [d + "_proto" if not d.startswith("@") else d for d in deps]
265+
native.proto_library(
266+
name = name + "_proto",
267+
srcs = srcs,
268+
deps = proto_lib_deps,
269+
visibility = visibility,
270+
testonly = testonly,
271+
)
58272

59-
# TODO(martinz): replace with proto_library, when that works.
60-
cc_proto_library(
273+
# Create cc_proto_library that depends on the proto_library we just created
274+
# Don't use our cc_proto_library wrapper to avoid duplicate proto_library creation
275+
native.cc_proto_library(
61276
name = name,
62-
srcs = srcs,
63-
deps = deps,
64-
cc_libs = ["@com_google_protobuf//:protobuf"],
65-
protoc = "@com_google_protobuf//:protoc",
66-
default_runtime = "@com_google_protobuf//:protobuf",
277+
deps = [":" + name + "_proto"],
67278
testonly = testonly,
68279
visibility = visibility,
69280
)
@@ -76,7 +287,7 @@ DYNAMIC_DEPS = ["@local_config_tf//:libtensorflow_framework", "@local_config_tf/
76287

77288
def s2t_dynamic_binary(name, deps):
78289
"""Creates a .so file intended for linking with tensorflow_framework.so."""
79-
cc_binary(
290+
native.cc_binary(
80291
name = name,
81292
copts = DYNAMIC_COPTS,
82293
linkshared = 1,
@@ -89,7 +300,7 @@ def s2t_dynamic_library(
89300
deps = None):
90301
"""Creates a static library intended for linking with tensorflow_framework.so."""
91302
true_deps = [] if deps == None else deps
92-
cc_library(
303+
native.cc_library(
93304
name = name,
94305
srcs = srcs,
95306
alwayslink = 1,
@@ -169,15 +380,25 @@ def s2t_proto_library_cc(
169380
)
170381

171382
def s2t_proto_library_py(name, proto_library, srcs = [], deps = [], oss_deps = [], visibility = None, testonly = 0, api_version = None):
172-
"""Opensource py_proto_library."""
173-
_ignore = [proto_library, api_version]
174-
py_proto_library(
383+
"""Opensource py_proto_library.
384+
385+
Uses a custom rule implementation that properly generates Python from proto_library
386+
and provides PyInfo for Python library dependencies.
387+
388+
Note: s2t_proto_library creates {name}_proto for the proto_library, so we append _proto.
389+
"""
390+
_ignore = [api_version, srcs, deps]
391+
392+
if not proto_library:
393+
fail("proto_library parameter is required for s2t_proto_library_py")
394+
395+
# s2t_proto_library creates a proto_library named {name}_proto
396+
# So we need to reference it correctly
397+
actual_proto_library = ":" + proto_library + "_proto"
398+
399+
# Use our custom py_proto_library rule
400+
_py_proto_library_rule(
175401
name = name,
176-
srcs = srcs,
177-
srcs_version = "PY3ONLY",
178-
deps = ["@com_google_protobuf//:well_known_types_py_pb2"] + oss_deps,
179-
default_runtime = "@com_google_protobuf//:protobuf_python",
180-
protoc = "@com_google_protobuf//:protoc",
402+
deps = [actual_proto_library] + oss_deps,
181403
visibility = visibility,
182-
testonly = testonly,
183404
)

0 commit comments

Comments
 (0)