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

Rust Documentation: feature flag requirements not showing on docs.rs? #6123

Closed
n8henrie opened this issue Jan 8, 2023 · 8 comments · Fixed by #6146 or #6201
Closed

Rust Documentation: feature flag requirements not showing on docs.rs? #6123

n8henrie opened this issue Jan 8, 2023 · 8 comments · Fixed by #6146 or #6201

Comments

@n8henrie
Copy link
Contributor

n8henrie commented Jan 8, 2023

tldr: feature flag requirements (e.g. .diff() requires the diff feature) are showing up when I build docs locally and on GitHub Pages but not on docs.rs for some reason.

polars (rust) has a lot of feature flags, and I was having a little difficulty* figuring out which ones I was missing to get things to compile in polars-rust that I had working in polars-python. This led me to start a conversation on Reddit which helped me realize that:

I was wondering if there could perhaps be some kind of configuration on docs.rs or rustdoc that enables this, and was able to find:

Taking a look at your GA workflow, it builds locally with the feature flag requirements displayed: RUSTFLAGS="--cfg docsrs" cargo doc --features=docs-selection --package polars --open.

Comparing to axum, I don't see huge differences for why their feature requirements are being displayed on docs.rs and polars is not, except that they are using all-features = true where as polars uses features = ["docs-selection"] in Cargo.toml; building locally still shows the feature flag requirements, so that doesn't seem to be the issue.

Any ideas why the feature flag requirements aren't showing up on docs.rs?

* in spite of great documentation such as https://pola-rs.github.io/polars/polars/index.html#compile-times-and-opt-in-features

@ritchie46
Copy link
Member

Hmm.. i recall I tried this a while ago and bounced. Any help on making this working would be great.

@n8henrie
Copy link
Contributor Author

n8henrie commented Jan 8, 2023

Sweet. While I'm a novice in both, I love rust and love pandas, so I've been hoping for an opportunity to contribute to polars!

n8henrie added a commit to n8henrie/polars that referenced this issue Jan 10, 2023
This makes it so one doesn't need to manually configure the
documentation cfg for feature flags.

rust-lang/rust#90502

Fixes pola-rs#6123
@n8henrie
Copy link
Contributor Author

n8henrie commented Jan 10, 2023

So I don't see a whole lot that's super obvious.

I did find that the feature flag warnings were failing locally for a while across a number of local builds; I had to cargo clean to get them to work, so I wonder a little about the caching.

I also found that the newish doc_auto_cfg feature might save a fair bit of work, so I ran a few sed scripts across the repo and put it to use (and removed the now unnecessary #[cfg_attr(docsrs, doc(cfg(feature = "foo")))] everywhere).

I was having better luck creating the docs with feature flags using RUSTDOCFLAGS as compared to RUSTFLAGS, so I made that change in the GitHub Actions.

Here is a little test crate POC showing it working on docs.rs:
https://docs.rs/n8henrie/0.1.4/n8henrie/struct.TestDocsFeatures.html

I don't know of a way to "test run" the changes on docs.rs (something like pypi-test for python, but for crates -> docs.rs). Does anyone know of something like this, or a way to test a change that has been already working locally, just not on docs.rs?

EDIT: From my local build (target/doc/polars/prelude/enum.Expr.html#method.diff):

Screenshot 2023-01-09 at 17 28 59

@ritchie46
Copy link
Member

Cool stuff! :)

I was having better luck creating the docs with feature flags using RUSTDOCFLAGS as compared to RUSTFLAGS, so I made that change in the GitHub Actions.

Could you explain me a bit what the difference is between those? When should we use one vs the other?

I don't know of a way to "test run" the changes on docs.rs (something like pypi-test for python, but for crates -> docs.rs). Does anyone know of something like this, or a way to test a change that has been already working locally, just not on docs.rs?

Not that I am aware of. :( I often have to do a second release because the docsrs has an unexpected failure not catched by our CI.

@n8henrie
Copy link
Contributor Author

@ritchie46 in light of #6164 can we reopen this issue?

I assume this was the GA run? Looks like it was from 1 minute prior to the revert commit.

When I download that artifact and open polars/prelude/enum.Expr.html#method.diff, it looks to be working:

Screenshot 2023-01-11 at 05 23 27

Perhaps this is the wrong artifact?

@ritchie46 ritchie46 reopened this Jan 11, 2023
@n8henrie
Copy link
Contributor Author

Well, I got my fork building the docs https://n8henrie.com/polars/polars/prelude/enum.Expr.html#method.diff so I should have better luck replicating at least the GitHub Pages issues now.

While I'm looking at this, would you have any interest in simplifying the GitHub Pages deployment (to remove the external dependency on ghp-import and rely only on official GitHub Actions steps)? If so I'd be happy to get this working on my fork and create a separate PR. Here's the GitHub Pages deployment workflow for an example project of mine: https://github.com/n8henrie/simplenet/blob/master/.github/workflows/sphinx.yml

Obviously would need a little translation from Python -> Rust, but I'm fairly confident that I could just swap out the upload path and you could end up with something perhaps a bit simpler and hopefully better supported.

No immediate benefits I can think of though, since your current system seems to get the job done.

@n8henrie
Copy link
Contributor Author

Reverting back to the prior commit seems to be working fine at my fork: https://n8henrie.com/polars/polars/prelude/enum.Expr.html#method.diff

Let's not merge quite yet to see if we can figure out why it didn't work last time it was merged.

@n8henrie
Copy link
Contributor Author

n8henrie commented Jan 16, 2023

Could you explain me a bit what the difference is between those? When should we use one vs the other?

I wish I was finding better information; a few issues:

What I know for sure is that on my fork I am reliably getting the feature flag warnings to build when using RUSTDOCFLAGS but not with RUSTFLAGS.

Same with this like POC, cargo new --lib foo (and a runner):

src/lib.rs:

#![cfg_attr(docsrs, feature(doc_auto_cfg))]

#[cfg(feature = "bar")]
pub fn bar() {
    println!("Hello, world!");
}

docs.sh:

#!/usr/bin/env bash

set -Eeuf -o pipefail

main() {
  cargo clean
  RUSTFLAGS='--cfg docsrs' cargo +nightly doc --features=bar --open
  grep -rF 'crate feature' target/doc/foo || {
    echo "Docs didn't work!"
    exit 1
  }
}
main "$@"

In other news, I wonder if this is what is causing the docs.rs issue: NM, you're already doing that right ¯\_(ツ)_/¯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants