Skip to content

feat: stop generating imports when not necessary#1335

Merged
f0rmiga merged 2 commits intobazel-contrib:mainfrom
linzhp:imports
Jul 28, 2023
Merged

feat: stop generating imports when not necessary#1335
f0rmiga merged 2 commits intobazel-contrib:mainfrom
linzhp:imports

Conversation

@linzhp
Copy link
Copy Markdown
Contributor

@linzhp linzhp commented Jul 21, 2023

When gazelle:python_root is not set or is at the root of the repo, we don't need to set imports for python rules, because that's the Bazel's default. This would reduce unnecessary verbosity.

@linzhp linzhp requested a review from f0rmiga as a code owner July 21, 2023 23:03
@f0rmiga
Copy link
Copy Markdown
Member

f0rmiga commented Jul 21, 2023

I don't think the logic is correct, based on the changed tests. Could you elaborate, please? I may be missing something.

@linzhp
Copy link
Copy Markdown
Contributor Author

linzhp commented Jul 22, 2023

Consider this workspace, where imports are not set in any of the py rules:

-- MODULE.bazel --
module(name = "py_imports")

rules_python_version = "0.24.0"

bazel_dep(name = "rules_python", version = rules_python_version)
bazel_dep(name = "rules_python_gazelle_plugin", version = rules_python_version)
bazel_dep(name = "gazelle", version = "0.31.0", repo_name = "bazel_gazelle")

python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(
    configure_coverage_tool = True,
    is_default = True,
    python_version = "3.9",
)
-- WORKSPACE --
-- pkg1/BUILD.bazel --
load("@rules_python//python:defs.bzl", "py_binary")

py_binary(
    name = "pkg1",
    srcs = ["__init__.py", "main.py"],
    main = "main.py",
    deps = ["//pkg2"],
)
-- pkg1/__init__.py --
-- pkg1/main.py --
from pkg2 import lib

print(lib.hello())
-- pkg2/BUILD.bazel --
load("@rules_python//python:defs.bzl", "py_library")

py_library(
    name = "pkg2",
    srcs = ["__init__.py", "lib.py"],
    visibility = ["//visibility:public"],
)
-- pkg2/__init__.py --
-- pkg2/lib.py --
def hello():
    return "hello world"

It's perfectly fine to run:

bazel run --enable_bzlmod //pkg1
INFO: Analyzed target //pkg1:pkg1 (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //pkg1:pkg1 up-to-date:
  bazel-bin/pkg1/pkg1
INFO: Elapsed time: 0.125s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/pkg1/pkg1
hello world

This is because Bazel use the root of the workspace as PYTHONPATH by default, so the module pkg2/lib.py is imported as from pkg2 import lib. From Gazelle's perspective, this means when # gazelle:python_root is not set or is set at the root of the repo, we don't need to assign any value to imports attribute.

The tests are updated because # gazelle:python_root is not set in those tests, and no import attribute is populated for the py rules in those tests, reflecting the new behavior.

Note that the new behavior only affects new targets generated by Gazelle. Existing imports attribute will be preserved, whether they are written by hand or populated by Gazelle in the past.

Does this make sense?

Copy link
Copy Markdown
Member

@f0rmiga f0rmiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense! Thanks!

@f0rmiga f0rmiga added this pull request to the merge queue Jul 28, 2023
Merged via the queue into bazel-contrib:main with commit bb8c485 Jul 28, 2023
@linzhp linzhp deleted the imports branch July 28, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants