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

cargo: Fix feature resolution #12952

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

Conversation

xclaesse
Copy link
Member

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 #12945

@xclaesse xclaesse requested a review from jpakkane as a code owner March 10, 2024 01:01
run_unittests.py Dismissed Show dismissed Hide dismissed
mesonbuild/cargo/__init__.py Fixed Show fixed Hide fixed
@xclaesse xclaesse force-pushed the cargo-global-state branch 3 times, most recently from 0ca3969 to 8008bb7 Compare March 11, 2024 13:45
@xclaesse xclaesse force-pushed the cargo-global-state branch 2 times, most recently from d2da16b to 4aa5b35 Compare June 17, 2024 15:16
@swick
Copy link
Contributor

swick commented Jun 17, 2024

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.

@xclaesse
Copy link
Member Author

@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
Copy link
Member Author

@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
Copy link
Member Author

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

@swick
Copy link
Contributor

swick commented Jun 17, 2024

Same error locally as in CI.

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

@swick
Copy link
Contributor

swick commented Jun 17, 2024

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"

@xclaesse
Copy link
Member Author

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.

@swick
Copy link
Contributor

swick commented Jun 17, 2024

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.

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.
The library name defaults to its package name, but it can be different.
For example:
- package name: cairo-sys-rs
- library name: cairo-sys
- dependency name: ffi
In the case the main project has a .wrap file for a cargo subproject,
that subproject's Cargo.lock must be loaded before we can recursively
fetch all its dependencies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants