#186 support new Rust 1.60+ syntax for "optional dependencies" and "dependency features"
Closed: Fixed 2 years ago by decathorpe. Opened 2 years ago by decathorpe.

documentation:

I think our dependency generators will just choke if they encounter a crate that uses those new features, so there will probably need to be adaptations:

  • features can now have the same name as an optional dependency
  • optional dependency foo that have the same name as a feature foo is now referenced as dep:foo in feature dependencies
  • conditional dependencies are not supported by dependency generators yet at all: foo?/bar would mean "require feature bar of crate foo if optional dependency foo is pulled in explicitly in some other way"

What worries me most is that our mapping of "optional dependency" / "cargo feature" to subpackages might no longer work with this scheme (because optional dependencies and a cargo features can now have the same name, but different dependencies ...).


This example from the docs looks like it should be solvable with our current approach:

[dependencies]
ravif = { version = "0.6.3", optional = true }
rgb = { version = "0.8.25", optional = true }

[features]
avif = ["dep:ravif", "dep:rgb"]

Here, there's no overlap between feature names and optional dependencies.


This example looks like it might break slightly:

[dependencies]
serde = { version = "1.0.133", optional = true }
rgb = { version = "0.8.25", optional = true }

[features]
serde = ["dep:serde", "rgb?/serde"]

