language/proto: dependency resolution from prefixed roots

[alexeagle]

I'm working in a large monorepo migrating to Bazel. The proto imports do not currently match the repo structure. It seems there is no way to instruct the generation of proto_library targets to search subdirectories to resolve import statements.

At prior clients I've been forced to pre-factor all the code by adjusting proto import statements to match the repository structure, but it won't work this time, as the prior build system needs to keep working during the migration.

The affordances for generating import_prefix and strip_import_prefix only affect how protoc is called, not what deps gazelle will generate in the proto_library targets.

Repro

Since protoc allows multiple include directories, the existing protos are laid out this way:

├── BUILD
├── path
│   └── to
│       ├── app
│       │   └── api.proto
│       ├── root1
│       │   └── libA
│       │       └── a.proto
│       └── root2
│           └── libB
│               └── b.proto
└── WORKSPACE

And the dependency graph is api.proto -> b.proto -> a.proto. However the import statements look like so:

api.proto:

package app;

import "libB/b.proto";

b.proto:

package libB;

import "libA/a.proto";

(full tree: https://github.com/alexeagle/gazelle_proto_resolve_roots/commit/2ce22df2eb98ed1dede8a7cdc02f2e0c3fe45a6d )

Running gazelle in this repo produces non-functional proto_library targets, no surprise:

proto_library(
    name = "app_proto",
    srcs = ["api.proto"],
    visibility = ["//visibility:public"],
    deps = ["//libB:libB_proto"],  # <--- this is not the right package, needs path/to/root2
)

(full gazelle output: https://github.com/alexeagle/gazelle_proto_resolve_roots/commit/00edb8935c8b07db520d576dc126b74a10997f0a)

Analysis

Of course Gazelle does give an affordance for overriding dependency resolution with the #gazelle:resolve directive. In my repro I would add

# gazelle:resolve proto libA/a.proto //path/to/root1/libA:a_proto
# gazelle:resolve proto libB/b.proto //path/to/root2/libB:b_proto

to the root BUILD file (or some common ancestor of the code to be generated). This fixes the problem.

However, in my large monorepo there are 439 distinct import statements among the proto sources, so I'd have to write a tool to generate approx. that many #gazelle:resolve directives, then build some guardrails around that to ensure developers keep that list up-to-date as code changes. Not great.

Another option would be to re-organize all the code, eliminating the path/to/ segments, and collapsing root1 and root2 to the repository root directory. Then the import statements match the repository paths as Gazelle naively expects. However, there's another build system being used, and during the transition to Bazel both need to continue working. The effort to relocate all the proto files and update the legacy build system is deemed too high.

Proposal

Add an indexing operation similar to Gazelle extensions in other languages. When we walk the tree and find that a.proto contains a statement package libA we add this to the index as libA/a.proto -> path/to/root1/libA/a.proto. Later when resolving an import we'll find this entry and be able to generate correct deps for proto_library.

Similar issues:

• https://github.com/bazelbuild/bazel-gazelle/issues/1084 - applies to external, not deps within the same repo
• https://github.com/bazelbuild/bazel-gazelle/issues/897 - resolved with strip_prefix workaround, and applies to external repos

[linzhp]

Sorry for late reply. I checked out https://github.com/alexeagle/gazelle_proto_resolve_roots and add two gazelle:proto_strip_import_prefix directives, it seems to work:

diff --git a/path/to/root1/BUILD.bazel b/path/to/root1/BUILD.bazel
new file mode 100644
index 0000000..7418ed2
--- /dev/null
+++ b/path/to/root1/BUILD.bazel
@@ -0,0 +1 @@
+# gazelle:proto_strip_import_prefix /path/to/root1
diff --git a/path/to/root2/BUILD.bazel b/path/to/root2/BUILD.bazel
new file mode 100644
index 0000000..a592e2f
--- /dev/null
+++ b/path/to/root2/BUILD.bazel
@@ -0,0 +1 @@
+# gazelle:proto_strip_import_prefix /path/to/root2

What do you think?

[aptenodytes-forsteri]

What do you think?

So the proto_strip_import_prefix was there the whole time (looks like it has been around 3 years) and solves this exact problem? So I guess we just missed that directive?

[aptenodytes-forsteri]

Hm doesn't seem to work for my test case.

--- FAIL: TestFullGeneration (0.09s)
    --- FAIL: TestFullGeneration/include_paths (0.09s)
        files.go:185: include_paths/BUILD.bazel diff (-want,+got):
              (
                """
                # gazelle:go_generate_proto false
            -   # gazelle:proto_include_paths path/to/root_1,path/to/root_2,path/to/app
            +   # gazelle:proto_strip_import_prefix /path/to/root_1
            +   # gazelle:proto_strip_import_prefix /path/to/root_2
            +   # gazelle:proto_strip_import_prefix /path/to/app
                
                filegroup(
                ... // 10 identical lines
                """
              )
        files.go:185: include_paths/path/to/app/BUILD.bazel diff (-want,+got):
              (
                """
                ... // 3 identical lines
                    name = "app_proto",
                    srcs = ["api.proto"],
            +       strip_import_prefix = "/path/to/app",
                    visibility = ["//visibility:public"],
            -       deps = ["//path/to/root_2/libB:libB_proto"],
            +       deps = ["//libB:libB_proto"],
                )
                
                ... // 6 identical lines
                """
              )

Notice how the dep becomes //libB:libB_proto instead of //path/to/root_2/libB:libB_proto

https://github.com/bazelbuild/bazel-gazelle/blob/8a1359335163792ad660247dbd503dc08a0e5764/language/proto/testdata_full_gen_test/include_paths/BUILD.in

[aptenodytes-forsteri]

Ah but I guess the above comment didn't put the directives in the BUILD files where they appear to belong. So maybe what we missed is that we need to put those directives at the location where the prefix is stripped.

[linzhp]

Correct. My patch shows you where/how to put those directives.