api: generate whole directory and sync

tools/proto_format.sh

@@ -25,4 +25,4 @@ bazel build ${BAZEL_BUILD_OPTIONS} --//tools/api_proto_plugin:default_type_db_ta
   @envoy_api//docs:protos --aspects //tools/protoxform:protoxform.bzl%protoxform_aspect --output_groups=proto \
   --action_env=CPROFILE_ENABLED=1 --host_force_python=PY3
 
-./tools/proto_sync.py "$1" ${PROTO_TARGETS}
+./tools/proto_sync.py "--mode=$1" ${PROTO_TARGETS}

tools/proto_sync.py

@@ -2,13 +2,15 @@
 
 # Diff or copy protoxform artifacts from Bazel cache back to the source tree.
 
+import argparse
 import os
 import pathlib
 import re
 import shutil
 import string
 import subprocess
 import sys
+import tempfile
 
 from api_proto_plugin import utils
 
@@ -44,6 +46,7 @@
 
 IMPORT_REGEX = re.compile('import "(.*)";')
 SERVICE_REGEX = re.compile('service \w+ {')
+PACKAGE_REGEX = re.compile('\npackage ([^="]*);')
 PREVIOUS_MESSAGE_TYPE_REGEX = re.compile(r'previous_message_type\s+=\s+"([^"]*)";')
 
 
@@ -59,79 +62,44 @@ def __init__(self, message):
         message)
 
 
-def LabelPaths(label, src_suffix):
-  """Compute single proto file source/destination paths from a Bazel proto label.
+def GetDirectoryFromPackage(package):
+  """Get directory path from package name or full qualified message name
 
   Args:
-    label: Bazel source proto label string.
-    src_suffix: suffix string to append to source path.
-
-  Returns:
-    source, destination path tuple. The source indicates where in the Bazel
-      cache the protoxform.py artifact with src_suffix can be found. The
-      destination is a provisional path in the Envoy source tree for copying the
-      contents of source when run in fix mode.
+    package: the full qualified name of package or message.
   """
-  src = utils.BazelBinPathForOutputArtifact(label, src_suffix)
-  dst = 'api/%s' % utils.ProtoFileCanonicalFromLabel(label)
-  return src, dst
+  return '/'.join(s for s in package.split('.') if s and s[0].islower())
 
 
-def SyncProtoFile(cmd, src, dst):
-  """Diff or in-place update a single proto file from protoxform.py Bazel cache artifacts."
+def GetDestinationPath(src):
+  """Obtain destination path from a proto file path by reading its package statement.
 
   Args:
-    cmd: 'check' or 'fix'.
-    src: source path.
-    dst: destination path.
-  """
-  if cmd == 'fix':
-    shutil.copyfile(src, dst)
-  else:
-    try:
-      subprocess.check_call(['diff', src, dst])
-    except subprocess.CalledProcessError:
-      raise RequiresReformatError('%s and %s do not match' % (src, dst))
-
-
-def SyncV2(cmd, src_labels):
-  """Diff or in-place update v2 protos from protoxform.py Bazel cache artifacts."
-
-  Args:
-    cmd: 'check' or 'fix'.
-    src_labels: Bazel label for source protos.
+    src: source path
   """
-  for s in src_labels:
-    src, dst = LabelPaths(s, '.v2.proto')
-    SyncProtoFile(cmd, src, dst)
+  src_path = pathlib.Path(src)
+  contents = src_path.read_text(encoding='utf8')
+  matches = re.findall(PACKAGE_REGEX, contents)
+  if len(matches) != 1:
+    raise RequiresReformatError("Expect {} has only one package declaration but has {}".format(
+        src, len(matches)))
+  return pathlib.Path(GetDirectoryFromPackage(
+      matches[0])).joinpath(src_path.name.split('.')[0] + ".proto")
 
 
-def SyncV3Alpha(cmd, src_labels):
-  """Diff or in-place update v3alpha protos from protoxform.py Bazel cache artifacts."
+def SyncProtoFile(cmd, src, dst_root):
+  """Diff or in-place update a single proto file from protoxform.py Bazel cache artifacts."
 
   Args:
     cmd: 'check' or 'fix'.
