meson icon indicating copy to clipboard operation
meson copied to clipboard

cargo: Fix feature resolution

Open xclaesse opened this issue 2 years ago • 8 comments

Introduce a global Cargo interpreter state that keeps track of enabled
features on each crate.

Before generating AST of a Cargo subproject, it downloads every
sub-subproject and resolves the set of features enabled on each of them
recursively. When it later generates AST for one its dependencies, its
set of features and dependencies is already determined.

This is based on top of https://github.com/mesonbuild/meson/pull/12945

xclaesse avatar Mar 10 '24 01:03 xclaesse

Before this MR it was possible to have a wrap or Cargo.lock for one specific crate which then also has a Cargo.lock with its own dependencies. With this MR, that doesn't seem to be the case anymore.

swick avatar Jun 17 '24 15:06 swick

@swick I'm not sure exactly what you mean. If the main project has a wrap for a cargo dependency, the Cargo.lock from that dep will be used as well.

The big limitation this PR leaves is when you have multiple "entry point" into cargo. For example when the main project does :

adep = dependency('a-rs')
bdep = dependency('b-rs')

If a and b have a common c-rs crate, it will be configured with whatever features a needs. If b then needs extra features that's an error.

The easy workaround for that would be to have a single entry point, for example by adding rust.dependencies('a-rs', 'b-rs').

xclaesse avatar Jun 17 '24 15:06 xclaesse

@swick I'm not sure exactly what you mean. If the main project has a wrap for a cargo dependency, the Cargo.lock from that dep will be used as well.

Actually, reading the code again (been a while), it's a bit more complicated than that. The way Cargo works is the top level Cargo.lock takes precedence over all other dependencies. The main project's Cargo.lock is supposed to contain the whole dependency tree. It may not work exactly that way in Meson currently... need to double check.

xclaesse avatar Jun 17 '24 15:06 xclaesse

@swick I think I understood what you mean and I added a commit to fix your case. Can you test it again please?

xclaesse avatar Jun 17 '24 17:06 xclaesse

Same error locally as in CI.

e: self.is_subproject is a function: if not self.is_subproject():

swick avatar Jun 17 '24 19:06 swick

With that fixed it's behaving as before.

e: The questions are answered in the docs update of this PR. I still don't know how to make my setup work though.

If I have tow dependencies, their features are not unified though, are they? This is what I currently get:

subprojects/serde_derive-1.0.102/meson.build:67:-1: ERROR: Problem encountered: Dependency syn-1-rs previously configured with features ['quote', 'default', 'clone-impls', 'derive', 'parsing', 'printing', 'proc-macro'] but need ['quote', 'visit', 'default', 'clone-impls', 'derive', 'parsing', 'printing', 'proc-macro']

I'm also failing to turn on the specific feature:

project(...
  default_options: {
    'syn-1-rs:feature-visit': true,
  },
)
subprojects/syn-1.0.82/meson.build:1:-1: ERROR: In subproject syn-1-rs: Unknown options: "syn-1-rs:feature-visit"

swick avatar Jun 17 '24 20:06 swick

If I have tow dependencies, their features are not unified though, are they?

They are not unified, that's the limitation I mentioned that could be worked around by adding e.g. rust.dependencies('a-rs', 'b-rs').

I'm also failing to turn on the specific feature:

This PR removes those per subproject feature options because features are global. I think we should add a global module option like -Drust.features=foo,bar. More similar to cargo: https://doc.rust-lang.org/cargo/reference/features.html#command-line-feature-options.

As workaround for now, you can create a dummy cargo project that depends on both crates you need, and use that as meson subproject.

xclaesse avatar Jun 17 '24 22:06 xclaesse

As workaround for now, you can create a dummy cargo project that depends on both crates you need, and use that as meson subproject.

Yeah, that seems to work. Now the biggest issue seems to be target.cfg.dependencies not working.

swick avatar Jun 17 '24 23:06 swick