Skip to content

cargo: Add global features options #14663

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xclaesse
Copy link
Member

@xclaesse xclaesse commented May 30, 2025

  • Add rust.add_cargo_features() method to add features from meson.build.
    It is similar to add_global_arguments(), we can add global cargo
    features until it is first used.
  • Add rust.cargo_features and rust.cargo_no_default_features CLI options
    to allow the user to add extra features.

@bonzini
Copy link
Collaborator

bonzini commented May 31, 2025

So, I thought more about this, and I think there are multiple parts:

  1. options of the superproject

  2. feature resolution over the whole Cargo workspace

  3. consulting the result of (2) from subprojects' meson/meson.build.

This pull request is that it conflates use 1 and use 2. It is not possible to set the cargo features based on options of the superproject, which has two problems:

  • the set of desired subproject options may vary depending on the superproject's options (those defined in meson_options.txt). So it is important to be able to configure the initial set of features starting from get_option().

  • using global rust.* options lets the user mess with the initial set of features. This makes it too easy for the user to break the build.

So these two uses should be separated. This is the direction I went with my own proposals at #14639 and #14660, though admittedly that was more of a gut design than something well-thought: for use 1, define options in meson_options.txt as usual. For use 2, pass features by hand to the Rust module as in #14639.

This leaves use 3, i.e. consulting the result of global feature resolution from the meson/meson.build files, a global rust.* option is not needed either. It's also messy because you can specify features such as foo/bar, resulting in different features being enabled for different subproject. The meson/meson.build file then would have to check both bar and foo/bar. It's much easier to query these with get_option() as in older versions of Meson; either with many feature-foo bools as in #14660, or with a single features option of combo or array type.

So in the end I'm not sure that this PR is a good step, but maybe I'm overlooking something.

@xclaesse
Copy link
Member Author

xclaesse commented May 31, 2025

I think the problem here is we can't rely on traditional Meson options for Cargo features. Cargo features are global and additive by nature.

Let's assume the main project has a cargo subproject named foo-rs-1 that has 2 optional features: f1 and f2. Let's assume the main project does require feature f1 from foo-rs-1. If user builds with -Drust.cargo_features=f2, now foo-rs-1 must be configured with f1 AND f2. User option should not override main project features, but add to it.

foo-rs-1 could also have a 3rd feature f3 that the main project only requires if built with -Dsomething=true.

If cargo features were a traditional per-subproject Meson option, user could do -Dfoo-rs-1:cargo_features=f3 and that would override what the main project wants, instead of adding.

This PR only covers the user side, but I agree we still need a way for main project to also be able to add features.

consulting the result of (2) from subprojects' meson/meson.build.

That's an easy one, when generating the AST we know the final list of features enabled on that crate. It in fact already write it at the end:

dep = declare_dependency(
  link_with : lib,
  variables : {'features' : 'f1'}
)

We can just declare that list before calling subdir:

enabled_features = ['f1']
...
subdir('meson')
...
dep = declare_dependency(
  link_with : lib,
  variables : {'features' : enabled_features}
)

...

@bonzini
Copy link
Collaborator

bonzini commented May 31, 2025

So I think we agree that both #14660 and this are kind of dead ends, and something like #14639 is more complex but the right way to do it?

@xclaesse xclaesse force-pushed the cargo-user-features branch from 3033ccf to e59aa4a Compare May 31, 2025 19:46
@xclaesse
Copy link
Member Author

Updated this PR to add rust.add_cargo_features('f1', 'foo/f2', ...). This lets the main project define features in addition to those set by the user with -Drust.cargo_features=f1,....

@xclaesse
Copy link
Member Author

Also define features variable before subdir().

@bonzini I think this should cover all your cases?

xclaesse added 2 commits June 1, 2025 19:35
- Add rust.add_cargo_features() method to add features from meson.build.
  It is similar to add_global_arguments(), we can add global cargo
  features until it is first used.
- Add rust.cargo_features and rust.cargo_no_default_features CLI options
  to allow the user to add extra features.
@xclaesse xclaesse force-pushed the cargo-user-features branch from 567fa5f to d6a2c1f Compare June 1, 2025 23:35
@bonzini
Copy link
Collaborator

bonzini commented Jun 2, 2025

Defining features early is clearly a bugfix, and it could be split to its own PR. I am not sure about the API for features; I still feel like it should be part of the support for non-subproject Cargo.tomls.

@xclaesse
Copy link
Member Author

xclaesse commented Jun 2, 2025

I still feel like it should be part of the support for non-subproject Cargo.tomls.

What do you mean by non-subproject Cargo.toml?

@bonzini
Copy link
Collaborator

bonzini commented Jun 2, 2025