-    src_labels: Bazel label for source protos.
+    src: source path.
   """
-  for s in src_labels:
-    src, dst = LabelPaths(s, '.v3alpha.proto')
-    # Skip empty files, this indicates this file isn't modified in next version.
-    if os.stat(src).st_size == 0:
-      continue
-    # Skip unversioned package namespaces. TODO(htuch): fix this to use the type
-    # DB and proper upgrade paths.
-    if 'v1' in dst:
-      dst = re.sub('v1alpha\d?|v1', 'v3alpha', dst)
-      SyncProtoFile(cmd, src, dst)
-    elif 'v2' in dst:
-      dst = re.sub('v2alpha\d?|v2', 'v3alpha', dst)
-      SyncProtoFile(cmd, src, dst)
-    elif 'envoy/type/matcher' in dst:
-      dst = re.sub('/type/matcher/', '/type/matcher/v3alpha/', dst)
-      SyncProtoFile(cmd, src, dst)
-    elif 'envoy/type' in dst:
-      dst = re.sub('/type/', '/type/v3alpha/', dst)
-      SyncProtoFile(cmd, src, dst)
+  # Skip empty files, this indicates this file isn't modified in this version.
+  if os.stat(src).st_size == 0:
+    return
+  dst = dst_root.joinpath(GetDestinationPath(src))
+  dst.parent.mkdir(0o755, True, True)
+  shutil.copyfile(src, str(dst))
 
 
 def GetImportDeps(proto_path):
@@ -162,7 +130,7 @@ def GetImportDeps(proto_path):
           continue
         if import_path.startswith('envoy/'):
           # Ignore package internal imports.
-          if os.path.dirname(os.path.join('api', import_path)) == os.path.dirname(proto_path):
+          if os.path.dirname(proto_path).endswith(os.path.dirname(import_path)):
             continue
           imports.append('//%s:pkg' % os.path.dirname(import_path))
           continue
@@ -187,7 +155,7 @@ def GetPreviousMessageTypeDeps(proto_path):
   matches = re.findall(PREVIOUS_MESSAGE_TYPE_REGEX, contents)
   deps = []
   for m in matches:
-    target = '//%s:pkg' % '/'.join(s for s in m.split('.') if s and s[0].islower())
+    target = '//%s:pkg' % GetDirectoryFromPackage(m)
     deps.append(target)
   return deps
 
@@ -237,34 +205,68 @@ def BuildFileContents(root, files):
   return BUILD_FILE_TEMPLATE.substitute(fields=formatted_fields)
 
 
-def SyncBuildFiles(cmd):
+def SyncBuildFiles(cmd, dst_root):
   """Diff or in-place update api/ BUILD files.
 
   Args:
     cmd: 'check' or 'fix'.
   """
-  for root, dirs, files in os.walk('api/'):
+  for root, dirs, files in os.walk(str(dst_root)):
     is_proto_dir = any(f.endswith('.proto') for f in files)
     if not is_proto_dir:
       continue
     build_contents = BuildFileContents(root, files)
     build_path = os.path.join(root, 'BUILD')
-    if cmd == 'fix':
-      with open(build_path, 'w') as f:
-        f.write(build_contents)
-    else:
-      with open(build_path, 'r') as f:
-        if build_contents != f.read():
-          raise RequiresReformatError('%s is not canonically formatted' % build_path)
+    with open(build_path, 'w') as f:
+      f.write(build_contents)
+
+
+def GenerateCurrentApiDir(api_dir, dst_dir):
+  """Helper function to generate original API repository to be compared with diff.
+  This copies the original API repository and deletes file we don't want to compare.
+
+  Args:
+    api_dir: the original api directory
+    dst_dir: the api directory to be compared in temporary directory
+  """
+  dst = dst_dir.joinpath("envoy")
+  shutil.copytree(str(api_dir.joinpath("envoy")), str(dst))
+
+  for p in dst.glob('**/*.md'):
+    p.unlink()
+  # envoy.service.auth.v2alpha exist for compatibility while we don't run in protoxform
+  # so we ignore it here.
+  shutil.rmtree(str(dst.joinpath("service", "auth", "v2alpha")))
 
 
 if __name__ == '__main__':
-  cmd = sys.argv[1]
-  src_labels = sys.argv[2:]
-  try:
-    SyncV2(cmd, src_labels)
-    SyncV3Alpha(cmd, src_labels)
-    SyncBuildFiles(cmd)
-  except ProtoSyncError as e:
-    sys.stderr.write('%s\n' % e)
-    sys.exit(1)
+  parser = argparse.ArgumentParser()
+  parser.add_argument('--mode', choices=['check', 'fix'])
+  parser.add_argument('--api_repo', default='envoy_api')
+  parser.add_argument('--api_root', default='api')
+  parser.add_argument('labels', nargs='*')
+  args = parser.parse_args()
+
+  with tempfile.TemporaryDirectory() as tmp:
+    dst_dir = pathlib.Path(tmp).joinpath("b")
+    for label in args.labels:
+      SyncProtoFile(args.mode, utils.BazelBinPathForOutputArtifact(label, '.v2.proto'), dst_dir)
+      SyncProtoFile(args.mode, utils.BazelBinPathForOutputArtifact(label, '.v3alpha.proto'),
+                    dst_dir)
+    SyncBuildFiles(args.mode, dst_dir)
+
+    current_api_dir = pathlib.Path(tmp).joinpath("a")
+    current_api_dir.mkdir(0o755, True, True)
+    api_root = pathlib.Path(args.api_root)
+    GenerateCurrentApiDir(api_root, current_api_dir)
+
+    diff = subprocess.run(['diff', '-Npur', "a", "b"], cwd=tmp, stdout=subprocess.PIPE).stdout
+
+    if diff.strip():
+      if args.mode == "check":
+        print("Please apply following patch to directory '{}'".format(args.api_root),
+              file=sys.stderr)
+        print(diff.decode(), file=sys.stderr)
+        sys.exit(1)
+      if args.mode == "fix":
+        subprocess.run(['patch', '-p1'], input=diff, cwd=str(api_root.resolve()))