Skip to content

Commit 208cef3

Browse files
authored
tools: simplify the format api to remove unnecessary xfrom related logic (#39697)
Commit Message: tools: simplify the format api to remove unnecessary xfrom related logic Additional Description: Now the `xformed` dep make no sense because its content is not used in the formatting. It's very helpful and make big help when we migrate from v2 -> v3. But now, it's time to clean up this unneessary logic. This PR change updated some logic of format api script. And I will create a PR to clean up all other xformed things after this PR is merged. Risk Level: low. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wangbaiping(wbpcode) <wbphub@gmail.com>
1 parent 76c48b4 commit 208cef3

2 files changed

Lines changed: 6 additions & 104 deletions

File tree

tools/proto_format/BUILD

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
load("@aspect_bazel_lib//lib:jq.bzl", "jq")
22
load("@envoy_repo//:path.bzl", "PATH")
3-
load("@rules_pkg//pkg:mappings.bzl", "pkg_files", "strip_prefix")
4-
load("@rules_pkg//pkg:pkg.bzl", "pkg_tar")
53
load("@rules_python//python:defs.bzl", "py_binary")
64
load("//tools/base:envoy_python.bzl", "envoy_genjson", "envoy_py_data", "envoy_pytool_binary")
75
load("//tools/python:namespace.bzl", "envoy_py_namespace")
@@ -75,18 +73,6 @@ envoy_py_data(
7573
src = ":data_srcs",
7674
)
7775

78-
pkg_files(
79-
name = "xformed_files",
80-
srcs = ["//tools/protoxform:api_protoxform"],
81-
strip_prefix = strip_prefix.from_pkg(),
82-
)
83-
84-
pkg_tar(
85-
name = "xformed",
86-
srcs = [":xformed_files"],
87-
extension = "tar.gz",
88-
)
89-
9076
py_binary(
9177
name = "format_api",
9278
srcs = ["format_api.py"],
@@ -103,13 +89,11 @@ genrule(
10389
cmd = """
10490
$(location :format_api) \
10591
--outfile=$@ \
106-
--xformed=$(location :xformed) \
10792
--build_file=$(location //tools/type_whisperer:api_build_file) \
10893
--protoprinted=$(location //tools/protoprint:protoprinted) \
10994
""",
11095
tools = [
11196
":format_api",
112-
":xformed",
11397
"//tools/protoprint:protoprinted",
11498
"//tools/type_whisperer:api_build_file",
11599
],

tools/proto_format/format_api.py

Lines changed: 6 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Mangle protoxform and protoprint artifacts.
44

55
import argparse
6-
from collections import defaultdict
76
import os
87
import pathlib
98
import re
@@ -95,70 +94,6 @@ def __init__(self, message):
9594
% message)
9695

9796

98-
def get_directory_from_package(package):
99-
"""Get directory path from package name or full qualified message name
100-
101-
Args:
102-
package: the full qualified name of package or message.
103-
"""
104-
return '/'.join(s for s in package.split('.') if s and s[0].islower())
105-
106-
107-
def get_destination_path(src):
108-
"""Obtain destination path from a proto file path by reading its package statement.
109-
110-
Args:
111-
src: source path
112-
"""
113-
src_path = pathlib.Path(src)
114-
contents = src_path.read_text(encoding='utf8')
115-
matches = PACKAGE_REGEX.findall(contents)
116-
if len(matches) != 1:
117-
raise RequiresReformatError(
118-
f"Expect {src} has only one package declaration but has {len(matches)}\n{contents}")
119-
package = matches[0]
120-
dst_path = pathlib.Path(
121-
get_directory_from_package(package)).joinpath(src_path.name.split('.')[0] + ".proto")
122-
# contrib API files have the standard namespace but are in a contrib folder for clarity.
123-
# The following prepends contrib for contrib packages so we wind up with the real final path.
124-
if 'contrib' in src:
125-
if 'v3alpha' not in package and 'v4alpha' not in package and package not in CONTRIB_V3_ALLOW_LIST:
126-
raise ProtoSyncError(
127-
"contrib extension package '{}' does not use v3alpha namespace. "
128-
"Add to CONTRIB_V3_ALLOW_LIST with an explanation if this is on purpose.".format(
129-
package))
130-
131-
dst_path = pathlib.Path('contrib').joinpath(dst_path)
132-
# Non-contrib can not use alpha.
133-
if not 'contrib' in src:
134-
if (not 'v2alpha' in package and not 'v1alpha1' in package) and 'alpha' in package:
135-
raise ProtoSyncError(
136-
"package '{}' uses an alpha namespace. This is not allowed. Instead mark with "
137-
"(xds.annotations.v3.file_status).work_in_progress or related annotation.".format(
138-
package))
139-
return dst_path
140-
141-
142-
def sync_proto_file(srcs, dst):
143-
"""Pretty-print a proto descriptor from protoxform.py Bazel cache artifacts."
144-
145-
Args:
146-
dst_srcs: destination/sources path tuple.
147-
"""
148-
assert (len(srcs) > 0)
149-
# If we only have one candidate source for a destination, just pretty-print.
150-
if len(srcs) == 1:
151-
src = srcs[0]
152-
else:
153-
# We should only see an active and next major version candidate from
154-
# previous version today.
155-
assert (len(srcs) == 2)
156-
src = [s for s in srcs if s.endswith('active_or_frozen.proto')][0]
157-
shutil.copy(src, dst)
158-
rel_dst_path = get_destination_path(src)
159-
return ['//%s:pkg' % str(rel_dst_path.parent)]
160-
161-
16297
def get_import_deps(proto_path):
16398
"""Obtain the Bazel dependencies for the import paths from a .proto file.
16499
@@ -295,7 +230,7 @@ def find_pkgs(package_version_status, api_root):
295230
return set([os.path.dirname(p)[len(api_root) + 1:] for p in api_protos])
296231

297232

298-
def format_api(mode, outfile, xformed, printed, build_file):
233+
def format_api(mode, outfile, printed, build_file):
299234

300235
with tempfile.TemporaryDirectory() as tmp:
301236
dst_dir = pathlib.Path(tmp)
@@ -304,27 +239,13 @@ def format_api(mode, outfile, xformed, printed, build_file):
304239
with tarfile.open(printed) as tar:
305240
tar.extractall(printed_dir)
306241

307-
xformed_dir = dst_dir.joinpath("xformed")
308-
xformed_dir.mkdir()
309-
with tarfile.open(xformed) as tar:
310-
tar.extractall(xformed_dir)
311-
312-
paths = []
313-
dst_src_paths = defaultdict(list)
314-
315242
for label in data["proto_targets"]:
316243
_label = label[len('@@envoy_api//'):].replace(':', '/')
317-
for suffix in ["active_or_frozen", "next_major_version_candidate"]:
318-
xpath = xformed_dir.joinpath(f"pkg/{_label}.{suffix}.proto")
319-
path = printed_dir.joinpath(f"{_label}.proto")
320-
321-
if xpath.exists() and os.stat(xpath).st_size > 0:
322-
target = dst_dir.joinpath(_label)
323-
target.parent.mkdir(exist_ok=True, parents=True)
324-
dst_src_paths[str(target)].append(str(path))
244+
source = printed_dir.joinpath(f"{_label}.proto")
245+
target = dst_dir.joinpath(_label)
246+
target.parent.mkdir(exist_ok=True, parents=True)
247+
shutil.copy(source, target)
325248

326-
for k, v in dst_src_paths.items():
327-
sync_proto_file(v, k)
328249
sync_build_files(mode, dst_dir)
329250

330251
# Add the build files
@@ -339,7 +260,6 @@ def format_api(mode, outfile, xformed, printed, build_file):
339260
active_pkgs=deps_format(active_pkgs), frozen_pkgs=deps_format(frozen_pkgs)))
340261

341262
shutil.rmtree(str(printed_dir))
342-
shutil.rmtree(str(xformed_dir))
343263
with tarfile.open(outfile, "w:gz") as tar:
344264
tar.add(dst_dir, arcname=".")
345265

@@ -349,11 +269,9 @@ def format_api(mode, outfile, xformed, printed, build_file):
349269
parser.add_argument('--mode', choices=['check', 'fix'])
350270
parser.add_argument('--outfile')
351271
parser.add_argument('--protoprinted')
352-
parser.add_argument('--xformed')
353272
parser.add_argument('--build_file')
354273
args = parser.parse_args()
355274

356275
format_api(
357-
args.mode, str(pathlib.Path(args.outfile).absolute()),
358-
str(pathlib.Path(args.xformed).absolute()), args.protoprinted,
276+
args.mode, str(pathlib.Path(args.outfile).absolute()), args.protoprinted,
359277
pathlib.Path(args.build_file))

0 commit comments

Comments
 (0)