Add cc toolchain files to rustc dependencies (for linker)

[m3rcuriel]

Currently if you're using an artifacted compiler the Rust rules will complain that they can't find the linker script. This is because it is not a dependency.

I'm pretty sure there's a correct way to do this with the new toolchain system but I was looking around and saw that people mostly stuck with this with some TODOs to find a better way.

I tried to get it using ctx.toolchains but that doesn't actually have files, it seems.

[mfarrugi]

Looks fine to me, but I'd like another thumbs up since I know the least about bazel :)

…

On Tue, Apr 23, 2019, 18:19 Lee Mracek ***@***.***> wrote: @m3rcuriel <https://github.com/m3rcuriel> requested your review on: #217 <#217> Add cc toolchain files to rustc dependencies (for linker) as a code owner. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAROG6DNYHTKMSTWUDXUWBDPR6DOZANCNFSM4HH6254A> .

[damienmg]

This is fine to me, please add a TODO about finding a canonical way to include C++ toolchains files (indeed that should comes with cc_toolchain, @hlopko any idea?).

The only downside of doing that I can see is that we may have way to much files in the input of the action.

[hlopko]

Hi all,

to make sure I understand the issue correctly, you want to do this so the linker is present for the linking action, not the linker script (one is tool that does linking, another one is a script that ld can consume to tweak the linking process, used e.g. to specify symbol visibility etc), correct?

The best way to access C++ toolchain files is to use all_files , which is only in 0.25 (bazelbuild/bazel@f59038d) :). Until then, ctx.attr._cc_toolchain.files is good enough.

The best way to access the C++ toolchain is to use find_cpp_toolchain which is forward compatible with bazelbuild/bazel#6516. But this makes the full sense only in 0.25 with all_files.

[m3rcuriel]

@hlopko correct, this is in order to ensure that the linker itself (and its dependencies!) are available in the sandbox during linking.

I was going to use all_files as I am a nerd and run whatever most recent Bazel I can but stuck with cxt.attr._cc_toolchain.files because I realized all_files is extremely new.

Currently we get the toolchain out of find_cpp_toolchain. When we want to migrate to all_files we just have to move the depset down about 50 lines below where we get the linker to pass into rustc anyways.

[damienmg]

Suggested change

	# TODO wait for CcToolchainInfo.all_files to be available
	# C++ library linked in rust libraries need the linker to be available at compile-time.
	# TODO: Switch to CcToolchainInfo.all_files once available.

[m3rcuriel]

@mfarrugi I have no idea why this failed CI on one OS, and I don't have OS X to test on.

[mfarrugi]

You can get CI to run again by rebasing or making some cosmetic changes. At a glance the osx error looks like it's not flakey.. but would have failed on all platforms if that were the case.

…

On Sat, May 4, 2019 at 3:00 PM Lee Mracek ***@***.***> wrote: @mfarrugi <https://github.com/mfarrugi> I have no idea why this failed CI on one OS, and I don't have OS X to test on. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAROG6ACANDNQO56CR3MSZTPTXMLVANCNFSM4HH6254A> .

[m3rcuriel]

So we should probably add an artifacted compiler/linker set up to test now that I've gotten it working.

The reason it was failing is because ctx.files._cc_toolchain is an empty list on Linux, but contains File(s) on OS X. It needed to be wrapped in a depset. That it didn't fail / that no one but me has encountered the problem this PR should fix means that I should probably take my setup, strip it down, and make it part of the test suite.

[m3rcuriel]

Now featuring the required fixes to get this to work with --incompatible_enable_cc_toolchain_resolution.

CcToolchainInfo.all_files is introduced in 0.25.0, thus invalidating my TODO.

I'm fairly sure that flipping --incompatible_enable_cc_toolchain_resolution will just break this completely but I haven't downloaded an older Bazel version to check.