fix: use correct runfiles path for venv import paths (#299)

In `_make_import_path`, `ctx.workspace_name` is always `_main` when
evaluating the deps of targets in the main repo. This includes deps that
are in external repos, e.g.

    importer = use_extension("//:import.bzl", "importer")
    use_repo(importer, "myrepo")

with

    py_test(..., deps = ["@myrepo//foo"])

In this case, we actually want to use the label's `workspace_name` attr
which will be the same as the runfiles path that we need for the venv
import paths.

Also added a new e2e test that creates this external repo and verifies
the fix. Without the fix in place, the test does fail as described in
the e2e/repository-rule-deps/README.md "Repro" section.

---

### Test plan
- New test case added in `e2e/repository-rule-deps`.
- Manually tested against bazel 6.5 and 7.0.2.

Fixes: #276

.github/workflows/ci.yaml

@@ -20,14 +20,15 @@ jobs:
   test:
     uses: bazel-contrib/.github/.github/workflows/bazel.yaml@v6
     with:
-      folders: '[".", "e2e/smoke"]'
+      folders: '[".", "e2e/smoke", "e2e/repository-rule-deps"]'
       # TODO: Build Windows tools and add to toolchain
       # TODO(alex): switch the root folder to bzlmod
       exclude: |
         [
           {"os": "windows-latest"},
           {"folder": ".", "bazelversion": "6.4.0"},
-          {"folder": ".", "bzlmodEnabled": true}
+          {"folder": ".", "bzlmodEnabled": true},
+          {"folder": "e2e/repository-rule-deps", "bzlmodEnabled": false}
         ]
 
   verify-bcr-patches:

e2e/repository-rule-deps/.bazelrc

@@ -0,0 +1,7 @@
+common --test_output=all
+
+# The test only works with bzlmod enabled.
+common --enable_bzlmod
+
+# The imports=[".."] only works properly when this matches the python toolchain major version.
+common --@aspect_rules_py//py:interpreter_version=3.11.6

e2e/repository-rule-deps/.bazelversion

@@ -0,0 +1 @@
+7.0.2

e2e/repository-rule-deps/BUILD.bazel

@@ -0,0 +1,34 @@
+load("@aspect_rules_py//py:defs.bzl", "py_test")
+
+all_modules = [
+    "directmod",
+    "flat",
+    "subdir",
+    "toplevel",
+]
+
+always_deps = [
+    "//direct/directmod",
+    "//toplevel",
+]
+
+py_test(
+    name = "test",
+    srcs = ["test.py"],
+    args = all_modules,
+    deps = always_deps + [
+        "@myrepo//:subdir",
+        "@myrepo//flat",
+    ],
+)
+
+py_test(
+    name = "all_direct",
+    srcs = ["test.py"],
+    args = all_modules,
+    main = "test.py",
+    deps = always_deps + [
+        "//imported:subdir",
+        "//imported/flat",
+    ],
+)

e2e/repository-rule-deps/MODULE.bazel

@@ -0,0 +1,37 @@
+"Bazel dependencies"
+
+bazel_dep(name = "aspect_rules_py", version = "0.0.0", dev_dependency = True)
+local_path_override(
+    module_name = "aspect_rules_py",
+    path = "../..",
+)
+
+# Because we use a prerelease of rules_py, we must compile the rust tools from source.
+bazel_dep(name = "rules_rust", version = "0.40.0")
+
+rust = use_extension("@rules_rust//rust:extensions.bzl", "rust")
+rust.toolchain(
+    edition = "2021",
+    versions = ["1.74.1"],
+)
+use_repo(rust, "rust_toolchains")
+
+register_toolchains("@rust_toolchains//:all")
+
+#---SNIP--- Below here is re-used in the snippet published on releases
+# Minimum version needs:
+# feat: add interpreter_version_info to py_runtime by @mattem in #1671
+bazel_dep(name = "rules_python", version = "0.29.0", dev_dependency = True)
+
+python = use_extension("@rules_python//python/extensions:python.bzl", "python")
+python.toolchain(
+    configure_coverage_tool = True,
+    python_version = "3.11.6",
+)
+
+# test case for py_library in an imported / generated repository
+bazel_dep(name = "bazel_skylib", version = "1.5.0")
+
+importer = use_extension("//rules:import.bzl", "importer")
+use_repo(importer, "myrepo")
+# end test case

e2e/repository-rule-deps/README.md

@@ -0,0 +1,38 @@
+# Test Case: py_library in generated / external repository
+
+See [#276](https://github.com/aspect-build/rules_py/issues/276).
+
+## Structure
+
+```mermaid
+graph TD;
+
+test(:test) --> directmod(direct:directmod)
+test --> subdir("@myrepo//:subdir")
+test --> flat("@myrepo//bar")
+    subgraph "@myrepo"
+    subdir
+    flat
+    end
+```
+
+## Subdirs
+
+- [./BUILD.bazel](./BUILD.bazel) contains the demonstration `py_test` that makes use of the modules.
+- [rules/](rules/) contains the "importer" rule used to create the external repository under test. It simply copies the files from the fixed path `./imported`.
+- [direct/](direct/) counterexample showing that a `py_library` from within the `_main` repository is working as expected.
+- [imported/](imported/) contains the contents of the external repository to be imported.
+- [toplevel/](toplevel/) contains a simple python package defined at the same level as the test (no `imports` required).
+
+## Repro
+
+The `:test` test case imports two modules from `@myrepo` ("flat" using `imports=["."]` and "subdir" using `imports=[".."]`), and two from `_main`. The packages from `_main` are correctly seen by python while the other two are not.
+
+The second test case `:all_direct` skips the `repository_rule` to demonstrate that the `test.py` itself is correctly implemented and can actually pass when the bug is not present.
+
+
+```console
+$ bazel test ...
+...
+AssertionError: import errors: ["No module named 'flat'", "No module named 'subdir'"]
+```

e2e/repository-rule-deps/direct/BUILD.bazel

@@ -0,0 +1 @@
+##

e2e/repository-rule-deps/direct/directmod/BUILD.bazel

@@ -0,0 +1,8 @@
+load("@aspect_rules_py//py:defs.bzl", "py_library")
+
+py_library(
+    name = "directmod",
+    srcs = ["__init__.py"],
+    imports = [".."],
+    visibility = ["//visibility:public"],
+)

e2e/repository-rule-deps/direct/directmod/__init__.py

@@ -0,0 +1 @@
+name = "directmod"

e2e/repository-rule-deps/imported/BUILD.bazel

@@ -0,0 +1,10 @@
+load("@aspect_rules_py//py:defs.bzl", "py_library")
+
+py_library(
+    name = "subdir",
+    srcs = [
+        "subdir/__init__.py",
+    ],
+    imports = ["."],
+    visibility = ["//visibility:public"],
+)

e2e/repository-rule-deps/imported/flat/BUILD.bazel

@@ -0,0 +1,10 @@
+load("@aspect_rules_py//py:defs.bzl", "py_library")
+
+py_library(
+    name = "flat",
+    srcs = [
+        "__init__.py",
+    ],
+    imports = [".."],
+    visibility = ["//visibility:public"],
+)

e2e/repository-rule-deps/imported/flat/__init__.py

@@ -0,0 +1 @@
+name = "flat"

e2e/repository-rule-deps/imported/subdir/__init__.py

@@ -0,0 +1 @@
+name = "subdir"

e2e/repository-rule-deps/rules/BUILD.bazel

@@ -0,0 +1,4 @@
+exports_files(
+    srcs = ["import.bzl"],
+    visibility = ["//visibility:public"],
+)

e2e/repository-rule-deps/rules/import.bzl

@@ -0,0 +1,30 @@
+load("@bazel_skylib//lib:paths.bzl", "paths")
+
+def _myrepo_impl(repository_ctx):
+    p = repository_ctx.workspace_root.get_child(repository_ctx.attr.path)
+    find_cmd = ["find", str(p), "-maxdepth", "1", "-mindepth", "1"]
+    r = repository_ctx.execute(find_cmd)
+    if r.return_code != 0:
+        fail(r.stdout + "\n" + r.stderr)
+
+    files = r.stdout.splitlines()
+    for f in files:
+        repository_ctx.symlink(f, paths.basename(f))
+
+    r = repository_ctx.execute(find_cmd)
+    if r.return_code != 0:
+        fail(r.stdout + "\n" + r.stderr)
+
+myrepo = repository_rule(
+    implementation = _myrepo_impl,
+    attrs = {
+        "path": attr.string(mandatory = True),
+    },
+)
+
+def _importer_impl(module_ctx):
+    myrepo(name = "myrepo", path = "imported")
+
+importer = module_extension(
+    implementation = _importer_impl,
+)

e2e/repository-rule-deps/test.py

@@ -0,0 +1,28 @@
+import importlib
+import sys
+
+
+def main():
+    print("sys.path entries:")
+    for p in sys.path:
+        print(p)
+    errors = []
+    mod_names = sys.argv[1:]
+    mods = {}
+    for name in mod_names:
+        try:
+            mods[name] = importlib.import_module(name)
+        except ImportError as e:
+            errors.append(e)
+
+    assert not errors, f"import errors: {[str(e) for e in errors]}"
+
+    for name in mod_names:
+        expected = name
+        actual = mods[name].name
+        print(f"{name}.name = ", mods[name].name)
+        assert expected == actual, f"expected: {expected!r}, actual: {actual!r}"
+
+
+if __name__ == "__main__":
+    main()

e2e/repository-rule-deps/toplevel/BUILD.bazel

@@ -0,0 +1,7 @@
+load("@aspect_rules_py//py:defs.bzl", "py_library")
+
+py_library(
+    name = "toplevel",
+    srcs = ["__init__.py"],
+    visibility = ["//visibility:public"],
+)

e2e/repository-rule-deps/toplevel/__init__.py

@@ -0,0 +1 @@
+name = "toplevel"

py/private/py_library.bzl

@@ -130,7 +130,7 @@ def _make_import_path(label, workspace, base, imp):
 def _make_imports_depset(ctx, imports = [], extra_imports_depsets = []):
     base = paths.dirname(ctx.build_file_path)
     import_paths = [
-        _make_import_path(ctx.label, ctx.workspace_name, base, im)
+        _make_import_path(ctx.label, ctx.label.workspace_name or ctx.workspace_name, base, im)
         for im in getattr(ctx.attr, "imports", imports)
     ] + [
         # Add the workspace name in the imports such that repo-relative imports work.