jvolkman / rules_pycross

Bazel + Python rules for cross-platform external dependencies
Apache License 2.0
58 stars 21 forks source link

Compatibility with `rules_python_gazelle_plugin` #100

Open wingsofovnia opened 3 months ago

wingsofovnia commented 3 months ago

I am currently replacing rules_python in a rules_python + rules_python_gazelle_plugin + rules_py setup with rules_pycross and found some challenges in making rules_python_gazelle_plugin work with rules_pycross.

With this improvement, pycross can be plugged in into Gazelle.

MODULE.bazel

lock_import = use_extension("@rules_pycross//pycross/extensions:lock_import.bzl", "lock_import")
lock_import.import_poetry(
    lock_file = "//:poetry.lock",
    project_file = "//:pyproject.toml",
    repo = "pip",
)

BUILD.bazel

load("@pip//:requirements.bzl", "all_whl_requirements")
load("@gazelle//:def.bzl", "DEFAULT_LANGUAGES", "gazelle", "gazelle_binary")
load("@pip//:requirements.bzl", "all_whl_requirements")
load("@rules_python_gazelle_plugin//manifest:defs.bzl", "gazelle_python_manifest")
load("@rules_python_gazelle_plugin//modules_mapping:def.bzl", "modules_mapping")

modules_mapping(
    name = "gazelle.metadata",
    tags = ["manual"],
    wheels = all_whl_requirements,
)

gazelle_python_manifest(
    name = "gazelle.mapping",
    modules_mapping = ":gazelle.metadata",
    pip_repository_name = "pip",
)

gazelle_binary(
    name = "gazelle_bin",
    languages = DEFAULT_LANGUAGES + [
        "@rules_python_gazelle_plugin//python",
    ],
)

gazelle(
    name = "gazelle.update",
    gazelle = ":gazelle_bin",
)

gazelle(
    name = "gazelle.check",
    args = ["-mode=diff"],
    gazelle = ":gazelle_bin",
)

The missing piece is that rules_pycross has whl targets structured differently than ones in rules_python, expected by Gazelle plugin.

deps = ["@pip//:flask"], # rules_pycross
deps = ["@pip//flask"],  # rules_python

There was an attempt to align pycross whl targets with ones in rules_python which was declined (for a good reason):

Essentially, we need to prefix with :. I was able to workaround this by simply putting a genrule in the middle doing exactly that:

load("@aspect_bazel_lib//lib:jq.bzl", "jq")

modules_mapping(
    name = "_gazelle.metadata",
    tags = ["manual"],
    wheels = all_whl_requirements,
)

jq(
    name = "gazelle.metadata",
    srcs = [":_gazelle.metadata"],
    filter = 'with_entries(.value |= ":\\(.)")',
)

gazelle_python_manifest(
    name = "gazelle.mapping",
    modules_mapping = ":gazelle.metadata",
    pip_repository_name = "pip",
    tags = ["manual"],
)

This might be a very optimistic look at that and only work with a small set of dependencies I tested in a PoC project. See the working example: https://github.com/wingsofovnia/bazel-pycross-poetry-gazelle-example

I decided to start this issue to outline issues, some workarounds and gather feedback on possible directions to improve the setup.

@ewianda for viz as you've been probably trying to achieve similar goal.

jvolkman commented 3 months ago

I provided some of my thoughts about the layout here. The other issue you'll run into is dashes vs. underscores. pycross uses the Python name normalization, whereas rules_python does its own stuff on top.

A few options:

  1. Update modules_mapping to somehow support multiple layout types
  2. Have pycross generate some sort of compatibility repo that has aliases to the real targets where gazelle expects.
  3. Add a build rule that does what your jq target does in the example above, but with all required steps.

I think 1 is the best option, but I don't know anything about its feasibility. 2 may be useful for other aspects of peoples' migrations. 3 I think is the most straightforward and self-contained option, and I'd probably start there.

wingsofovnia commented 3 months ago

I extended jq target to fully normalise names:

jq(
    name = "gazelle.metadata",
    srcs = [":__gazelle.metadata"],
    filter = 'with_entries(.value |= ":\\(. | gsub(\"[-_.]+\"; \"-\") | ascii_downcase)")',
)

and gazelle_python.yaml looked alright but it appears that Gazelle plugin itself does additional replacement and although gazelle_python.yaml is now valid, e.g.

manifest:
  modules_mapping:
    dateutil.relativedelta: :python-dateutil
  pip_repository:
    name: pip

but

> bazel run //:gazelle.check
--- app/BUILD.bazel 1970-01-01 00:00:00.000000001 +0000
+++ app/BUILD.bazel 1970-01-01 00:00:00.000000001 +0000
@@ -6,6 +6,7 @@
     visibility = ["//:__subpackages__"],
     deps = [
         "@pip//:flask",
+        "@pip//:python_dateutil",
     ],
 )

Which is very sad :(

Update: found the place where this happens.

wingsofovnia commented 2 months ago

After a discussion in https://github.com/bazelbuild/rules_python/issues/1939, I contributed some directives that can change Gazelle plugin's label conventions and it works with rules_pycross now. It will land with https://github.com/bazelbuild/rules_python/pull/1976 PR if accepted.

Until then, Bazel 7.2.0+ users can use the plugin revision from PR's repo:

bazel_dep(name = "rules_python_gazelle_plugin", version = "0.0.0")

git_override(
  module_name = "rules_python_gazelle_plugin",
  commit = "1cc0168b57fb88faa97711081edb071c6bd23260",
  remote = "https://github.com/wingsofovnia/rules_python",
  strip_prefix = "gazelle", # New to Bazel 7.2.0+
)

and apply in your BUILD.bazel:

# gazelle:python_label_convention :$distribution_name$
# gazelle:python_label_normalization pep503

I've also updated the example mentioned in the post to use this plugin's revision if anyone is interested: https://github.com/wingsofovnia/bazel-pycross-poetry-gazelle-example (e832636). I would however recommend using pdm for new projects as its support seems to be more complete, see #34.

jvolkman commented 2 months ago

Thanks for working on that!

ewianda commented 2 months ago

Very nice

jvolkman commented 1 month ago

Hi @wingsofovnia, can this be closed now?

wingsofovnia commented 1 month ago

Yes, this has been resolved. Shall I update the README.md with a Gazelle how-to?

jvolkman commented 1 month ago

Yes, this has been resolved. Shall I update the README.md with a Gazelle how-to?

That would be great, thanks!