incompatible_remove_legacy_whole_archive: Remove --legacy_whole_archive while setting default to false

[hlopko]

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.

[hlopko]

CC @trybka

[dkelmer]

@hlopko I added the breaking-change-0.26 label since no one seems to have spoken up about increasing the window. TBH, I'm not sure what the significance of the label is if you don't actually flip the flag, but letting you know jic :)

[hlopko]

@dkelmer Thank you! Because the flag is only available in 0.25, people actually didn't have a chance to try it out yet. I'm not in hurry to flip this flag, so I added migration-0.26 and breaking-change-0.27.

[dkelmer]

Sounds good :)

[pmuetschard]

I think this breaks Android. On Android, to build an .so, one has to create a cc_library rule and depend on it from the android_binary target. This will cause bazel to create an .a for the cc_library and then attempt to create the .so from it. Without wrapping the .a in --whole-archive, well, no symbols make it into the .so.

Example commands I'm seeing:

clang ... -fPIC -c foo.cpp -o foo.o
ar rcsD ...libfoo.a ...foo.o
clang -shared -o ..._nativedeps/...so ...libfoo.a

Adding features = ["legacy_whole_archive"] to the cc_library or the android_binary rule had no effect.

[hlopko]

Did you try adding alwayslink = 1 to cc libraries symbols of which you need in the final .so?

[pmuetschard]

Adding alwayslink = 1 did indeed bring back the --whole-archive flag for the Android libs.

[laurentlb]

This flag was not flipped in 0.27, targeting 1.0?

[hlopko]

Yes, this didn't make it into 0.27, we're postponing the flip until 1.0. Could you create the labels so I can update our flags?

[ghost]

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?

[hlopko]

Hey @johnmarkwayve, What you could do is to introduce a cc_library wrapper that you use throughout your project. In the wrapper you'll set alwayslink attribute to a select (https://docs.bazel.build/versions/master/configurable-attributes.html). The default condition will be 0, the condition with a build_setting or --define will be 1. Whenever you'll be building the .so for distribution, you will pass that build_setting or --define.

This will roughly emulate the behavior of --legacy_whole_archive. Would that work for you?

[hlopko]

Oh I was just told that alwayslink attribute is not selectable. That complicates things. then your cc_library wrapper will have to create 2 cc_libraries, one with alwayslink set, one with it unset, and an alias rule with the select in its 'actual' attribute. Select will choose one of cc_libraries.

[ghost]

@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)?

[trybka]

@johnmarkwayve yeah, this is actually just a deficiency of how cc_library represents dependencies.

alwayslink serves other purposes (e.g. if you have module initializers that register something and no actual ODR-use of any symbol in the library to tell the linker to keep this library).

However --legacy_whole_archive was a different kind of hammer that forced everything to be linked into a shared library .so.

In actuality, alwayslink is more of a property of a dependency edge rather than a dependency itself.

One could imagine having an alwayslink_deps attribute that could accomplish this.

In lieu of such a thing, alwayslink with some alias()/select() magic should work.
Note, you only need alwayslink on libraries which define entry-point symbols. The other dependencies should be kept by the linker properly.

The alternatives are less palatable (IMO), but:
You can pass --undefined SYMBOL for each symbol in linkopts, and that will cause the linker to keep those symbols as well.
You can write out an assembly file that refers to the symbols in .rodata somehow, but that's likely even less maintainable.

[jwnimmer-tri]

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 RobotLocomotion/drake#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.

[ghost]

To add a question for discussion:

What exactly is "cc_library" supposed to encapsulate or represent?

1. An actual collection of built artefacts for the given platform?
2. The packaging of source code and dependencies?
3. The provision of some high-level functionality?
4. Something else?

I would argue that having always_link being an upstream parameter (as it is at the moment), forces you into something like (1). While with (2) or (3) point towards at least having the option of specifying dependencies as whole-archive (it's the downstream target that gets to decide how it uses its dependencies).

I agree with the original point of this issue, that whole-archive shouldn't be default, and I'm not against having always_link upstream as a hint ("it doesn't make much sense to use this library without linking the lot"). But as @trybka says, really it's a feature of the dependency edge, which is defined by the downstream target, not the upstream cc_library.

[hlopko]

One random idea that I didn't pursue is that maybe we can produce a linker script (version script) that will export all symbols present? I haven't written a linker script but it sounds like it might be possible? Passing the linker script to the cc_binary producing shared library is simple.

[jwnimmer-tri]

Update: in RobotLocomotion/drake#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.