Support linking using cc_common.link

[cerisier]

Currently, tests, executables and shared libraries are linked using zig.

However, there are scenarios where it is useful to defer the link using the linker configured by rules_cc:

1. it's not possible to override the linker using zig build-* (-flld is lld only and is ignored on macos)
2. SPEED, on macos we go from 40s to 8s linking
3. control, not all options are supported and some are even swallowed (zig cc silently ignores -fuse-ld= option ziglang/zig#18357)

Proposed approach

Introduce a linkmode string_flag with 2 possible values ["zig" (default), "cc"] and use that in zig_build_impl to perform the linking accordingly.

When linkmode = "cc", we always produce a static library and use cc_common.link to perform the link, letting rules_cc deal with linker_inputs from transitive C deps.

When zig is used, nothing changes, zig is used to produce the final artifact and linker inputs of transitive C deps are collected and added to the compiler CLI arguments just like it is the case today.

[aherrmann]

+1 for introducing that feature.

---

When linkmode = "cc", we always produce a static library and use cc_common.link to perform the link, letting rules_cc deal with linker_inputs from transitive C deps.

So, Zig always generates a static library and cc_common.link then produces the final shared library (zig_shared_library) or executable (zig_binary, zig_test). Sounds good.

---

Using a string flag sounds plausible to me. E.g. one could globally configure the CC linked on MacOS for the speed but use the Zig linker on Linux for the cross-compilation support.

Just for reference:
Using a string flag means that users can change the behavior globally using --@rules_zig//whatever/the/flag/is:called, or can change it using the zig_configure* set of rules.
Using an attribute would mean that the behavior is defined in the BUILD files on the target directly. It can still be made platform specific using select, but it can't be changed after the fact using command line flags or transitions.

Features like the target platform or release vs. debug mode can be set via flag/transition because they depend on the external factors like whether I'm debugging or creating a release or which platform I'm working on.

[cerisier]

The way things work today, I didn't want to introduce an attribute for it because I feel like the decision should be downstream via the flag or via transitions.

[cerisier]

While implementating this, I stumbled upon something I thought I could escape.

cc_common.link main_output attribute is considered a private API. This is needed when when linking a zig_shared_library.

There is an exemption list fo this attribute in Bazel source code directly, which basically checks the apparent repo name requesting the api.

A technique used by other rulesets such as rules_rust, rules_python and more recently rules_mojo exists to expose the API through a custom repo.

bazelbuild/bazel#23838 (comment)

I will use the same technique to predict the output so that i can expose is as a DefaultInfo.

[aherrmann]

@cerisier thanks for the update. Could you expand a little on why this is necessary? Shouldn't the output artifact be accessible through the LibraryToLink or executable fields of the output?
Note, if the library symlinks are the issue, then take a look at https://bazel.build/rules/lib/builtins/LibraryToLink.html#resolved_symlink_dynamic_library. I added it for a rules haskell use case way back when.

[cerisier]

The main_output attribute is useful when you need a specific generated filename, usually through shared_lib_name attribute (just like cc_shared_library provides).

For instance, python extension .so files must follow a very specific naming.
In the case of ZML, the usecase is that we create a sandbox whose .so files must be named with a specific version (.so.1).

So basically, to support a shared_lib_name attribute, we need this hack like others do.
If not, users can still use a copy_file to replicate the behavior.

See also bazelbuild/bazel#23838

[cerisier]

Additionally, I would like to advocate for removing CcInfo from the providers of zig_shared_library to match the behavior of cc_shared_library.

Moreoever, was the prior behavior of forwarding the CcInfo from the cdeps via the providers of zig_shared_library a wanted behavior ? Unless mistaken, the C dependencies are already linked in to .so file and should not be forwarded ?

[aherrmann]

So basically, to support a shared_lib_name attribute, we need this hack like others do.
If not, users can still use a copy_file to replicate the behavior.

Right, makes sense. Thanks for explaining the details! Okay, let's go with this approach. Could you please document that it relies on this unstable API with a link to the upstream issue?

Additionally, I would like to advocate for removing CcInfo from the providers of zig_shared_library to match the behavior of cc_shared_library.

Yes, I think that falls into the scope of #285.

Moreoever, was the prior behavior of forwarding the CcInfo from the cdeps via the providers of zig_shared_library a wanted behavior ?

The history is that initially I only introduced zig_library for static libraries behaving similarly to cc_library. I.e. returning CcInfo and forwarding any transitive C library dependencies. zig_shared_library was added later on (#103) following the same pattern as zig_library. And zig_static_library was only very recently renamed from zig_library (#491).

Unless mistaken, the C dependencies are already linked in to .so file and should not be forwarded ?

This is not always the case. E.g. this test case confirms that transitive symbols are not linked into static libraries.

Shared libraries can dynamically load other shared libraries, which need to be forwarded to any dependents.

Coming back to #285. I think the correct behavior for zig_static_library and zig_shared_library would be to follow the model of cc_static_library and cc_shared_library. These give users control over which libraries to bundle into the generated outputs, and which to depend on as separate libraries.

[cerisier]

Right, makes sense. Thanks for explaining the details! Okay, let's go with this approach. Could you please document that it relies on this unstable API with a link to the upstream issue?

Yes!

The history is that initially I only introduced zig_library for static libraries behaving similarly to cc_library. I.e. returning CcInfo and forwarding any transitive C library dependencies. zig_shared_library was added later on (#103) following the same pattern as zig_library. And zig_static_library was only very recently renamed from zig_library (#491).

Unless mistaken, the C dependencies are already linked in to .so file and should not be forwarded ?

This is not always the case. E.g. this test case confirms that transitive symbols are not linked into static libraries.

Shared libraries can dynamically load other shared libraries, which need to be forwarded to any dependents.

Coming back to #285. I think the correct behavior for zig_static_library and zig_shared_library would be to follow the model of cc_static_library and cc_shared_library. These give users control over which libraries to bundle into the generated outputs, and which to depend on as separate libraries.

I understand better now. However, isn't it a bug to provides transitive C dependencies that have already been linked into the shared library for the case of zig_shared_library ?

[aherrmann]

I understand better now. However, isn't it a bug to provides transitive C dependencies that have already been linked into the shared library for the case of zig_shared_library ?

On the CC rules side, cc_library targets do not link in symbols from their library dependencies. For the static library path they just collect transitive static library dependencies and only the final static binary will link all required symbols in. For the dynamic library path they will also just collect transitive dynamic library dependencies and only the final dynamic binary (typically a test) will link them all in dynamically. Therefore it is correct to forward all transitive libraries. To my knowledge this behavior stems from Google internal practices.

The newer cc_static|shared_library rules are different, they let users control which libraries should be bundled into the output of the current target and which should be forwarded as transitive library dependencies. To that end they also return different providers than CcInfo, see cc_shared_library and cc_static_library.

As mentioned above zig_library was initially modeled after cc_library. Following #285, as zig_static|shared_library align with their cc_static|shared_library counterparts, the way in which transitive dependencies are managed and the shape of the providers that are returned also need to be adjusted.

[cerisier]

I understand.

This is not always the case. E.g. this test case confirms that transitive symbols are not linked into static libraries.

However, I think there is a bug in the implementation that makes this test pass.
Currently, transitive C libraries are added to the zig build command as archives (.a), which are wrongly treated by zig build and added as members of the output archive, resulting in an archive of an archive.

When build //link-dependencies/static-library:library, and inspect the archive, we see:

% ar t bazel-out/aarch64-fastbuild/bin/link-dependencies/static-library/liblibrary.a
bazel-out/aarch64-fastbuild/bin/link-dependencies/static-library/libadd.a
/tmp/zig-cache/tmp/f464da6cf2cef346/liblibrary_zcu.o

which is a malformed archive.

So here, the code that currently handles cdeps for zig (even prior to #522), basically tries to bundle all C dependencies in the target output, but it doesn't happen because the archive is malformed.

Linking against that library works only because of this faulty behavior. The transitive C libraries get linked, and the archive within the archive gets ignored (as long as the archive is not linked as --whole-archive, in which case linking fail with duplicate symbol).

[aherrmann]

Oh! Sorry, I overlooked that before. Yes, that's a bug then, good catch!

This rings a bell and I thought that issue had been addressed at some point in the past, but, it turns out that that was only for shared objects, see #275.

[cerisier]

Closed by #522

Will continue the conversation in #285