I mean building pure-Rust targets (i.e. those that don't link directly to any C dependencies) from a Cargo.toml file, even if they are not from a subproject. I described it a bit more in the rust.workspace() proposal.

@xclaesse
Copy link
Member Author

xclaesse commented Jun 2, 2025

You mean if the main project has a Cargo.toml? I thought about adding meson setup --method=cargo and not even require a meson.build file at all.

@bonzini
Copy link
Collaborator

bonzini commented Jun 2, 2025

You mean if the main project has a Cargo.toml

Or more than one. In practice for the use cases where people will use Meson there will be mixed C/Rust code and some kind of Meson declarations will be needed. But most likely you will have some leaf crates that don't need any Meson-specific code, only perhaps some override_dependency. And even for those that have mixed C and Rust it's useful to have them participate in global feature resolution.

@eli-schwartz
Copy link
Member

I think the problem here is we can't rely on traditional Meson options for Cargo features. Cargo features are global and additive by nature.

Is that a rustc or a cargo limitation?

How does the rust ecosystem handle multiple crates that use the same word for a build option with different semantic meanings? English is a limited namespace unless you start adding the crate name as a prefix to all build options, which, well, "works" but also lol...

@bonzini
Copy link
Collaborator

bonzini commented Jun 4, 2025

It's a Cargo limitation. They live with it because most crates will not have any, or will only have "std" and "alloc". If a "feature" can be implemented through a separate compilation unit without running into language limitations, thet do that because the crate acts as both compilation and distribution unit.

@eli-schwartz
Copy link
Member

If it's only a limitation of cargo when building a cargo dependency graph then maybe we just don't need to implement its flaws, and instead we can just treat cfgs as independently toggleable per-project settings even if the cfg names happen to coincide?

@LingMan
Copy link

LingMan commented Jun 4, 2025

This reads a bit like a misunderstanding.

There is no global set of all feature names that would cause collisions if two crates happen to use the same name for one of their features.

Both on the command line and in the Cargo.toml you can only enable your own features (e.g. --features foo) or the features of your direct dependencies (e.g. --features mydep/foo).

If a crate wants to expose a feature of one of it's dependencies to its dependents, it has to define its own feature (potentially with the same name) and then enable the dependency's feature based on that.

Let's say two crates, a and b, in the dependency tree separately depend on foo. a enables foo's f1, b enables foo's f2. Then foo will only be compiled once with both f1 and f2 enabled. That's the global feature unification.

@eli-schwartz
Copy link
Member

Ok. Then it sounds like meson project options are a good mapping to cargo features.

And the problem is "just" resolving the issue where, to put it in "regular" C/C++ terms, multiple subprojects each have a dependency on a third subproject with different default_options: [...].

It's not cargo-specific for a subproject libfoobar to require option-one=true for the sake of subp-a, and require option-two=true for subp-b. They aren't clashing requirements, you can build libfoobar.so with support for both optional interfaces if suitably configured.

Just, usually if you don't explicitly set both enabled in the superproject, the first call to dependency() is the only one that passes default_options to the subproject invocation. And then building fails because, idk, subp-b tries to build against libfoobar.so's optional dbus support but libfoobar.so got configured instead with the optional libmicrohttpd embedded server.

Or, libfoobar.so is actually Gtk-3 and subp-b tries to build unconditionally against the gdk X11 backend (because everyone knows that only X11 exists), but the first call to dependency() was in subp-a which made sure to toggle on the Wayland gdk backend (because of how incredibly modern it is). And some bright fellow figured that it makes sense to not default to X11 since "nobody should be using that anymore" (look, I have to deal with Gentoo users, this is a sore point).

@eli-schwartz
Copy link
Member

Which basically leaves me wondering why it's impossible to observe any discussion about rust without the claim being made that rust is "leaving behind traditional approaches, and the old assumptions don't work anymore", when that is manifestly untrue...

@bonzini
Copy link
Collaborator

bonzini commented Jun 4, 2025

Yes, it is possible and it's how it worked until 1.7.0. It's just a pain in the neck.

It's not that Rust leaves behind traditional approaches, in fact I hate it when it does. It's more that Rust already has a declarative build syntax which is enough in most cases, and there's a lot of tooling built around that syntax (sometimes it's easy to replicate it in Meson, see ninja clippy or ninja rustdoc; sometimes it's not). For that reason I'd rather avoid having to duplicate build rules in meson.build whenever the contents of one or more Cargo.toml files are enough to describe how the project is built.

@eli-schwartz
Copy link
Member

Perhaps what we really need here (?) is per subproject options but also build the full Cargo.toml dependency graph at once and "smartly" toggle the defaults during project synthesis?

@bonzini
Copy link
Collaborator

bonzini commented Jun 4, 2025

Perhaps what we really need here (?) is per subproject options but also build the full Cargo.toml dependency graph at once and "smartly" toggle the defaults during project synthesis?

Yes, that is the direction that both @xclaesse and myself agree on. He has this simpler solution which leaves most of the work to meson.build, while I was thinking of doing more Cargo.toml parsing to find the features, but the differences are more at the language design level and less at the implementation level.

@xclaesse
Copy link
Member Author

xclaesse commented Jun 4, 2025

Hmm, I indeed misunderstood cargo's --features cli. I thought those features were enabled on all packages, but it's only on the main project. Thanks for pointing that!

I think in that case this PR doesn't make much sense, I need to rethink this.

@xclaesse
Copy link
Member Author

xclaesse commented Jun 4, 2025

The way per subproject option was working before had some issues:

  • They get evaluated after we generated the ast, that's too late. Enabling a feature can in turn enable other features on subprojects we already configured. To fix that, we now evaluate the whole dep tree, and all their features, before generating any AST.

  • We have no way to know the list of features a crate has. They don't have to be declared in Cargo.toml. That means we can't have a list of boolean options, but rather a free-form array of strings.

  • User should not be able to override features enabled on a cargo subproject from CLI. They should only add to that list, or even not have no control and leave it to the main project entirely.

@LingMan
Copy link

LingMan commented Jun 4, 2025

We have no way to know the list of features a crate has. They don't have to be declared in Cargo.toml.

Huh? That would be news to me.

$ cargo build --features lol
error: the package 'foo' does not contain this feature: lol

Optional dependencies create an implicit feature of the same name, which then wouldn't be listed in the [features] section of the manifest. A bit more parsing work, but very solvable. Are you referring to that?

They should only add to that list, [...]

Yes, features can be enabled but not disabled.

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.

4 participants