pw_protobuf_compiler: Don't rely on options file ordering

Instead of assuming pwpb_options appear first, explicitly process them
first, falling back on regular .options.

Change-Id: Id5cb6d4b88feb60da36d408659b56363db276b0b
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/247472
Presubmit-Verified: CQ Bot Account <[email protected]>
Reviewed-by: Wyatt Hepler <[email protected]>
Lint: Lint 🤖 <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Pigweed-Auto-Submit: Alexei Frolov <[email protected]>
Docs-Not-Needed: Alexei Frolov <[email protected]>

pw_protobuf/py/pw_protobuf/options.py

@@ -14,7 +14,6 @@
 """Options file parsing for proto generation."""
 
 from fnmatch import fnmatchcase
-import itertools
 from pathlib import Path
 import re
 
@@ -54,40 +53,46 @@ def load_options_from(options: ParsedOptions, options_file_name: Path):
                 continue
 
 
-_PWPB_OPTIONS_EXTENSION = '.pwpb_options'
-
-
-def load_options(
+def _load_options_with_suffix(
     include_paths: list[Path],
     proto_file_name: Path,
     options_files: list[Path],
-    allow_generic_options_extension: bool = False,
+    suffix: str,
 ) -> ParsedOptions:
     """Loads the options for the given .proto file."""
     options: ParsedOptions = []
 
-    suffixes = [_PWPB_OPTIONS_EXTENSION]
-    if allow_generic_options_extension:
-        suffixes.append('.options')
-
     # First load from any files that were directly specified.
-    for options_file, suffix in itertools.product(options_files, suffixes):
+    for options_file in options_files:
         if (
             options_file.name == proto_file_name.with_suffix(suffix).name
             and options_file.exists()
         ):
             load_options_from(options, options_file)
-            break  # .pwpb_options is always first; stop if it is found.
 
     # Then search specified search paths for options files.
     for include_path in include_paths:
-        for suffix in suffixes:
-            options_file_name = include_path / proto_file_name.with_suffix(
-                suffix
-            )
-            if options_file_name.exists():
-                load_options_from(options, options_file_name)
-                break  # .pwpb_options is always first; stop if it is found.
+        options_file_name = include_path / proto_file_name.with_suffix(suffix)
+        if options_file_name.exists():
+            load_options_from(options, options_file_name)
+
+    return options
+
+
+def load_options(
+    include_paths: list[Path],
+    proto_file_name: Path,
+    options_files: list[Path],
+    allow_generic_options_extension: bool = True,
+) -> ParsedOptions:
+    # Try to load pwpb_options first. If they exist, ignore regular options.
+    options = _load_options_with_suffix(
+        include_paths, proto_file_name, options_files, ".pwpb_options"
+    )
+    if allow_generic_options_extension and not options:
+        options = _load_options_with_suffix(
+            include_paths, proto_file_name, options_files, ".options"
+        )
 
     return options
 

pw_protobuf_compiler/pw_proto_library.bzl

@@ -667,29 +667,25 @@ _pw_nanopb_rpc_proto_library = rule(
 
 def _pw_proto_filegroup_impl(ctx):
     source_files = list()
-    pwpb_options_files = list()
-    legacy_options_files = list()
+    options_files = list()
 
     for src in ctx.attr.srcs:
         source_files += src.files.to_list()
 
     for options_src in ctx.attr.options_files:
         for file in options_src.files.to_list():
-            if file.extension == "pwpb_options":
-                pwpb_options_files.append(file)
-            elif file.extension == "options":
-                legacy_options_files.append(file)
+            if file.extension == "options" or file.extension == "pwpb_options":
+                options_files.append(file)
             else:
                 fail((
                     "Files provided as `options_files` to a " +
                     "`pw_proto_filegroup` must have the `.options` or " +
                     "`.pwpb_options` extensions; the file `{}` was provided."
                 ).format(file.basename))
 
-    # Ensure that .pwpb_options appear before regular .options.
     return [
         DefaultInfo(files = depset(source_files)),
-        PwProtoOptionsInfo(options_files = depset(pwpb_options_files + legacy_options_files)),
+        PwProtoOptionsInfo(options_files = depset(options_files)),
     ]
 
 pw_proto_filegroup = rule(