Enable specifying features for current and baseline

[Emilgardis]

Describe your use case

I have a crate which has a feature-flag unsupported which does not guarantee semver compatibility. This might not be the best contract but there's no way to make cargo semver-checks check-release not enable --all-features.

Describe the solution you'd like

cargo semver-checks check-release --exclude-feature unsupported

this would enable all features, except unsupported

Alternatives, if applicable

cargo semver-checks check-release --features foo,bar,new_feature --baseline-features foo,bar

could also work, but that would cause problems in CI as you wouldn't be able to easily specify what features are new or not. E.g in the example, new_feature is not in the baseline, but is part of the api

Additional Context

No response

[obi1kenobi]

Hi! Thanks for checking out cargo-semver-checks and for suggesting this new feature.

It seems like there's a broader need for flexibility around semver-checking. Here are a few of the other use cases I know of so far:

• increasing MSRV is technically semver-major but many crates consider it requiring only a minor version bump
• macros sometimes use code that must be made public (since it's used in macro-generated code which resides outside the macro's own crate) but which should not be used directly by external crates and therefore could be exempted from semver-checking

I think it's worth trying to solve for the more general theme here rather than trying to solve individual use cases separately (MSRV, macros, "unstable" features, etc.). One way to do that would be through a mechanism similar to clippy lints, where altered lint behaviors are applied to specific portions of the code or manifest: "MSRV is minor not major" / "don't semver-check this module" etc. The tracking issue for that is #58.

What do you think? Is that a reasonable approach that would address your project's needs as well?

[Emilgardis]

aha! a

#[semver_checks::ignore]
pub mod unsupported;

#[non_exhaustive]
pub enum Foo {
   StableFoo,
   #[semver_checks::ignore]
   #[cfg(feature = "unstable")]
   NewFoo,
}

would be interesting, but it would mean I'd have to sprinkle that in a few places. There's also an issue when the feature is not additive (even though features are supposed to be additive).

I'm satisfied with a mechanism like that however

[obi1kenobi]

Since a goal of cargo-semver-checks is to be merged into cargo itself (#61), I don't think we'll be able to support non-additive features since cargo also assumes that features are additive. This is in the same sense as cargo not supporting them: they might happen to work, or they might not, and it isn't a bug if they don't.

Do you feel like this proposed mechanism would satisfy the need described in the issue? If so, is it safe to close this issue in favor of #58 as the tracking issue for that mechanism?

[Emilgardis]

It does :) Thanks!

[Emilgardis]

Hmm, been thinking about this some more recently. The thing I actually want is to check if a specific set of features are still semver compatible. i.e have I accidentally introduced new behaviour when only feature foo_a is enabled.

Has this concern been captured anywhere @obi1kenobi ?

[obi1kenobi]

@tonowak and I were recently discussing this as a possible near-future use case. Right now, cargo-semver-checks only checks --all-features and only on the current platform where it's being executed. Users can check another platform's semver compliance by using that platform's toolchain, but no such option exists for features — and it should.

I'm fine with reopening this issue to track the ability to specify features explicitly. It's been tangentially mentioned elsewhere, but this seems like a good issue specifically for that.

[obi1kenobi]

One possible option:

• Instead of always checking --all-features, check the union of default features + features that are not prefixed with _. This allows internal/unstable/experimental/test-only features to be omitted in cargo-semver-checks' default run configuration.
• Then, allow features to be specified explicitly. This disables default features, and only enables the specified features — and can also include _-prefixed features. This way maintainers can check specific feature combinations.

[Tastaturtaste]

I want to chime in because I am also facing a problem with the --all-features flag.

I am currently working on matlab-sys, which exposes bindings to the native C interface of Matlab. This C interface can be used in two slightly different, incompatible, versions, which would get toggled by a -DFLAG in C. Following this advice for building *-sys crates and without a better idea, I translated these as cargo features. I know this is not how features are supposed to be used, but with C-bindings some things just are not purely additive.

I don't know how much work it would be to allow specifying the features when running semver-checks, but it would be really helpful for me. In the simplest case you could require the user to specify all flags manually as soon as at least one is specified and then just pass them verbatim to rustdoc I suppose?

