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

Add the ability to exclude experimental features from consideration #315

Closed
nmathewson opened this issue Jan 23, 2023 · 10 comments
Closed
Labels
C-enhancement Category: raise the bar on expectations

Comments

@nmathewson
Copy link
Contributor

Describe your use case

In our project, we have a development process where some parts of our API are explicitly labeled as "experimental, and not covered by semver." Our expectation is that eventually, these parts become stable, or are removed.

We put these experimental APIs behind one or more features, each explicitly labelled as experimental in our documentation. To replace most usage of --all-features, we provide a full feature that includes all non-experimental features.

As it stands today, this setup doesn't seem compatible with cargo-semver-checks, since cargo-semver-checks always uses --all-features (that is, if I understand correctly).

Describe the solution you'd like

The easiest solution for us would be a command-line option to cause cargo-semver-checks to replace the default --all-features. Possibly something like --not-all-features --features=<explict,list> would be workable?

Alternatives, if applicable

No response

Additional Context

No response

@nmathewson nmathewson added the C-enhancement Category: raise the bar on expectations label Jan 23, 2023
@ijackson
Copy link

--features wouldn't make sense without --not-all-features so maybe the latter is redundant.

@epage
Copy link
Collaborator

epage commented Jan 23, 2023

The ideal long term solution for this is rust-lang/cargo#10881 and respecting it.

@obi1kenobi
Copy link
Owner

Thanks for raising this! It's definitely a use case that should be supported. I have a suggested workaround, and a broader comment about the topic of "this code is public but not stable."

The workaround is this: cargo-semver-checks allows explicitly specifying the rustdoc JSON files to use as the current and the baseline versions, via the --current-rustdoc and --baseline-rustdoc options. Until cargo-semver-checks can implement something like you suggested, you can work around it by generating the rustdoc with only the stable features of your crate and only using cargo-semver-checks to compare the files you've already generated. This is less ergonomic than cargo-semver-checks doing it automatically, but will accomplish the same goal.

My broader comment:

An unfortunate difficulty is that the Rust ecosystem as a whole hasn't settled on the way to signal to downstream consumers that "this bunch of code is public but normal stability guarantees don't apply." It affects more than just not-yet-stable code. For example, another place where this comes up is macros: macro-generated code may use portions of the public API that aren't meant to be used directly, and aren't themselves stable except as used via the macros.

Some authors feel that #[doc(hidden)] implies semver-instability. Other crates add experimental features with names prefixed with unstable. Yet another option would be having explicit clippy-style attributes that allow explicitly marking items as semver-incompliant either in general or in specific ways (for example: "this pub method is experimental and it may gain or lose function parameters, but its return type will always be Self").

We'd need to do a decent bit of research to find the best design here, since cargo-semver-checks is going to be integrated into cargo and therefore our choices would have a significant ecosystem-wide impact. I'm definitely in favor of doing this work, but unfortunately due to its large scope, it's not something we have the bandwidth for at this point in the project. I hope that we're able to attract more contributors over time and get to it as soon as possible.

@epage
Copy link
Collaborator

epage commented Jan 23, 2023

Some authors feel that #[doc(hidden)] implies semver-instability. Other crates add experimental features with names prefixed with unstable.

Just wanted to call out that these are two different use cases, generally

  • #[doc(hidden)]: used internally by macros or associated types (associated type example)
  • features: the API items are meant for end-users but aren't stable yet

@obi1kenobi
Copy link
Owner

This is resolved by #426 and will be part of the next release, sometime in the next week or so — just need to finish up the docs for the new CLI flags: #458

@obi1kenobi
Copy link
Owner

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!

@nmathewson
Copy link
Contributor Author

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!

Thanks! We'll put out our releases at the start of each month, so we'll be trying this code out some more as the day approaches. For your amusement, here is the workaround we have been using up until now: https://gitlab.torproject.org/tpo/core/arti/-/blob/main/maint/semver-checks

@obi1kenobi
Copy link
Owner

Nice workaround! cargo-semver-checks does effectively the same thing internally now, with the added ability to cache baseline rustdoc JSON files if they correspond to a known crate version. Depending on how you use it, that might also give you a nice speed boost since as the comments show you've already noticed, rustdoc generation is one of the noticeably slow parts of the process now.

@nmathewson
Copy link
Contributor Author

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

Here's how it went: we managed to drop the entire old script, and replace it with:

cargo semver-checks \
      --only-explicit-features \
      --workspace \
      --features full \
      --baseline-rev "$LAST_VERSION"

Everything worked swimmingly and went much faster, and the outputs were much nicer.

I've opened issues for everything we ran into that might be fixed in the future:

Thanks again for working on this!

@obi1kenobi
Copy link
Owner

Awesome, that's great to hear!

I also appreciate you opening issues and offering to help, we definitely could use the help and I'm happy to point you in the right direction. Feel free to ping me anytime with questions. I'll put some pointers in each of the issues you mentioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: raise the bar on expectations
Projects
None yet
Development

No branches or pull requests

4 participants