bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.13k stars 4.05k forks source link

Targets in nested modules are visible from top module #21515

Open cerisier opened 7 months ago

cerisier commented 7 months ago

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

fmeum commented 7 months ago

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?

Wyverald commented 4 months ago

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 https://github.com/bazelbuild/bazel/issues/22208#issuecomment-2174586425. 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...

katre commented 4 months ago

See https://github.com/bazelbuild/bazel/issues/22208#issuecomment-2176012937: 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.