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

cfg_attr(docsrs) feature hints do not propagate to in kube for re-exports #1242

Open
clux opened this issue Jul 4, 2023 · 3 comments
Open
Labels
bug Something isn't working docs unclear documentation question Direction unclear; possibly a bug, possibly could be improved.

Comments

@clux
Copy link
Member

clux commented Jul 4, 2023

Current and expected behavior

Currently we set up some very nice doc annotations for our optional features such as ws:

#[cfg_attr(docsrs, doc(cfg(feature = "ws")))]

These show up perfectly on docs.rs/kube-client

Available on crate features client and ws only.

However, on the facade crate docs.rs/kube it's less good.

Available on crate feature client only.

This leads people to be confused because they are browsing docs.rs/kube and are being slightly lied to about feature choices as seen from continued confusion in comments from bugs like: #1159 (comment)

The problem arises because the ws feature flag is basically a kube-client and kube-core feature, and it's just propagated down into those crates. kube just re-exports everything it can find:

macro_rules! cfg_client {
($($item:item)*) => {
$(
#[cfg_attr(docsrs, doc(cfg(feature = "client")))]
#[cfg(feature = "client")]
$item
)*
}
}
macro_rules! cfg_config {
($($item:item)*) => {
$(
#[cfg_attr(docsrs, doc(cfg(feature = "config")))]
#[cfg(feature = "config")]
$item
)*
}
}
macro_rules! cfg_error {
($($item:item)*) => {
$(
#[cfg_attr(docsrs, doc(cfg(any(feature = "config", feature = "client"))))]
#[cfg(any(feature = "config", feature = "client"))]
$item
)*
}
}
cfg_client! {
pub mod api;
pub mod discovery;
pub mod client;
#[doc(inline)]
pub use api::Api;
#[doc(inline)]
pub use client::Client;
#[doc(inline)]
pub use discovery::Discovery;
}

Possible solution

Not sure exactly the best way forward here.

We could maybe try to explicitly namespace ws stuff better, but this is pretty insidious, and hard to retrofit without breaking a lot.

We could also perhaps consider importing the crates into a module workspace instead (merge the crates in one big crate). The need for having a multi-crate split is smaller these days with a locked versioning and re-exported features.

We could also just wait and hope docsrs gets better... It's not great that it doesn't detect this across crate boundaries, but it's also understandable, and probably hard. EDIT: yes. Open bug upstream rust-lang/rust#88743 . In a similar upstream bug, they do describe why it's hard: rust-lang/rust#83428 (comment)

Environment

docs.rs

@clux clux added bug Something isn't working docs unclear documentation question Direction unclear; possibly a bug, possibly could be improved. labels Jul 4, 2023
@clux clux changed the title cfg_attr docsrs annotations do not work for facade crate re-exports cfg_attr docsrs annotations do not work for crate re-exports Jul 7, 2023
@clux clux changed the title cfg_attr docsrs annotations do not work for crate re-exports cfg_attr(docsrs) feature hints do not propagate to in kube for re-exports Jul 7, 2023
@nightkr
Copy link
Member

nightkr commented Jul 7, 2023

The need for having a multi-crate split is smaller these days with a locked versioning and re-exported features.

One big thing (for me) is that feature flags aren't typechecked/isolated (for lack of a better word). Each combination of feature flags needs to be compiled/tested separately to make sure that it doesn't bitrot, while crates are isolated from each other by nature.

@clux
Copy link
Member Author

clux commented Aug 24, 2023

https://blog.rust-lang.org/2023/08/24/Rust-1.72.0.html#rust-reports-potentially-useful-cfg-disabled-items-in-errors might make the re-export doc failures a bit better, haven't tested it yet.

@clux
Copy link
Member Author

clux commented Aug 24, 2023

It is indeed much better now on 1.72. A way to test is to take out the default ws feature on examples/Cargo.toml (which emulates a user environment) then running cargo run --example pod_exec which now gives:

error[E0432]: unresolved imports `kube::api::AttachParams`, `kube::api::AttachedProcess`
  --> examples/pod_exec.rs:7:14
   |
7  |         Api, AttachParams, AttachedProcess, DeleteParams, PostParams, ResourceExt, WatchEvent, WatchParams,
   |              ^^^^^^^^^^^^  ^^^^^^^^^^^^^^^ no `AttachedProcess` in `api`
   |              |
   |              no `AttachParams` in `api`
   |              help: a similar name exists in the module: `PatchParams`
   |
note: found an item that was configured out
  --> /Users/clux/kube/kube/kube-client/src/api/mod.rs:14:31
   |
14 | pub use subresource::{Attach, AttachParams, Execute, Portforward};
   |                               ^^^^^^^^^^^^
   = note: the item is gated behind the `ws` feature
note: found an item that was configured out
  --> /Users/clux/kube/kube/kube-client/src/api/mod.rs:7:49
   |
7  | #[cfg(feature = "ws")] pub use remote_command::{AttachedProcess, TerminalSize};
   |                                                 ^^^^^^^^^^^^^^^
   = note: the item is gated behind the `ws` feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docs unclear documentation question Direction unclear; possibly a bug, possibly could be improved.
Projects
None yet
Development

No branches or pull requests

2 participants