slint-ui / document-features

Extract documentation for the feature flags from comments in Cargo.toml
Apache License 2.0
173 stars 13 forks source link

Building fails with vendored dependencies #20

Closed v-morlock closed 10 months ago

v-morlock commented 1 year ago

Hi, when building a crate which depends on this crate to generate its docs, building fails when using "vendored" dependencies, as cargo will normalize the Cargo.toml and won't create Cargo.toml.orig. Is there anything i can do about that?

ogoffart commented 1 year ago

What is the exact error?

Perhaps when no documentation is found, we could chose to either ignore it, or try to emit a warning. Or generate a text explaining the documentation were not found.

The mitigation is to make the crate optional as explained https://docs.rs/document-features/latest/document_features/#compatibility and to not enable this when building the docs as vendored

oscartbeaumont commented 1 year ago

Yeah, I think this is my fault for not correctly setting the dependency as optional in Specta as specified in the docs. Must have just missed it. 🤦

ErichDonGubler commented 10 months ago

The proposed mitigation to gate document-feature behind a feature works to resolve this issue, assuming that one has control over their dependencies or the cycles to fork a potentially deep dependency tree. Even with full control, however, this can still be a particularly time-consuming problem to solve, as the compilation failure can happen at a large distance between the crate depending on document-features and the workspace in which crates are being vendored in (see also RainbowCookie32/rusty-psn#172). We in Firefox ran into this issue while cargo vendor-ing wgpu:090f2f757c2d21a36c3bec1c0f43dc56aa9b9dd3, which expects to run on Rust 1.70 or higher. This assumption of minimum Rust version would seem to render the current Compatibility section's contents irrelevant for most readers. For convenience, the contents state (paraphrased):

The minimum Rust version required to use this crate is Rust 1.54 because of the feature to have macro in doc comments. You can make this crate optional and use #[cfg_attr()] statements to enable it only when building the documentation: You need to have two levels of cfg_attr because Rust < 1.54 doesn’t parse the attribute otherwise.

I think, for now, the best thing to do is to remove the error case entirely, and accept an empty set of features.

ErichDonGubler commented 10 months ago

One might reasonably suggest using a warning instead of an error in this case. Unfortunately, warning-level diagnostics in procedural macros are gated behind rust-lang/rust#54140 as Experimental portions of the proc_macro::Diagnostic API. For now, this is not an option.

ErichDonGubler commented 10 months ago

Just filed a Cargo issue to see if we might be able to resolve this upstream: https://github.com/rust-lang/cargo/issues/13191

ErichDonGubler commented 10 months ago

Ed Page from the Cargo team recommends that fallback to Cargo.toml.orig be attempted when no features are found to be documented. This file is emitted when cargo package and cargo vendor "normalize" files and lose commentary in the way that motivates the OP issue: https://github.com/rust-lang/cargo/issues/13191#issuecomment-1866885363

Ed also points to https://github.com/rust-lang/cargo/issues/4956 as a place for interesting discussion, where there is a proposal to add TOML fields for feature descriptions.

ogoffart commented 10 months ago

the crate already fallback to the .toml.orig file. In your case, that file probably does not exist.

I've made it an error because I thought someone using the crate without any feature would mean something is wrong and the programmer should be notified (eg, parse error on our misuse of the crate). But in this case, I realize this is too strong. You're right that this should be a warning instead, or just generate some documentation text saying no features was found.

ogoffart commented 10 months ago

Fixed in 0.2.8