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

Create a cargo workspace with testing instructions. #132

Closed
wants to merge 8 commits into from

Conversation

gibbz00
Copy link

@gibbz00 gibbz00 commented Oct 6, 2024

Not very useful, I know, but I'm interested in contributing to this project further.

I might send a couple of PRs similar to this one while taking a look around. Feel free to let me know if that's inappropriate :)

@lulf
Copy link
Member

lulf commented Oct 7, 2024

You can't mix different target architectures in the same workspace, so I'm not sure if this is going to work :/

@gibbz00
Copy link
Author

gibbz00 commented Oct 7, 2024

Not sure I 100% follow. Are there cases where you are interested in building separate crates for different targets? And why wouldn't the --target flag work?

@HaoboGu
Copy link

HaoboGu commented Oct 9, 2024

It's common for a Rust embedded library to be used in different targets. In this case, cargo workspace won't work. It doesn't support per crate target and per crate features.

@gibbz00
Copy link
Author

gibbz00 commented Oct 9, 2024

It's common for a Rust embedded library to be used in different targets. In this case, cargo workspace won't work. It doesn't support per crate target and per crate features.

Sorry, I don't intend to be rude, but you just repeated Ulf without pointing to any definitive sources.

You can place Cargo.tomconfig.tomls in each crate if workspace commands error due to incorrect target triples. cargo check and cargo test do already work without any issues, and I've not changed how the ci.sh script invokes each example with an explicit target tripple.

@HaoboGu
Copy link

HaoboGu commented Oct 9, 2024

You can place Cargo.tomls in each crate if workspace commands error due to incorrect target triples. cargo check and cargo test do already work without any issues, and I've not changed how the ci.sh script invokes each example with an explicit target tripple.

That makes no difference if you use cargo check -p crate_name on each sub-crate in workspace, AND it will break rust analyzer, since different crates may require different version of same dependency, which cannot be resolved by cargo right now. Some related discusses can be found here, here and here. Monorepo is more preferred way by now.

@gibbz00
Copy link
Author

gibbz00 commented Oct 9, 2024

My bad, I meant to write config.toml. In fact, that is exactly what the closed issue in the second link you sent recommends. So just to clarify, you aren't referring to an issue cargo but an issue in rust-analyzer? In that case rust-lang/rust-analyzer#10021 is the issue that should be tracked.

@gibbz00
Copy link
Author

gibbz00 commented Oct 9, 2024

since different crates may require different version of same dependency, which cannot be resolved by cargo right now.

Also, this is very very wrong.

@HaoboGu
Copy link

HaoboGu commented Oct 9, 2024

Yeah, I got you. Involving config.toml requires use cargo check -p crate_name for each crate, which has no advantage comparing with cargo check --manifest-path path. You cannot just do cargo check or cargo test in the workspace root when there are multiple targets

@HaoboGu
Copy link

HaoboGu commented Oct 9, 2024

My bad, I meant to write config.toml. In fact, that is exactly what the closed issue in the second link you sent recommends. So just to clarify, you aren't referring to an issue cargo but an issue in rust-analyzer? In that case rust-lang/rust-analyzer#10021 is the issue that should be tracked.

The tracking issue is: rust-lang/cargo#9406

@gibbz00
Copy link
Author

gibbz00 commented Oct 9, 2024

Involving config.toml requires use cargo check -p crate_name for each crate, which has no advantage comparing with cargo check --manifest-path path.

Awesome, thanks. Now I understand where you're coming from. I tried this in a toy example, and you're right, cargo doesn't seem to build with per package targets.

The tracking issue is: rust-lang/cargo#9406

Sweet, thank you!

@gibbz00 gibbz00 closed this Oct 9, 2024
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