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

Respecting .cargo/config.toml #8741

Closed
Veykril opened this issue May 6, 2021 · 6 comments
Closed

Respecting .cargo/config.toml #8741

Veykril opened this issue May 6, 2021 · 6 comments
Labels
S-unactionable Issue requires feedback, design decisions or is blocked on other work

Comments

@Veykril
Copy link
Member

Veykril commented May 6, 2021

This is more of a collection of some issues floating around regarding the .cargo/config.toml and general discussion of how to solve these.

As pointed out in #5904 (comment):

I don't see any reason right now why .cargo/config wouldn't already have an effect -- we're just calling cargo check. Or do you mean also using the target from .cargo/config for analysis (i.e. rust-analyzer.cargo.target)? Maybe cargo metadata doesn't use that configuration, or we call it in a way that prevents that; that might be worth investigating. We probably don't want to parse .cargo/config ourselves though.

We probably do not want to parse this ourselves but cargo metadata doesn't seem to expose this info in its output either, see https://doc.rust-lang.org/cargo/commands/cargo-metadata.html#output-format. So we either have to parse it ourselves or get the relevant info to be emitted by cargo metadata from what I can tell.

@Veykril Veykril added the S-unactionable Issue requires feedback, design decisions or is blocked on other work label May 6, 2021
@flodiebold
Copy link
Member

Note that we explicitly pass --filter-platform to cargo metadata, so I'm not sure whether it would even be possible for it to respect the target config.

@Veykril
Copy link
Member Author

Veykril commented May 8, 2021

Hm, ye that might be a problem. I wonder if parsing the config might be not such a bad idea though, we wouldn't have to parse everything but a few things we are interested in anyways(currently build.rustflags and build.target)?
Though I guess it becomes a bit more complex when it comes to target.<triple>.rustflags.

@Veykril
Copy link
Member Author

Veykril commented May 8, 2021

Then again we don't have toml in our dependency tree and I don't think we would want to add that. I wonder if it would make sense to add a subcommand to cargo to emit the configuration as json similar to what metadata does for packages? That way cargo would also be the one to resolve the paths to the configs not requiring us to search for them in the appropriate folders.

Edit: Turns out this is already an unstable thing rust-lang/cargo#9301 🎉

@Veykril
Copy link
Member Author

Veykril commented May 8, 2021

Actually, I think we can get by with this rust-lang/cargo#9357 once it lands?
Nevermind, we can use it to resolve rustflags but not to get the target triple defined in the cargo config.

@Veykril
Copy link
Member Author

Veykril commented May 8, 2021

Managed to throw a working branch together: Veykril@8989fb8

f1Gup1aiAn

This uses unstable cargo features, cargo config get to read the target triple out of the config to pass to cargo metadata --filter-platform and cargo rustc --print to read out the rustc_cfgs. If those commands fail they fall back to what we previously used, namely invoking rustc directly.

Now the question is, as we don't really use nightly apis, does not using unstable things also apply to cargo when we have fallbacks in place if those commands fail?

Anyways at least we got a (future) solution to these problems.

bors bot added a commit that referenced this issue May 9, 2021
8774: feat: Honor `.cargo/config.toml` r=matklad a=Veykril

![f1Gup1aiAn](https://user-images.githubusercontent.com/3757771/117545448-1dcaae00-b026-11eb-977a-0f35a5e3f2e0.gif)

Implements `cargo/.config` build target and cfg access by using unstable cargo options:

- `cargo config get` to read the target triple out of the config to pass to `cargo metadata` --filter-platform
- `cargo rustc --print` to read out the `rustc_cfgs`, this causes us to honor `rustflags` and the like.

If those commands fail, due to not having a nightly toolchain present for example, they will fall back to invoking rustc directly as we currently do.

I personally think it should be fine to use these unstable options as they are unlikely to change(even if they did it shouldn't be a problem due to the fallback) and don't burden the user if they do not have a nightly toolchain at hand since we fall back to the previous behaviour.

cc #8741
Closes #6604, Closes #5904, Closes #8430, Closes #8480

Co-authored-by: Lukas Wirth <[email protected]>
@Veykril
Copy link
Member Author

Veykril commented May 10, 2021

The only open issue about this now is #5828, which I'm unsure what the cause of is. I'll close this issue in favor of the already opened issue there since its the only remaining one.

@Veykril Veykril closed this as completed May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-unactionable Issue requires feedback, design decisions or is blocked on other work
Projects
None yet
Development

No branches or pull requests

2 participants