Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get all tests passing with --incompatible_disable_legacy_cc_provider for Bazel 0.25.0 #200

Closed
oquenchil opened this issue Mar 5, 2019 · 7 comments

Comments

@oquenchil
Copy link

oquenchil commented Mar 5, 2019

It seems that rules_rust will start breaking with the next Bazel release 0.25 because of an incompatible change (bazelbuild/bazel#7036)

ERROR: /home/bazel/.cache/bazel/_bazel_bazel/c922ef9e5f39c5bf7c8601dab4c6fea8/external/examples/ffi/rust_calling_c/BUILD:29:1: in rust_library rule @examples//ffi/rust_calling_c:matrix_dynamically_linked:
--
  | Traceback (most recent call last):
  | File "/home/bazel/.cache/bazel/_bazel_bazel/c922ef9e5f39c5bf7c8601dab4c6fea8/external/examples/ffi/rust_calling_c/BUILD", line 29
  | rust_library(name = 'matrix_dynamically_linked')
  | File "/home/bazel/.cache/bazel/_bazel_bazel/c922ef9e5f39c5bf7c8601dab4c6fea8/external/io_bazel_rules_rust/rust/private/rust.bzl", line 85, in _rust_library_impl
  | rustc_compile_action(ctx = ctx, toolchain = toolchain, cr...)), ...)
  | File "/home/bazel/.cache/bazel/_bazel_bazel/c922ef9e5f39c5bf7c8601dab4c6fea8/external/io_bazel_rules_rust/rust/private/rustc.bzl", line 180, in rustc_compile_action
  | collect_deps(crate_info.deps, toolchain)
  | File "/home/bazel/.cache/bazel/_bazel_bazel/c922ef9e5f39c5bf7c8601dab4c6fea8/external/io_bazel_rules_rust/rust/private/rustc.bzl", line 116, in collect_deps
  | fail(("rust targets can only depend o...)), ...")
  | attribute deps: rust targets can only depend on rust_library, rust_*_library or cc_library targets.<target @examples//ffi/rust_calling_c/c:native_matrix_so>

The code is using the old legacy provider cc, it can be replaced with CcInfo to get the same functionality as described here.

@mfarrugi
Copy link
Collaborator

mfarrugi commented Mar 5, 2019

Same as #190.

Looks like CI is on bazel 0.22 right now, which suggests it's still to early to bump rules_rust minimum bazel version to 0.22.

@oquenchil
Copy link
Author

Hey Marco, this is still showing up as broken in our CI. If I remember correctly 0.23 already had the API available, perhaps even as early as 0.22. Is anything blocking this that we can help with?

@mfarrugi
Copy link
Collaborator

#190 mentions the version constraints.

Which CI is broken? I don't want to do this upgrade right now because we are on v20 internally, and v24 is not released yet (the straightforward update bumps up our minimum version to v22, which has only been out for a month).

We could support both APIs if you need this change right now, and I would be happy to take a PR for it.

@oquenchil
Copy link
Author

rules_rust is in our list of downstream projects (in Bazel). We just wanted to let you know about this so that the breaking change doesn't catch you by surprise. If I understand correctly you will upgrade to v24 once it's released and then fix the incompatible change?

@mfarrugi
Copy link
Collaborator

Yes, I had planned to upgrade once v24 was released (so then we would have support 22, 23, and 24).

@hlopko
Copy link
Member

hlopko commented Mar 27, 2019

Hello, just a friendly notification that 0.24 was released yesterday :)

mfarrugi added a commit to mfarrugi/rules_rust that referenced this issue Mar 27, 2019
hlopko pushed a commit to bazelbuild/continuous-integration that referenced this issue Mar 29, 2019
mfarrugi added a commit that referenced this issue Apr 1, 2019
* #190, #200 :: Migrate to new cc provider.

* re-replace dep.cc.libs
@meteorcloudy
Copy link
Member

@mfarrugi Thanks for the fix!

meteorcloudy added a commit to bazelbuild/continuous-integration that referenced this issue Apr 2, 2019
meteorcloudy added a commit to bazelbuild/continuous-integration that referenced this issue Apr 2, 2019
* Re-enable Cartographer

... and really disable Tulsi

* Re-enable rules_rust

bazelbuild/rules_rust#200 is resolved
joeleba pushed a commit to joeleba/continuous-integration that referenced this issue Jun 17, 2019
joeleba pushed a commit to joeleba/continuous-integration that referenced this issue Jun 17, 2019
* Re-enable Cartographer

... and really disable Tulsi

* Re-enable rules_rust

bazelbuild/rules_rust#200 is resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants