bazelbuild / bazel

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

Bzlmod: MODULE.bazel API to import dependencies from URL or local file #21269

Closed aran closed 6 months ago

aran commented 7 months ago

Description of the bug:

archive_override(module_name="directly_imported", ...) should import all dependencies of "directly_imported" completely hiding the dependency structure of the internal implementation. Currently it can be necessary to open up "directly_imported", look at its MODULE.bazel, and directly import those transitive dependencies as well, breaking modularity.

Which category does this issue belong to?

No response

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

# MODULE.bazel
BAZEL_VERSION = "7.0.2"
BAZEL_INTEGRITY = "sha256-bqk/ux+PEkd2pcl97tKL0SBjeJGeocawW3SJEIFPHic="
BAZEL_URL = "https://github.com/bazelbuild/bazel/archive/refs/tags/{v}.tar.gz".format(v = BAZEL_VERSION)
bazel_dep(name = "bazel", version = BAZEL_VERSION)

archive_override(
    module_name = "bazel",
    integrity = BAZEL_INTEGRITY,
    urls = [BAZEL_URL],
    strip_prefix = "bazel-{v}".format(v = BAZEL_VERSION),
)
$ bazel query '@bazel//src/main/protobuf:*'
ERROR: Error computing the main repository mapping: in module dependency chain <root> -> bazel@_ -> googleapis@_: bad bazel_dep on module 'googleapis' with no version. Did you forget to specify a version, or a non-registry override?
    Fetching https://bcr.bazel.build/modules/protobuf/21.7/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/rules_jvm_external/5.2/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/rules_cc/metadata.json
    Fetching https://bcr.bazel.build/modules/grpc/metadata.json
    Fetching https://bcr.bazel.build/modules/rules_testing/0.0.4/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/bazel_skylib/metadata.json
    Fetching https://bcr.bazel.build/modules/blake3/1.3.3.bcr.1/MODULE.bazel
# MODULE.bazel
BAZEL_VERSION = "7.0.2"
BAZEL_INTEGRITY = "sha256-bqk/ux+PEkd2pcl97tKL0SBjeJGeocawW3SJEIFPHic="
BAZEL_URL = "https://github.com/bazelbuild/bazel/archive/refs/tags/{v}.tar.gz".format(v = BAZEL_VERSION)
bazel_dep(name = "bazel", version = BAZEL_VERSION)

archive_override(
    module_name = "bazel",
    integrity = BAZEL_INTEGRITY,
    urls = [BAZEL_URL],
    strip_prefix = "bazel-{v}".format(v = BAZEL_VERSION),
)

archive_override(
    module_name = "googleapis",
    integrity = BAZEL_INTEGRITY,
    urls = [BAZEL_URL],
    strip_prefix = "bazel-{v}/third_party/googleapis".format(v = BAZEL_VERSION),
)

archive_override(
    module_name = "remoteapis",
    integrity = BAZEL_INTEGRITY,
    urls = [BAZEL_URL],
    strip_prefix = "bazel-{v}/third_party/remoteapis".format(v = BAZEL_VERSION),
)
$ bazel query '@bazel//src/main/protobuf:*'
@bazel//src/main/protobuf:BUILD
@bazel//src/main/protobuf:_command_server_cc_grpc_grpc_codegen
@bazel//src/main/protobuf:action_cache.proto
@bazel//src/main/protobuf:action_cache_java_proto
...

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 7.0.2

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 ?

N/A

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

N/A

Have you found anything relevant by searching the web?

There are very few discussions of archive_override on those sources or more broadly.

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

I'm not a bazel expert, so maybe my mental model is wrong. My naive guess is something this:

fmeum commented 6 months ago

archive_override isn't at fault here: Bazel uses overrides itself, but overrides are (and can) only be applied to the root module. The project just isn't set up to be a bazel_dep, but it's also not meant to be one.

aran commented 6 months ago

Thanks for the clarification, it sounds like by designarchive_override doesn't compose. You could imagine something like bazel_archive_dep and bazel_local_dep that behaves the way I mistakenly expected archive_override to behave—A variant ofarchive_override but composable/modular. In that world, just for illustrative example, Bazel's MODULE.bazel would conceivably have

bazel_local_dep( # instead of local_path_override
    module_name = "remoteapis",
    path = "./third_party/remoteapis",
)

bazel_local_dep(
    module_name = "googleapis",
    path = "./third_party/googleapis",
)

and my example MODULE.bazel snippet would be:

bazel_archive_dep(
    module_name = "bazel",
    integrity = BAZEL_INTEGRITY,
    urls = [BAZEL_URL],
    strip_prefix = "bazel-{v}".format(v = BAZEL_VERSION),
)
aran commented 6 months ago

Updated the title to reflect that this is probably best interpreted as a feature suggestion that would behave the way I wrongly expected archive_override to behave, to let modules be provided from places other than published registries. No idea if this is something that the team would consider, I can close out if not.

fmeum commented 6 months ago

The problem is that bazel_local_dep by design can't be composable: What if both Bazel and your own module specify different local or remote versions of the remoteapis module? There is no way to reconcile those declarations short of regular root-level only overrides - and then we are back to the original experience.

aran commented 6 months ago

Yeah, that's an interesting issue about the module namespace. With a global namespace that would have to be an error. I was implicitly imagining a scoped namespace, so if I specified a different remoteapis somewhere else it would be a distinct set of rules and targets.

As a result of this I just learned you can't actually have the same module name imported from more than one registry and there is a first-registry-wins semantics for resolving module names. The sort of assumptions I had about modules are more wrong than I thought and I think the above "expected result" is not even close to feasible.