RFC: New crates packaging design (features have their own subpackages)

Project Discussion
1

TL;DR. Right now we are creating just one -devel subpackage for crate which contains Requires for all dependencies (including optional ones). The easiest way to solve this problem was to use “conditional Provides”, however they are not implemented and won’t be implemented. So the original proposal for solving this problem stays – split features into their subpackages and generate requires from there. Below goes the proposal for new packaging way.

Let’s take some example – uuid v0.7.1.

Cargo.toml 
[dependencies]
byteorder = { version = "1", default-features = false, features = ["i128"], optional = true }
md5 = { version = "0.3", optional = true }
rand = { version = "0.5", optional = true }
serde = { version = "1.0.56", default-features = false, optional = true }
sha1 = { version = "0.6", optional = true }
slog = { version = "2", optional = true }

[features]
default = ["std"]
std = []
v1 = []
v3 = ["md5", "rand"]
v4 = ["rand"]
v5 = ["sha1", "rand"]

# since rust 1.26.0
u128 = ["byteorder"]

# nightly rust
#------------------------
# Allow using `const fn`s
const_fn = ["nightly"]
# Nightly marker feature gate
nightly = []

What we generate now:

rust-uuid.spec 
%package        devel
Summary:        %{summary}
BuildArch:      noarch
Provides:       crate(uuid) = 0.7.1
Provides:       crate(uuid/byteorder) = 0.7.1
Provides:       crate(uuid/md5) = 0.7.1
Provides:       crate(uuid/rand) = 0.7.1
Provides:       crate(uuid/serde) = 0.7.1
Provides:       crate(uuid/sha1) = 0.7.1
Provides:       crate(uuid/slog) = 0.7.1
Provides:       crate(uuid/const_fn) = 0.7.1
Provides:       crate(uuid/default) = 0.7.1
Provides:       crate(uuid/nightly) = 0.7.1
Provides:       crate(uuid/std) = 0.7.1
Provides:       crate(uuid/u128) = 0.7.1
Provides:       crate(uuid/v1) = 0.7.1
Provides:       crate(uuid/v3) = 0.7.1
Provides:       crate(uuid/v4) = 0.7.1
Provides:       crate(uuid/v5) = 0.7.1
Requires:       cargo
Requires:       ((crate(byteorder) >= 1.0.0 with crate(byteorder) < 2.0.0) with crate(byteorder/i128))
Requires:       (crate(md5) >= 0.3.0 with crate(md5) < 0.4.0)
Requires:       (crate(rand) >= 0.5.0 with crate(rand) < 0.6.0)
Requires:       (crate(serde) >= 1.0.56 with crate(serde) < 2.0.0)
Requires:       (crate(sha1) >= 0.6.0 with crate(sha1) < 0.7.0)
Requires:       (crate(slog) >= 2.0.0 with crate(slog) < 3.0.0)

%files          devel
%{cargo_registry}/%{crate}-%{version}/

Notice that all dependencies in upstream are optional and none of them is in “default feature set”? And we basically pull them in in all buildroots when uuid is requested. And they have a lot of their own dependencies (we even have to filter out slog dependency entirely because we didn’t manage to package it). And even if did, we would forget that we filtered it out so it would rot there.

Now let’s look what I would like to propose:

rust-uuid.spec 
%package      -n rust-uuid-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid) = 0.7.1

%files        -n rust-uuid-devel
%{cargo_registry}/%{crate}-%{version}/

%package      -n rust-uuid+byteorder-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/byteorder) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(byteorder/i128) >= 1.0.0 with crate(byteorder/i128) < 2.0.0)

%files        -n rust-uuid+byteorder-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+const_fn-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/const_fn) = 0.7.1
Requires:        (crate(uuid) = 0.7.1 and crate(uuid/nightly) = 0.7.1)

%files        -n rust-uuid+const_fn-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+default-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/default) = 0.7.1
Requires:        (crate(uuid) = 0.7.1 and crate(uuid/std) = 0.7.1)

%files        -n rust-uuid+default-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+md5-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/md5) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(md5/default) >= 0.3.0 with crate(md5/default) < 0.4.0)

%files        -n rust-uuid+md5-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+nightly-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/nightly) = 0.7.1
Requires:        crate(uuid) = 0.7.1

%files        -n rust-uuid+nightly-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+rand-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/rand) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(rand/default) >= 0.5.0 with crate(rand/default) < 0.6.0)

%files        -n rust-uuid+rand-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+serde-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/serde) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(serde) >= 1.0.56 with crate(serde) < 2.0.0)

%files        -n rust-uuid+serde-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+sha1-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/sha1) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(sha1/default) >= 0.6.0 with crate(sha1/default) < 0.7.0)

%files        -n rust-uuid+sha1-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+slog-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/slog) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(slog/default) >= 2.0.0 with crate(slog/default) < 3.0.0)

