Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Targets in nested modules are visible from top module #21515

Open
cerisier opened this issue Feb 28, 2024 · 3 comments
Open

Targets in nested modules are visible from top module #21515

cerisier opened this issue Feb 28, 2024 · 3 comments
Assignees
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug

Comments

@cerisier
Copy link

cerisier commented Feb 28, 2024

Description of the bug:

Filing this as it seems unexpected but may not be a "bug".

It seems that targets declared in packages of nested modules are visible to the top module.
This seems unexpected as those nested targets may have dependencies that are only declared within the nested module and therefore not accessible from the top one.

The fact that those targets may be visible but then not buildable seems unexpected.

For instance, let's take an example of a module test with a nested module lib_a:

$ find .
./MODULE.bazel
./WORKSPACE
./README.md
./lib_a
./lib_a/MODULE.bazel
./lib_a/WORKSPACE
./lib_a/java/src/com/test/Main.java
$ cat ./lib_a/MODULE.bazel
module(name = "lib_a", version = "0.0.1")

bazel_dep(name = "rules_jvm_external", version = "6.0")

maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven")

maven.artifact(
    artifact = "guava",32323
    group = "com.google.guava",
    version = "31.1-jre",
    repositories = [
        "https://repo1.maven.org/maven2",
    ],
)

use_repo(maven, "maven")
cat ./lib_a/BUILD.bazel
load("@rules_jvm_external//:defs.bzl", "artifact")

java_binary(
    name = "lib_a",
    srcs = ["java/src/com/test/Main.java"],
    deps = [
        artifact("com.google.guava:guava"),
    ],
    main_class = "com.test.Main"
)

Running bazel query //... from the top module lists targets declared in the lib_a module:

$ bazel query //...
//lib_a:lib_a

But then running bazel build //lib_a:lib_a fails because of non visible dependencies from the top module.

$ bazel build //lib_a:lib_a
ERROR: no such package '@@[unknown repo 'maven' requested from @@]//': The repository '@@[unknown repo 'maven' requested from @@]' could not be resolved: No repository visible as '@maven' from main repository

This makes "sense" if Bazel treats those subdirectories as packages from the top module and not taking into account the repo marker files, but I find it unexpected that bzlmod makes it possible to bazel_dep that nested lib_a module but consider its nested targets as part of its own.

Note that obviously running bazel build @lib_a//:lib_a works fine.

However, my expectations would have been that once a module is declared as a bazel_dep, its targets may not be visible to the main module.

To circumvent this I can just add lib_a to .bazelignore tho but it doesn't seem ideal.

Maybe I am missing something ?
I'd also be very glad to learn more about practices about nested modules if it is one at all.

Which category does this issue belong to?

Core

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I made a github repo with a simple example:

https://github.com/cerisier/bazel-nested-module-build-error-repro

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 7.0.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@github-actions github-actions bot added the team-Core Skyframe, bazel query, BEP, options parsing, bazelrc label Feb 28, 2024
@cerisier cerisier changed the title Targets in nested repositories are visible but not buildable Targets in nested modules are visible from top module Feb 28, 2024
@fmeum
Copy link
Collaborator

fmeum commented Feb 28, 2024

Maybe we could make it so that wildcard patterns stop matching when they descend into a directory with a repo boundary marker?

Outright marking all packages below such directories as deleted isn't feasible as this would require traversing up to the root from all packages.

@Wyverald What do you think?

@meisterT meisterT added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. and removed team-Core Skyframe, bazel query, BEP, options parsing, bazelrc labels Feb 29, 2024
@meteorcloudy meteorcloudy added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Mar 5, 2024
@Wyverald Wyverald added the area-Bzlmod Bzlmod-specific PRs, issues, and feature requests label Jun 17, 2024
@Wyverald
Copy link
Member

Maybe we could make it so that wildcard patterns stop matching when they descend into a directory with a repo boundary marker?

See related update in #22208 (comment). Making wildcard patterns stop descending past repo boundary files could be a good alternative -- we just need to be careful with corner cases (what does //lib_a:... include? what does @@lib_a//... include?) and consistently implementing this across all the various impls of target pattern resolution (it feels like there are 3 impls in QueryCommand alone). That means @@//lib_a is still a valid package, although targets in there wouldn't be returned by //....