[obi1kenobi]

Thanks for sharing your use case, this is very helpful when designing a complex feature like this. I had a couple of follow-up questions:

This C interface can be used in two slightly different, incompatible, versions, which would get toggled by a -DFLAG in C.

Interesting! So I'm guessing if features could be specified here, you'd just run cargo-semver-checks twice — once with each feature enabled?

with C-bindings some things just are not purely additive.

Out of curiosity, what happens if users specify both features? What happens if they specify neither feature?

Since the resolver will enable all requested features but compile only once per major version, it's even possible to have a "diamond dependency" like this:

     A
    / \
   B   C
    \ /
matlab-sys

In this case, if B wants one of the features and C wants the other, users of A end up with both matlab-sys features enabled. By the sound of it, it seems like that wouldn't work — and hopefully break at compile time and not at runtime?

Might it be better to split the matlab-sys crate along the present-day feature boundary into two crates, or two major versions of the same crate? Might that produce better outcomes for users than the current approach?

[Tastaturtaste]

Interesting! So I'm guessing if features could be specified here, you'd just run cargo-semver-checks twice — once with each feature enabled?

Yes, in my case I would do just that.

Out of curiosity, what happens if users specify both features? What happens if they specify neither feature?

C extensions using the C-API are intended to be compiled from inside Matlab with the mex command. This command sets one of both features, specifying both is an error. Upon your question I discovered it is possible to build C extensions outside of Matlab and manually define the flags of both features, but I don't think that's a good idea. If neither feature is specified the legacy feature is picked automatically.

For my crate I have inserted a manual compile_error! if both features are enabled, because that matches the use of the mex command from within Matlab as close as possible. If neither feature is picked I have set a default as well.

The main difference between the two versions comes down to how arrays or matrices of complex numbers are handled and stored. For one feature they are managed as two different arrays, one for the real parts and one for the imaginary parts. For the other feature they are interleaved, so one array with real and complex parts in alternating order. Syntactically it would be possible to expose both features simultaneously I think, at least it works for the C-API as long as one compiles the code manually to circumvent the checks from the mex command.

But I really don't think anyone should both versions at the same time. There are functions (mxGetData) with the exact same signature in both versions which return a void * to the underlying storage. For the separate-complex case it points to just the real values of the complex numbers, for the interleaved-complex case it points to the array of interleaved complex numbers. It would be possible to expose the whole API of both features at once, but I don't know how this function would behave in that case. I guess it would depend on the array handle passed into the function and which version of some API function created the handle...

In this case, if B wants one of the features and C wants the other, users of A end up with both matlab-sys features enabled. By the sound of it, it seems like that wouldn't work — and hopefully break at compile time and not at runtime?

Yes, that could happen and as explained above I don't think mixing them is a good idea, which is why I check for that at compile time. I think that is preferable to the mess I explained in the paragraph above.

Might it be better to split the matlab-sys crate along the present-day feature boundary into two crates, or two major versions of the same crate? Might that produce better outcomes for users than the current approach?

In the normal workflow with C and the mex command it is possible to switch between both API versions with a simple command line flag and mostly reuse the code while not being able to compile with both API versions enabled at once. I wanted to emulate this usage pattern and really didn't and still don't expect people to actively want to enable and use both features at the same time intermixed. But you are right that there might be use-cases where one intermediate dependency uses one feature internally and another intermediate dependency the other internally, which should be fine and would not work with my current setup. I have to think about it I guess.

Sorry for this long response with details that are not really relevant to this issue, your questions definitely gave me food for thought though. I still think being able to specify features at the command line would be nice to have and in my totally uninformed and naive imagination not too difficult to implement.

[obi1kenobi]

Thanks for the context, it was definitely relevant. To build the best tool possible, it's useful for me to understand as many real-world use cases as possible — and the devil is always in the details. Much appreciated!

[obi1kenobi]

v0.21 was just released and allows specifying the exact set of features to use, both by feature group (e.g. only default features) as well as at the level of individual features.

Thanks for your patience! Would love to hear how it went if you get a chance to try it!