search

bazelbuild / bazel

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

Consider skipping `bazel_tools@_` from lockfile #19971

Open layus opened 1 year ago

layus commented 1 year ago

Description of the bug:

MODULE.bazel.lock files contain entries for bazel_tools builtin module, as well as all its dependencies. Because that module is builtin, its version changes with bazel releases, meaning that the lockfile gets invalidated when built with a different bazel version. Meaning that the bazel version is an integral part of the build dependencies.

It makes sense to keep bazel_tools and its dependencies in the lockfile if they take part of the modules version resolution. But maybe it would be better to keep them out of the modules version selection.

Overall, the question is maybe about compatibility between bazel versions. Are projects supposed to build only with a single bazel version as .bazelversion suggests ? In my context, being able to build a bazel project with different bazel versions makes things simpler.

Is there an official policy about bazel versions compatibility ?

Which category does this issue belong to?

Core, Documentation

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

No response

Which operating system are you running Bazel on?

NixOS

What is the output of bazel info release?

7.0.0rc2

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

https://github.com/NixOS/nixpkgs/pull/262152

What's the output of git remote get-url origin; git rev-parse master; 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 11 months ago

@Pavank1992 Could you reassign to team External-Deps?

meteorcloudy commented 11 months ago

Meaning that the bazel version is an integral part of the build dependencies.

This is true and exactly the reason why those builtin dependencies have to be recorded in the lockfile to ensure correctness. If you change Bazel version without making sure those builtin dependencies are in sync, it may produce unexpected build failures.

But I don't think this will break compatibilities for building with different Bazel versions, it just means the lockfile could be updated if you change the bazel version (unless if you enable --lockfile_mode=error).

As the starlarkification of native build rules progresses, we should be able to reduce the dependencies required by the @bazel_tools builtin modules. And hopefully one day, we won't need the builtin module at all.

For keeping Bazel compatibilities:

meteorcloudy commented 11 months ago

Do you actually need to switch Bazel versions frequently? If so, we would like to understand why.

layus commented 11 months ago

When packaging bazel for nix, it is right now quite complex, and nix does not provide all the released bazel versions. At a given point in time (or rater, in git history), there is only one bazel package for each major release. More generally, nix has such a packaging model that most packages should be able to rebuild when their dependencies are updated. Bazel assumes the same for module versions.

In most cases, building with a different, or at least close bazel release works though. And that is what nixpkgs (the main source of package definitions for nix) does. Bazel dependency is pinned on the major version (like bazel_5).

A pain point in nix packaging of bazel-built applications is to provide external dependencies in a sandboxed build. This was done by running some python scripts in a separate package to extract all the http files that bazel needs, and providing a pre-populated outputBase. Now, with MODULE.bazel.lock, it is easy to load the JSON file and extract all the dependencies in nix langauge itself (Like when you can do something directly in starlark instead of having to use an external repo for it). Sadly, I have discovered that this method does not work when I build with a bazel version different from the one used to compute the lockfile. bazel_tools dependencies required are not the ones locked by the lockfile, and are thus missing.

My next idea is to have two sources of external packages, the ones derived from the lockfile, and the ones derived from the bazel install. By merging these sources in a --repository_cache structure, I get all the external dependencies required to build a given project with my any nix-provided bazel.

At this stage, I realized there was redundancy in the lockfile and .bazelversion. The lockfile does not contain the bazelversion, but the version impacts the content of the lockfile. The lockfile does not specify all the needed information for offline builds (because it does not say anything about the bazel version).

I have not implemented the double source of external dependencies described here, because I was not sure it would work. It relies on the property that dependency resolution between two sets of constraints (MODULE.bazel and bazel_tools from bazel binary) will always result in versions that come from either one or the other. I think it is true, but it needs to be extensively checked.

To be honnest, at that time I was struggling with the newly added id- files, and lost interest in packaging anything else than bazel_7.0.0 itself.

See also https://github.com/NixOS/nixpkgs/pull/262152 for the related work.

meteorcloudy commented 11 months ago

Oh, that is interesting. We kind of have to solve similar problems for running integration tests on Bazel CI. We want to make sure we can run Bazel integration tests without internet access, therefore we need to provide a pre-populated repository cache for the inner Bazel inside the test by building the //src:test_repos in advance in the build step.

You might also be interested in https://github.com/bazelbuild/bazel/pull/20042

Some links: