bazel: incompatible_remove_legacy_whole_archive: Remove --legacy_whole_archive while setting default to false

Flag: --incompatible_remove_legacy_whole_archive Available since: 0.25 Will be flipped: 0.26, unless we hear from Bazel users it’s not enough. Bazel 1.0 is the latest flipping possibility, but we will gladly postpone the flip until then, if Bazel users need more time. Please speak up.

Motivation

The docs of this flag say:

–[no]legacy_whole_archive default: “true” When on, use --whole-archive for cc_binary rules that have linkshared=1 and either linkstatic=1 or ‘-static’ in linkopts. This is for backwards compatibility only. A better alternative is to use alwayslink=1 where required.

What this means is that all library archives (and library object groups, as defined by --start-lib/–end-lib) are added to the link command wrapped in -whole-archive, -no-whole-archive, which instructs the linker to always include the entire library. Even with dead code elimination, this can be pernicious if the library defines some global initializer, as that will cause all of the code dependencies of those initializers to be kept.

History

--(legacy_)whole-archive was introduced on 2004-04-19, with the commit description saying:

I think --whole-archive was just the magical incantation that got swigdeps.so working.
… You’re right though that the real question is what would break if we removed this, and I don’t really have a good idea.

Problem

“Legacy” whole-archive behavior is a poor default, but it is difficult to change.

--[no]legacy_whole_archive is a whole build option. It is impossible to “tag” individual shared, statically linked, dynamic libraries which can be built without “whole-archive” semantics.

Care must be taken to flip the default–libraries which export symbols must be tagged as alwayslink=1 unless some other mechanism exists in the dylib to preserve the symbols which are being exported.

It is a difficult thing to test: unexported symbols are only noticed at runtime, which means that 100% test coverage would have to exist for a depot-wide mass cleanup.

Migration

--legacy_whole_archive will be superseded by --incompatible_remove_legacy_whole_archive. --incompatible_remove_legacy_whole_archive, when true, removes both the old flag, and the behavior enabled by features (explained below).

Bazel now has a mechanism to configure individual cc_binary targets which can be built without --legacy_whole_archive. This mechanism still honors the --incompatible_remove_legacy_whole_archive. This mechanism is based on feature named “legacy_whole_archive”. When added to the features attribute of cc_binary, Bazel will keep the legacy whole archive behavior, but only for the cc_binary, not for the whole build.

This is the migration plan we will follow at Google:

  1. Add --features=legacy_whole_archive to the global .bazelrc.
  2. Users can start adding features = "-legacy_whole_archive" (note, minus here) to targets that are safe to build without the legacy behavior.
  3. Add a presubmit check telling users they should add features = [ "-legacy_whole_archive" ].
  4. Have a presubmit check comparing the binary size with and without the feature, to motivate users to migrate.
  5. Once everything is migrated, remove legacy_whole_archive from global .bazelrc.
  6. Celebrate
  7. Remove -legacy_whole_archive features from targets.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 20 (17 by maintainers)

Commits related to this issue

Most upvoted comments

Update: in https://github.com/RobotLocomotion/drake/pull/12262, I’ve written a cc_whole_archive_library(deps = ...) which is basically cc_library(alwayslink_deps = ...). It just rewrites libraries_to_link to add alwayslink = True to every edge, instead of forcing it to appear upstream. Anyone is welcome to reuse it (Apache-2.0) for yourselves if it helps.

@hlopko thanks for the advice. It’s a lot uglier than I’d like but it should work.

I realise my use case isn’t super common, but it doesn’t seem unreasonable either (“this .so target exists to be shipped standalone without dependencies, but the rest of the codebase wants to behave normally”). It seems odd to me that alwayslink has been put onto the depended-on targets rather than the final target? Why should my cc_library care whether you decide to link in the whole archive or not (meanwhile, the downstream target might be quite opinionated about how linking happens)?

I have the exact problem as @johnmarkwayve.

I have hundreds of small targets cc_library(linkstatic = 1) that are used both (1) by C++ applications with a main() routine (where we definitely do not want alwayslink = True for bloat reasons), and (2) by Python as a loaded module (as a single, large shared library composed of the small cc_library targets rolled into one).

Because the alwayslink is at the point of declaration instead of point of use (aka not on the edges), I can’t tag my small cc_library targets with it. But there seems to be no built-in way to roll up cc_library targets into a whole-archive shared library, which for my use case is a major regression and loss of capability.

It seems like at minimum an attribute like the alwayslink_deps proposed above should exist, so that I can write …

cc_library(
  name = "all.a",
  alwayslink = True,
  alwayslink_deps = [":small1", ":small2", ":small3"],
)

… at which point I could convert all.a into a cc_binary(all.so) shared library by writing one more simple rule.

The work-around of using a macro to declare two cc_library targets (one with and without alwayslink) is a non-starter. For one thing, it compiles all of the sources twice (even if I mark them as “manual” and only induce builds on the flavors that are actually used). For another, it gets us into a “red object” vs “blue object” world, where the deps lines need to be respelled inside the macro to have the correct color depending on whether the ultimate cc_binary is a program or a shared library.

For now my workaround is to implement a cc_whole_archive_library rule (see https://github.com/RobotLocomotion/drake/pull/12262) that repacks all objects into a single static archive, which I can then label with alwayslink = True. This is a bit gross from a scale perspective (build footprint and I/O) but appears to get the job done.

But in general I’d like if cc_library and/or cc_binary had the capability to deliver shared objects out of the box, instead of having to roll my own starlark for it.

I know I’m very late to the game here, but what’s the suggested mitigation? I had been using cc_binary with linkshared and linkstatic in a few places to force a build of a .so which contains all symbols of all dependencies so it could be shipped on its own. I only care about linking in these symbols in the few cases where I want to ship a dependency-free .so, so I definitely don’t want to pollute all of my other cc_library rules with alwayslink = True just to support this fairly rare use case?