We would probably need to collapse both the serde feature and serde optional dependency into the same subpackage for dependency resolution (but in this case, that should not be a problem, because they're always both enabled at the same time).

And the generated dependencies for this package rust-foo+serde-devel might need to be something like:

Requires: crate(serde)
Requires: (crate(rgb/serde) if crate(rgb))

I'm not fond of if rich dependencies, but I am not sure if this can be solved any other way in RPM metadata.


I'm only worried that some developers might get "creative" with these new features and do something weird. According to the documentation,

In some cases, you may not want to expose a feature that has the same name as the optional dependency. For example, perhaps the optional dependency is an internal detail, or you want to group multiple optional dependencies together, or you just want to use a better name. If you specify the optional dependency with the dep: prefix anywhere in the [features] table, that disables the implicit feature.

the "implicit feature" for optional dependencies is only disabled if the explicit, new dep: form is used anywhere in Cargo.toml.

Sounds like a nightmare to me ... but at least we have a test suite for this kind of stuff in rust2rpm :(


When an explicit feature mentions an optional dependency as "dep:optional", then that dependency is no longer a feature of its own at all -- regardless of whether the explicit feature is called the same thing or not. So we don't need a rust-foo+optional-devel subpackage for that dependency at all, just rust-foo+feature-devel depending on crate(optional).

For the conditional dependency, I don't think it's completely global, more like

%package -n rust-foo+serde-devel
Requires: crate(serde)
Requires: (crate(rgb/serde) if crate(foo/rgb))

i.e. looking for its sibling subpackage as the condition, especially because "rgb?" could be yet another feature name rather than a dependency.

especially because "rgb?" could be yet another feature name rather than a dependency.

Hmm, actually this point doesn't make sense, because a regular feature wouldn't have some "foo?/bar" sub-feature to enable this way.

But still, I think if crate(foo/rgb) is more correct than if crate(rgb).

One thing that should help is that cargo read-manifest is also now explicit about optional dependencies that create an implicit feature, so this manifest:

[dependencies]
serde = { version = "1.0.133", optional = true }
rgb = { version = "0.8.25", optional = true }

[features]
serde = ["dep:serde", "rgb?/serde"]

will now list these features:

  "features": {
    "rgb": [
      "dep:rgb"
    ],
    "serde": [
      "dep:serde",
      "rgb?/serde"
    ]
  },

So I think rust2rpm doesn't need to figure out optional dependencies as implicit features at all anymore -- just take what's spelled out here.

:frowning: looks like crates are starting to use this feature now ...

I think we need to do the following:

  • specify how to map those new dependency types to RPM equivalents
  • implement mapping in the dependency generator
  • add unit tests :rocket:
  • release a new rust2rpm version :tada:
[dependencies]
serde = { version = "1.0.133", optional = true }
rgb = { version = "0.8.25", optional = true }

[features]
serde = ["dep:serde", "rgb?/serde"]

Ok, to start things off, I think these examples would translate into:

  • an rgb subpackage for the optional rgb dependency, with a generated dependency on:

(crate(rgb) >= 0.8.25 with crate(rgb) < 0.9~)

(same as "old" style, since the rgb feature is not named as dep:rgb in the features table)

  • a serde subpackage for the serde feature, with generated dependencies on both:

(crate(serde) >= 1.0.133 with crate(serde) < 2.0~)

and

((crate(rgb/serde) >= 0.8.25 with crate(rgb/serde) < 0.9~) if (crate(rgb) >= 0.8.25 with crate(rgb) < 0.9~)

Do we need to specify the version restrictions in all the places here? I think we do ...

  • no subpackage for the serde optional dependency, since it is named as dep:serde in the features table

This seems to correspond to what cargo read-manifest prints, so we should only need to map the new foo?/bar style dependencies to (crate(foo/bar) if crate(foo)) (with version restrictions, as shown above?)?

Metadata Update from @decathorpe:
- Issue set to the milestone: 23

2 years ago

The time has arrived: zbus 3.0.0 uses the new syntax and can't be updated.
https://bugzilla.redhat.com/show_bug.cgi?id=2119164

Punting to v24 since this will require a complete rewrite of the RPM generators for Provides and Requires.

Metadata Update from @decathorpe:
- Issue set to the milestone: 24 (was: 23)

2 years ago

gstreamer-rs v0.19 is also blocked by this. could somebody comment on https://pagure.io/fedora-rust/rust2rpm/issue/186#comment-804788 whether what I wrote makes sense or not?

Maybe give it a try the way you proposed in https://pagure.io/fedora-rust/rust2rpm/issue/186#comment-804788 and we can see if it seems to work right in practice?

Seems sane to me, but then again I don't know much about rust and cargo.

@jistone What do you think?

Yeah, I'll probably start from there. Since the current way this is implemented in rust2rpm is completely incompatible with these features, it needs to be written from scratch anyway, so I'll probably do some fancy TDD and start by writing simple unit tests :) I've now encountered a few crates that started using this feature (zbus 3, gstreamer, rust_decimal), so we have good real-world test cases, as well.

I still think you don't need to manually figure out whether something was or wasn't named as a dep:foo, because cargo read-manifest is making that explicit, as I mentioned in https://pagure.io/fedora-rust/rust2rpm/issue/186#comment-794897.

Even in the basic case without these conditional sub-features, that metadata should be easier now. For a simple optional serde, 1.59.0 used to give empty "features":{}, and you'd have to figure out that the optional dependency also created a feature. Since 1.60.0, this is always spelled out like "features":{"serde":["dep:serde"]}.

Ok, that's good to know. But we still need to deal with dep:foo style dependencies, which right now, break both the BuildRequires and Requires generators.

What is the state of this issue? Has anyone started to work on this? This comment mentions something but I am not sure if he had time to continue with the development since then. I would like to help resolve this issue.

I have been working on this here:
https://pagure.io/fedora-rust/cargo2rpm

The SemVer parser is done, the feature resolver is done (I hope), and today I have worked on the dependency resolver. I hope to have a prototype ready for testing in the next few days.

Oh perfect, thanks for the work! Let me know if I can help with anything!

This syntax should now be fully supported by our tools as of rust-packaging v24 / rust2rpm v24. It is available in rawhide.

Metadata Update from @decathorpe:
- Issue close_status updated to: Fixed
- Issue set to the milestone: None (was: 24)
- Issue status updated to: Closed (was: Open)

2 years ago

Log in to comment on this ticket.

Metadata