On the other hand, this is technically a breaking change. I wonder if this would break any realistic use cases? I remember @alexeagle had some comments about this at some point.

Outright marking all packages below such directories as deleted isn't feasible as this would require traversing up to the root from all packages.

I think this already happens -- see PackageLookupFunction, which calls into LocalRepositoryLookupFunction, which always goes all the way up to the root.

cc @katre again...

Wyverald added a commit that referenced this issue Jun 18, 2024
…le_workspace`

This still leaves the question of "what do we do instead?". See issues #22208 and #21515.

Fixes #22754.
@katre
Copy link
Member

katre commented Jun 18, 2024

See #22208 (comment): you are correct that LRLF recurses up to the root of the main repository. As I said in the other comment, the check is actually not clever, and only looks for marker files: it doesn't check whether that directory is actually used as a repo.

copybara-service bot pushed a commit that referenced this issue Jun 20, 2024
…le_workspace`

This still leaves the question of "what do we do instead?". See issues #22208 and #21515.

Fixes #22754.

Closes #22774.

PiperOrigin-RevId: 645148811
Change-Id: Ib9d07d2ecbc3a79e3341de6739de1c3349124d6b
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Jun 20, 2024
…le_workspace`

This still leaves the question of "what do we do instead?". See issues bazelbuild#22208 and bazelbuild#21515.

Fixes bazelbuild#22754.

Closes bazelbuild#22774.

PiperOrigin-RevId: 645148811
Change-Id: Ib9d07d2ecbc3a79e3341de6739de1c3349124d6b
github-merge-queue bot pushed a commit that referenced this issue Jun 20, 2024
…--noenable_workspace` (#22837)

This still leaves the question of "what do we do instead?". See issues
#22208 and #21515.

Fixes #22754.

Closes #22774.

PiperOrigin-RevId: 645148811
Change-Id: Ib9d07d2ecbc3a79e3341de6739de1c3349124d6b

Commit
1246ff4

Co-authored-by: Xdng Yng <wyverald@gmail.com>
iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Jun 21, 2024
…le_workspace`

This still leaves the question of "what do we do instead?". See issues bazelbuild#22208 and bazelbuild#21515.

Fixes bazelbuild#22754.

Closes bazelbuild#22774.

PiperOrigin-RevId: 645148811
Change-Id: Ib9d07d2ecbc3a79e3341de6739de1c3349124d6b
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Jun 27, 2024
…le_workspace`

This still leaves the question of "what do we do instead?". See issues bazelbuild#22208 and bazelbuild#21515.

Fixes bazelbuild#22754.

Closes bazelbuild#22774.

PiperOrigin-RevId: 645148811
Change-Id: Ib9d07d2ecbc3a79e3341de6739de1c3349124d6b
github-merge-queue bot pushed a commit that referenced this issue Jun 28, 2024
…--noenable_workspace` (#22911)

This still leaves the question of "what do we do instead?". See issues
#22208 and #21515.

Fixes #22754.

Closes #22774.

PiperOrigin-RevId: 645148811
Change-Id: Ib9d07d2ecbc3a79e3341de6739de1c3349124d6b

Commit
1246ff4

Co-authored-by: Xdng Yng <wyverald@gmail.com>
fmeum pushed a commit to fmeum/bazel that referenced this issue Jul 18, 2024
…--noenable_workspace` (bazelbuild#22837)

This still leaves the question of "what do we do instead?". See issues
bazelbuild#22208 and bazelbuild#21515.

Fixes bazelbuild#22754.

Closes bazelbuild#22774.

PiperOrigin-RevId: 645148811
Change-Id: Ib9d07d2ecbc3a79e3341de6739de1c3349124d6b

Commit
bazelbuild@1246ff4

Co-authored-by: Xdng Yng <wyverald@gmail.com>
mbland added a commit to EngFlow/example that referenced this issue Aug 8, 2024
Requested by @laszlocsomor during review of the upcoming EngFlow blog
post: "Migrating to Bazel Modules (a.k.a. Bzlmod) - Repo Names and
Runfiles".

The .bazelignore file is required for now to prevent `bazel build //...`
from recursing into the modules under `runfiles/` pending resolution of:

- bazelbuild/bazel#22208
- bazelbuild/bazel#21515
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Projects
None yet
Development

No branches or pull requests

8 participants