Migrate to the modern linker input API. #463
Conversation
|
3.0.0 |
|
docs/index.md mentions 0.26.0, so that should probably be updated. I don't have a good way of verifying 0.26.0 still works. |
|
Does anyone know what it would take to remove |
benjaminp
force-pushed
the
modern-linking
branch
from
dd5f049 to
6cfd5d5
Compare
|
It should probably just be renamed or inlined into the callers as desired. |
|
So this pull request addresses the usage of any legacy/deprecated behavior in this file? |
|
Yes. |
|
Would it make sense to move those methods into |
|
Probably. I can do that in this PR if you like. |
|
I think that'd be a nice improvement but I leave it up to @illicitonion and @dfreese 😃 |
|
Pinging this PR. It'd be nice to be able clean up those legacy files |
|
@damienmg do you have time during your day today to check this out? |
benjaminp
force-pushed
the
modern-linking
branch
from
6cfd5d5 to
6340990
Compare
| @@ -11,8 +11,8 @@ def get_libs_for_static_executable(dep): | |||
| Returns: | |||
| depset: A depset[File] | |||
| """ | |||
| libraries_to_link = dep[CcInfo].linking_context.libraries_to_link | |||
| return depset([_get_preferred_artifact(lib) for lib in libraries_to_link.to_list()]) | |||
| linker_inputs = dep[CcInfo].linking_context.linker_inputs.to_list() | |||
There was a problem hiding this comment.
Can you delete this file and move the contents into util.bzl?
benjaminp
force-pushed
the
modern-linking
branch
from
6340990 to
8460f34
Compare
benjaminp
force-pushed
the
modern-linking
branch
from
8460f34 to
a8dd4af
Compare
There was a problem hiding this comment.
LGTM
We should probably try to set up CI running the claimed minimum supported version at some point, but that's definitely not a matter for this PR :)
Over to @damienmg to merge!
Yes unfortunately this is a large task as the bazel CI is unlikely to do so. Also Bazel 4 is supposed to be a LTS and I would expect they would test rules with LTS for the support duration of the LTS. At which point the minimal version should always probably be the oldest LTS still supported.
Done 👍 |
|
Sorry for merging, I spoke too fast. We need to wait a bit more to see if the rebasing worked. |
Summary: We pull in bazelbuild/rules_rust#468 to make it easier to build protos, while not pulling in bazelbuild/rules_rust#463, because that would require us to update Bazel to 3.0.0. Happily, the commit that we need merged immediately before the commit that we don’t want! (Yes, this is a wee bit precarious; we’re working on the Bazel upgrade: #4277.) Test Plan: With both Bazel 2.1.0 and Bazel 3.7.0, everything builds, all tests pass, and TensorBoard looks okay enough. wchargin-branch: rules-rust-20201105a wchargin-source: bb3180a3a2a415d800d6a2a925fef0cb4e376d2f
Summary: We pull in bazelbuild/rules_rust#468 to make it easier to build protos, while not pulling in bazelbuild/rules_rust#463, because that would require us to update Bazel to 3.0.0. Happily, the commit that we need merged immediately before the commit that we don’t want! (Yes, this is a wee bit precarious; we’re working on the Bazel upgrade: #4277.) Test Plan: With both Bazel 2.1.0 and Bazel 3.7.0, everything builds, all tests pass, and TensorBoard looks okay enough. wchargin-branch: rules-rust-20201105a
Brought up in the discussion of bazelbuild#463, was that testing against multiple bazel versions with bazelci didn't seem possible without a fair bit of work. Judging based on usage that I have seen, we probably moved too quickly on bumping the minimum version to 3.0.0, but until we can actually test against our minimum version, we won't be able to reasonably version breaking changes as suggested by bazelbuild#415. This initial PR leaves a lot of on the table in terms of potential optimizations, and is fixed on Ubuntu 20.04 LTS for the moment.
See bazelbuild/bazel#10860.