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

Genquery: cycle in dependency graph #2522

Open
UebelAndre opened this issue Feb 27, 2024 · 5 comments
Open

Genquery: cycle in dependency graph #2522

UebelAndre opened this issue Feb 27, 2024 · 5 comments
Labels

Comments

@UebelAndre
Copy link
Collaborator

It seems using genquery on any Rust targets leads to failures due to a cycle in the dependency graph. The diff below can be used to repro the issue and get the output that follows.

diff --git a/crate_universe/BUILD.bazel b/crate_universe/BUILD.bazel
index 370dc5c3..ffb18c11 100644
--- a/crate_universe/BUILD.bazel
+++ b/crate_universe/BUILD.bazel
@@ -76,6 +76,12 @@ rust_binary(
     deps = [":cargo_bazel"],
 )

+genquery(
+    name = "genquery",
+    scope = [":cargo_bazel_bin"],
+    expression = "deps(:cargo_bazel_bin)",
+)
+
 alias(
     name = "bin",
     actual = ":cargo_bazel_bin",
~/Code/rules_rust (main ✗) bazel build //crate_universe:genquery
ERROR: /Users/user/Code/rules_rust/util/process_wrapper/BUILD.bazel:31:36: in rust_binary_without_process_wrapper rule //util/process_wrapper:process_wrapper: cycle in dependency graph:
    //crate_universe:genquery (e5528793e6772dcf5ff2b9774ea78fbb21b115ab96d65cadb8cb5b68206d2fe6)
    //crate_universe:genquery (577c459edef5fa899d167957b8c740aeb5edc881970dd81d94adeef309381026)
    //crate_universe:cargo_bazel_bin
.-> //util/process_wrapper:process_wrapper
|   //util/import:import
|   //util/import:import_macro_label
|   //util/import:import_macro
|   //util/import:import_macro_impl
`-- //util/process_wrapper:process_wrapper
ERROR: Analysis of target '//crate_universe:genquery' failed; build aborted
@UebelAndre UebelAndre added the bug label Feb 27, 2024
@UebelAndre
Copy link
Collaborator Author

cc @scentini @cfredric

@UebelAndre
Copy link
Collaborator Author

@daivinhtran
Copy link
Contributor

Could you the fix simply be removing _import_macro_dep from rust_binary_without_process_wrapper?

@UebelAndre
Copy link
Collaborator Author

I opened #2559 since it doesn't seem like there's a simple fix here. The import stuff likely needs it's own bootstrapping rule. Since that code is dead in the repo though, I think it's fair to remove this integration. Though I'd be happy to review a better fix. As long as the new integration test I added passes I'd be a happy camper.

github-merge-queue bot pushed a commit that referenced this issue Mar 26, 2024
@scentini
Copy link
Collaborator

@gregce @katre it seems like the issue here is genquery's sensitivity to loading phase dependency cycles. Looking through internal channels, --experimental_skip_ttvs_for_genquery should be the fix, however it is not enabled by default for Bazel; is there a reason for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants