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

Conditional compilation of workspace members via features #11526

Closed
IsaccBarker opened this issue Jan 2, 2023 · 4 comments
Closed

Conditional compilation of workspace members via features #11526

IsaccBarker opened this issue Jan 2, 2023 · 4 comments
Labels
A-features Area: features — conditional compilation A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@IsaccBarker
Copy link

IsaccBarker commented Jan 2, 2023

Problem

I'm writing a kernel and I want to split out x86 and x86_64 specific code, so I can publish it to crates.io and possible use it in another project. Best way I can think to do this is through workspaces. However, when compiling my kernel, I don't want to compile the x86 crate, when, for example, building for aarch64 or RISC-V. I'm currently doing something like the below:

// src/lib.rs

#![no_std]
#[cfg(target_arch = "x86")]
mod x86;
#[cfg(target_arch = "x86")]
pub use x86::*;

This obviously isn't optimal. Similar to #5220?

Proposed Solution

I would propose something like below:

[workspace]
members = [
    "<kernel_name>",
]

[workspace.'cfg(feature = "x86")']
members = [
    "<x86 crate>"
]

or

[workspace]
members = [
    "<kernel_name>",
    "feature.x86:<x86 crate>"
]

or something to that affect.

Notes

No response (yet, haha!)

@IsaccBarker IsaccBarker added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jan 2, 2023
@weihanglo
Copy link
Member

Thank you for proposing this feature! Personally, I've got questions on this proposal:

  • cfg(feature = "…") are never a thing even in a non-workspace Cargo.toml manifest. Will it confuse people that workspace supports cfg(feature = …) whereas others are not? Or does it imply we need find a way to support them all (which is unlikely in the near feature)?
  • Where are those "workspace features" defined? What does that mean when comparing to package features? Will package's default features affect workspace features?
  • Having "workspace features" means a developer need to specify in via command line, probably as cargo build --workspace --features x86. It feels a bit lengthy. libc depends on target cfg, which is auto-detected by rustc without any user interaction. What's the technical problem on this solution to you?

I am not entirely understand your use case. Looks like #5220 can cover it. Could you share more on that?

@IsaccBarker
Copy link
Author

IsaccBarker commented Jan 4, 2023

Thanks for your response, sorry this isn't so timely.

I must have not been thinking very clearly when I wrote that example! Maybe something like this could avoid "workspace features".

# /Cargo.toml
[workspace]
members = [
    "foo",
]

# /foo/Cargo.toml
# ...
[dependencies]
bar = {version = "0.1.0", path = "../bar", optional = true}

[features]
bar = ["dep:bar", "workspace_member:bar"]
# ...

# /bar/Cargo.toml
# ...

Which achieves bar only being compiled and linked in if foo has the "bar" feature specified. It's inline with how you might have a feature depend on a dependency like bar = ["dep:bar"].

In addition, would it be clearer if a workspace member had to be declared as "optional" much like a dependency would? I can see three ways of doing this, if at all:

# Way 1, /Cargo.toml
[workspace]
members = [ "bar:optional" ]

# Way 2, /Cargo.toml
[workspace]
members = [ "bar" ]

[workspace.bar]
optional = true

# Way 3, /bar/Cargo.toml
optional = true

Sorry it wasn't initially clear :)

@IsaccBarker IsaccBarker changed the title Per feature config for workspace Conditional compilation of workspace members via features Jan 4, 2023
@weihanglo
Copy link
Member

weihanglo commented Jan 13, 2023

Thanks for the understandable examples.

Given the feature syntax and possible extension is a bit in flux. I would suggest not extend it meanings to workspace at this moment. Give it a glance on the label A-features Area: features — conditional compilation for what it will evolve. Besides, way 2 and 3 might be a reasonable syntax, but how do we expect to activate them, via cargo build -p bar? Cargo already provides that functionality today.

I can see the proposed solutions to fix things, but I don't really see the "problem". If we can select member via -p/--package, do we still need a new syntax? Moreover, -p/--package flag support shell glob syntax. A user can also suffix crate that supported to build under x86 and run cargo build -p '*-x86' to build all of them.

BTW, I feel like we cannot entirely avoid #[cfg(target_arch = "x86")], since the crate still needs to compile only on that platform.

@weihanglo
Copy link
Member

From #11987 (comment) epage said,

As the syntax is laid out, I read it as cargo resolving differently depending on the platform which means the lockfile is no longer cross-platform.

This is a good argument why this might not be a good idea.

Given this reason, and also it is conceptually the same as #5220, I'm going to close due to inactivity for months. Thanks for the proposal anyway!

@weihanglo weihanglo closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-features Area: features — conditional compilation A-workspaces Area: workspaces C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

No branches or pull requests

2 participants