-
Notifications
You must be signed in to change notification settings - Fork 104
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
Cargo Raze ignores binary dependencies #218
Comments
@acmcarther While you're around, maybe you could dump some context on this issue and the change associated with it? |
@damienmg you might want to check this out as well. |
I started a thread on zulip and I think the issue has been identified. It seems |
I'm wondering now if there should be a new |
This appears to be an issue in |
I think until binary dependencies are officially supported by |
@acmcarther I could really use your input here. Is this already a known and solved issue? |
I don't want to set an expectation that I always appear when asked (I am really flaky), but I'll try to answer questions that I have the context for. Bummer about this situation. Aight so the backstory: Originally, the implementation of this crate linked cargo as a dependency directly and used its internals to resolve the complete set of dependencies. With that, we could identify the BUILD files that needed to be generated and generate them. It turned out that having To that end, at some point in the past, Cargo got a "cargo-metadata" command which purported to emit all of the metadata that one would need in order to construct the full dependency tree and what not. I refactored the raze implementation in order to support using either cargo internals, or "cargo-metadata". In practice, the folks harmed by the ongoing dependency that this crate had on I had a general awareness that there might be issues here which were not obvious because they would only show up for users with esoteric use cases. It's surprising to me to read that so many dependencies are omitted. You've linked above a relevant cargo issue. That probably represents the root cause. Basically, I don't have a good idea of what the next steps should be. There's always the option of adding an optional dependency on the cargo internals again and supporting that as an implementation again. Beyond that though I really don't know the options, especially as they pertain to changes within cargo itself. |
@acmcarther Hey! Thanks so much for your reply! I hope I'm not over stepping by pinging everyone. I asked for you in particular because I had a feeling you would have context on this 😃. I have an idea for explicitly handling binary dependencies which I'm hoping to show in a PR soon. Maybe I'll be able to finish it over the weekend. Other than that, maybe cargo_internal could be added again behind a feature flag but I feel like that would not be the greatest since it would reintroduce old issues and feels kinda like relying on undefined behavior. But even then maybe it's the smoothest implementation. |
This is resolved as of #227 and was released in |
I was doing some tests trying to refresh things in
rules_rust
and came across an issue with generating targets for@io_bazel_rules_rust//wasm_bindgen/raze/...
.There appears to have been a regression introduced in 816ea69 (merged by #134) where a lot of the targets are no longer being generated. I've yet to nail down the specific cause. But, if you run
cargo raze
on or after this commit, you get the following output.This however, is a much smaller list than what is already generated. If you go back one commit before the one I mentioned, you get a list similar to what was originally generated back in October 2019 (see bazelbuild/rules_rust#240)
The text was updated successfully, but these errors were encountered: