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

Add note about rules_foreign_cc when using @cargo_raze//:raze. #395

Merged
merged 1 commit into from
Mar 6, 2021

Conversation

PiotrSikora
Copy link
Contributor

Signed-off-by: Piotr Sikora piotrsikora@google.com

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora PiotrSikora requested a review from acmcarther as a code owner March 6, 2021 03:42
@PiotrSikora
Copy link
Contributor Author

cc @UebelAndre

@dfreese dfreese merged commit 89a9a3c into google:main Mar 6, 2021
@UebelAndre
Copy link
Collaborator

This is not accurate. The macros are loading rules_foreign_cc. Did you find this to not be the case?

@PiotrSikora
Copy link
Contributor Author

This is not accurate. The macros are loading rules_foreign_cc. Did you find this to not be the case?

I found this not to be the case. I'm adding support for running @cargo_raze//:raze in the Proxy-Wasm Rust SDK repo in proxy-wasm/proxy-wasm-rust-sdk#88. You can check it out to verify:

$ git clone https://github.com/PiotrSikora/proxy-wasm-rust-sdk -b cargo-raze_v0.11.0 /tmp/raze
$ cd /tmp/raze

Right now, it includes http_archive(rules_foreign_cc) in bazel/repositories.bzl and it works fine:

$ bazel --noworkspace_rc run @cargo_raze//:raze -- --manifest-path=$(pwd)/Cargo.toml
WARNING: The following rc files are no longer being read, please transfer their contents or import their path into one of the standard rc files:
/tmp/raze/.bazelrc
Starting local Bazel server and connecting to it...
INFO: Invocation ID: d770fc81-2664-4888-bf33-9a8863100331
INFO: SHA256 (https://mirror.bazel.build/ftp.pcre.org/pub/pcre/pcre-8.43.tar.gz) = 0b8e7465dc5e98c757cc3650a20a7843ee4c3edf50aaf60bb33fd879690d2c73
DEBUG: Rule 'cargo_raze__pcre' indicated that a canonical reproducible form can be obtained by modifying arguments sha256 = "0b8e7465dc5e98c757cc3650a20a7843ee4c3edf50aaf60bb33fd879690d2c73"
DEBUG: Repository cargo_raze__pcre instantiated at:
  /tmp/raze/WORKSPACE:13:44: in <toplevel> /home/piotrsikora/.cache/bazel/_bazel_piotrsikora/29e3b7276a8bbe7fc5a302b857cd333d/external/proxy_wasm_rust_sdk/bazel/dependencies.bzl:27:28: in proxy_wasm_rust_sdk_cargo_raze_dependencies
/home/piotrsikora/.cache/bazel/_bazel_piotrsikora/29e3b7276a8bbe7fc5a302b857cd333d/external/cargo_raze/repositories.bzl:41:22: in cargo_raze_repositories
/home/piotrsikora/.cache/bazel/_bazel_piotrsikora/29e3b7276a8bbe7fc5a302b857cd333d/external/cargo_raze/third_party/pcre/pcre_repositories.bzl:7:10: in pcre_repositories
/home/piotrsikora/.cache/bazel/_bazel_piotrsikora/29e3b7276a8bbe7fc5a302b857cd333d/external/bazel_tools/tools/build_defs/repo/utils.bzl:201:18: in maybe
Repository rule http_archive defined at:
/home/piotrsikora/.cache/bazel/_bazel_piotrsikora/29e3b7276a8bbe7fc5a302b857cd333d/external/bazel_tools/tools/build_defs/repo/http.bzl:336:31: in <toplevel>
INFO: Analyzed target @cargo_raze//:raze (206 packages loaded, 15993 targets configured).
INFO: Found 1 target...
Target @cargo_raze//impl:cargo_raze_bin up-to-date:
  bazel-bin/external/cargo_raze/impl/cargo_raze_bin
INFO: Elapsed time: 46.023s, Critical Path: 12.31s
INFO: 342 processes: 263 remote cache hit, 79 internal.
INFO: Build completed successfully, 342 total actions
INFO: Build completed successfully, 342 total actions

But when you remove http_archive(rules_foreign_cc) from bazel/repositories.bzl and its workspace definitions from bazel/dependencies.bzl, then it fails:

$ bazel --noworkspace_rc run @cargo_raze//:raze -- --manifest-path=$(pwd)/Cargo.toml
WARNING: The following rc files are no longer being read, please transfer their contents or import their path into one of the standard rc files:
/home/piotrsikora/Google/proxy-wasm-rust-sdk/.bazelrc
INFO: Invocation ID: f23b6f18-524c-48a3-b345-ab498013727d
ERROR: Failed to load Starlark extension '@rules_foreign_cc//:workspace_definitions.bzl'.
Cycle in the workspace file detected. This indicates that a repository is used prior to being defined.
The following chain of repository dependencies lead to the missing definition.
 - @rules_foreign_cc
This could either mean you have to add the '@rules_foreign_cc' repository with a statement like `http_archive` in your WORKSPACE file (note that transitive dependencies are not added automatically), or move an existing definition earlier in your WORKSPACE file.
ERROR: cycles detected during target parsing
INFO: Elapsed time: 1.919s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded)
FAILED: Build did NOT complete successfully (0 packages loaded)

@UebelAndre
Copy link
Collaborator

That's because you're loading cargo_raze_transitive_deps in the same .bzl where yrules_foreign_cc is being defined (via cargo_raze_repositories).

@UebelAndre
Copy link
Collaborator

If you move the load for the transitive deps into a second file. You should be able to resolve your issue. But as it's written in the README.md, the comment is incorrect since that will work if you add it to a WORKSPACE file

@PiotrSikora
Copy link
Contributor Author

If you move the load for the transitive deps into a second file. You should be able to resolve your issue.

Unless I'm mistaken, that would require a third file (repositories.bzl to load @cargo_raze, dependencies.bzl to call cargo_raze_repositories() and a new file to call cargo_raze_transitive_deps()), which isn't pretty.

But as it's written in the README.md, the comment is incorrect since that will work if you add it to a WORKSPACE file

That's a fair point, although most "real world" projects (at least, those that I interact with) don't clutter WORKSPACE too much.

Also, why don't we import rules_rust via macros as well? It seems to work fine, see: #396.

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

Successfully merging this pull request may close these issues.

3 participants