%files        -n rust-uuid+slog-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+std-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/std) = 0.7.1
Requires:        crate(uuid) = 0.7.1

%files        -n rust-uuid+std-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+u128-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/u128) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(byteorder/i128) >= 1.0.0 with crate(byteorder/i128) < 2.0.0)

%files        -n rust-uuid+u128-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+v1-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/v1) = 0.7.1
Requires:        crate(uuid) = 0.7.1

%files        -n rust-uuid+v1-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+v3-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/v3) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(md5/default) >= 0.3.0 with crate(md5/default) < 0.4.0)
Requires:        (crate(rand/default) >= 0.5.0 with crate(rand/default) < 0.6.0)

%files        -n rust-uuid+v3-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+v4-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/v4) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(rand/default) >= 0.5.0 with crate(rand/default) < 0.6.0)

%files        -n rust-uuid+v4-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

%package      -n rust-uuid+v5-devel
Summary:         %{summary}
BuildArch:       noarch
Provides:        crate(uuid/v5) = 0.7.1
Requires:        crate(uuid) = 0.7.1
Requires:        (crate(sha1/default) >= 0.6.0 with crate(sha1/default) < 0.7.0)
Requires:        (crate(rand/default) >= 0.5.0 with crate(rand/default) < 0.6.0)

%files        -n rust-uuid+v5-devel
%ghost %{cargo_registry}/%{crate}-%{version}/Cargo.toml

Obviously the %files part can be optimized, so ignore it for now. It uses %ghost to get requires generated, but basically it is empty subpackage :slight_smile:

P.S. I put “expanded” version of spec files in the text for visibility, in reality those are generated automatically.

What do you think about this approach? Any suggestions and comments are very appreciated.

\cc @ngompa @jistone @eclipseo

1 Like
2

I am not sure what process would require such granularity? What does it change to depend on crate(uuid/byteorder) instead of crate(uuid)? You pull the whole uuid as a dependency anyway.
And in the end you still need to package all the optional dependencies to achieve this, so I don’t get the need for the overhead?
Am I missing something here? Or the goal is only to lower the number of pulled dependencies?

3

Depending on crate(uuid/byteorder) will bring not only this crate into the buildroot, but also byteorder crate with feature i128 compared to crate(uuid). So lower number of packages pulled in buildroot is one of the goals.

Right now if package is too complex – we just filter out the feature. With new concept, we would not filter anything, it’s just package would have broken dependencies. But probably nobody is going to use it, and if they do – they get some meaningful error like rust-uuid+byteorder-devel depends on crate(byteorder/i128), but nothing provides it, not something like nothing provides crate(uuid/byteorder).

There is also the case with bootstrapping (I can’t find which crate was that so I will use some generic names): foo depends on bar through feature while bar depends on foo directly. In this case, I had to drop feature from foo, build it, then build bar and then bring feature back and rebuild foo. If we would have that feature in the subpackage, I would build foo, subpackage would have broken dependencies (but no one cares), then I would build bar which would fix dependencies.

4

I guess I view packaging a Rust package as packaging all the available options and deps rather than providing only parts of the features, leaving things incomplete. That’s what I would do even with this.

5

The question is at which point of time you want that :wink: Also when you put such stuff into a module, that complicates things a lot, because you don’t include 5-10 packages but you have to include 50 just because you put everything into the one place.

Also I agree that we definitely want to package all dependencies, but sometimes it takes quite some time and I would like to move forward with incomplete deps rather than blocking everything on this.

6

The main issue with this is that Fedora’s tooling is centered heavily around not having broken dependencies, so deliberately introducing subpackages with broken dependencies is not likely to go over well. Plus, since I fixed spam-o-matic, we’ll get spammed about broken dependencies in packages we control as soon as emails are being sent again…

1 Like
7

Well, in ideal world we would just package missing stuff and be done with it. But in Fedora world we could just package all crates in a module and exclude packages with broken dependencies so that they don’t get shipped. Or just leave them as-is, because there is no spam-o-matic for modules :stuck_out_tongue:

8

There’s a middle ground where we do make subpackages for supported features, but still have a policy of removing features that can’t be supported. This way you still get your minimized buildroot, but we do have to be mindful of patching Cargo.toml. For bootstrapping issues, I think it would be OK to let such features be temporarily broken while other things get updated, but stuff we don’t plan on supporting should be removed.

9

Should we patch out Cargo.toml to remove features or we just remove subpackage and be done with it?

10

Until we have --minimal-lock-file, we have to continue patching Cargo.toml, no? And actually, that’s probably true for bootstrapping issues too.

11

For things like nightly we won’t. For optional features where we don’t have dependency, yes. Until we have --minimal-lock-file we have to patch it out.

12

So here is the pull request!

It is not tested yet and tests are failing, but I will eventually fix them.