rules_python icon indicating copy to clipboard operation
rules_python copied to clipboard

[Gazelle][proto] How to automatically resolve protobuf target

Open cyrilhuang opened this issue 1 year ago • 8 comments
trafficstars

🐞 bug report

Affected Rule

gazelle resolve

Is this a regression?

No

Description

I have a python file with a proto definition imported. When I try to use gazelle to auto generate py_library target, it fails to validate dependency. I look into some code in bazel_gazelle/resolve/index.go, no proto target defined in there.

🔬 Minimal Reproduction

  
import a.b.c_pb2
  

and then run bazel run :py_gazelle

🔥 Exception or Error

  
gazelle: ERROR: failed to validate dependencies for target "//:check": "a.b.c_pb2" at line 11 from "check.py" is an invalid dependency: possible solutions:
	1. Add it as a dependency in the requirements.txt file.
	2. Instruct Gazelle to resolve to a known dependency using the gazelle:resolve directive.
	3. Ignore it with a comment '# gazelle:ignore a.b.c_pb2' in the Python file.

"a.b" at line 11 from "check.py" is an invalid dependency: possible solutions:
	1. Add it as a dependency in the requirements.txt file.
	2. Instruct Gazelle to resolve to a known dependency using the gazelle:resolve directive.
	3. Ignore it with a comment '# gazelle:ignore a.b' in the Python file.

"a" at line 11 from "check.py" is an invalid dependency: possible solutions:
	1. Add it as a dependency in the requirements.txt file.
	2. Instruct Gazelle to resolve to a known dependency using the gazelle:resolve directive.
	3. Ignore it with a comment '# gazelle:ignore a' in the Python file.


  

🌍 Your Environment

Operating System:

  
ubuntu 20.04
  

Output of bazel version:

  
Bazelisk version: v1.16.0
Build label: 6.2.1
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Fri Jun 2 16:59:58 2023 (1685725198)
Build timestamp: 1685725198
Build timestamp as int: 1685725198
  

Rules_python version:

  
0.25
  

Anything else relevant? rules_python_gazelle_plugin version 0.28

cyrilhuang avatar Jan 18 '24 22:01 cyrilhuang

Right now you need to use the gazelle directive to tell gazelle how to resolve a particular proto import, it does not happen automatically.

aignas avatar Jan 18 '24 22:01 aignas

Just spitballing here, but would a reasonable solution be to create a script that queries all the protobuf targets from the workspace, associates them with their import path, and then updates a notated section of the top-level BUILD to define these gazelle directives?

Side note: It'd be nice if there was a mechanism to define gazelle directives outside of comments, eg) like stackb/rules_proto allows specification of a different configuration file: https://github.com/stackb/rules_proto?tab=readme-ov-file#yaml-configuration. Just checking that that's not a pattern that's more widely used around the gazelle infrastructure?

I'm curious what y'all think, and hopeful that my questions aren't too newbish, I'm just starting to familiarize myself with gazelle.

michael-christen avatar Jan 22 '24 02:01 michael-christen

If I were starting to look into this I would first

  • look at what bazel-gazelle does for go proto library management (if I remember correctly there is an if statement somewhere that handles creation of proto targets).
  • copy that to rules python gazelle with modifications to use py_proto_library.

The second bullet point is where the difficulty lies and I don't know the details of how one would achieve that.

aignas avatar Jan 22 '24 22:01 aignas

@michael-christen That's the only way I can think of to default enable py_proto_library auto resolution. But for a monorepo with abundant proto definitions, lots of directives will be in the top-level BUILD file.

As @aignas noted, gazelle supports go_proto_library resolution natively, users do not have to manually do query and update of proto denition. I think they achivev this through import path, but not sure about that. Hopefully this part of logic is integrated in python gazelle.

cyrilhuang avatar Jan 23 '24 03:01 cyrilhuang

Thanks y'all, if I find some time I'll look around there.

michael-christen avatar Jan 23 '24 07:01 michael-christen

We use stackb/rules_proto to generate our proto_py_library rules.

This patch https://github.com/vihangm/rules_python/commit/96c9aa02e7ceba9c8ef6bb780ab7d48f24aa372e maps the import to what's indexed by rules_proto and works (however I don't know if this is a great solution since it's leaking rules_proto implementation details).

vihangm avatar Feb 14 '24 22:02 vihangm

@aignas, just to clarify, does #1933 fix this or bypass how we'd handle this to the protobuf library?

michael-christen avatar Jun 05 '24 13:06 michael-christen

I was closing all proto related issues with the move of the py_proto_library out from the rules_python. Might have include an issue too many...

So far no automatic discovery of proto rules is done.

aignas avatar Jun 06 '24 07:06 aignas