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

Cargo doc does not pass --cfg doc for build scripts #8944

Closed
jnqnfe opened this issue Dec 5, 2020 · 5 comments
Closed

Cargo doc does not pass --cfg doc for build scripts #8944

jnqnfe opened this issue Dec 5, 2020 · 5 comments
Labels
A-build-scripts Area: build.rs scripts C-bug Category: bug Command-doc

Comments

@jnqnfe
Copy link

jnqnfe commented Dec 5, 2020

The issue of needing to be able to disable things like pkg-config checks in build scripts for FFI -sys crates has been raised previously (for example this or this). We now have cfg(doc) as of Rust 1.41, however it seems that this is not used when running build scripts. Why? Please could this be changed/fixed?

I have just successfully converted some of my crates to use cfg(doc) outside of the build script, from previously using the dox feature technique. Wanting to rid my crates now of the dox feature I tried fixing the last remaining use of it - the build script. Unfortunately I have found that cfg(doc) does not work there, nor does CARGO_CFG_DOC. Looking at the output from cargo doc -vv it is clear that cargo is not signalling doc mode to rustc when executing the build script.

I am aware of the rustc-args key of docs.rs metadata, and I believe that setting it to ["--cfg", "doc"] may work around this problem for docs.rs, allowing me to ditch my dox feature.

However, (1) why am I having to implement such a workaround, why is cargo not passing it along itself? (2) although this helps in the docs.rs case, it does not enable users generating local documentation to get to do so without the presence of the system library, and it would be nice to allow them to do so, because why have such a restriction.

I could leave the dox feature in place to address point 2, but it only helps for direct generation of crate docs, not so much when it's a dependency of something else. Additionally, it complicates things compared to cargo just passing --cfg doc along, and I don't really want the messy cruft of keeping dox around.

I'm using the Debian Sid cargo package, which is currently version 1.42.1 (0.43.1). I did check through the cargo changelog and saw no signs that this has already been addressed.

@jnqnfe jnqnfe added the C-bug Category: bug label Dec 5, 2020
@jnqnfe
Copy link
Author

jnqnfe commented Dec 5, 2020

Related to / dup of #8811

@jnqnfe
Copy link
Author

jnqnfe commented Dec 5, 2020

I've updated to cargo v1.46 (0.47) and there seems to be no difference.

@ehuss
Copy link
Contributor

ehuss commented Jan 7, 2021

Linking some similar issues: #3053 and #4001. Currently build scripts don't differentiate on any of the commands/modes.

@ehuss ehuss added A-build-scripts Area: build.rs scripts Command-doc labels Jan 7, 2021
jnqnfe added a commit to jnqnfe/pulse-binding-rust that referenced this issue Jul 18, 2023
pkg-config checks performed by build scripts need to be disabled when
building docs on docs.rs. rust version 1.41 introduced a new `doc` feature
for doc-build mode detection, which replaced the previous ugly `dox`
solution, but unfortunately the official `doc` flag is not passed along to
build scripts (see [1]). a workaround was thus needed which was to use a
cargo.toml entry to force presence of a flag that build scripts can see. i
chose to use the name `doc` for this simply for consistency with the
official flag.

this solution worked fine until the authors of the `quote` crate made a
change (see [2] as pointed out to me at [3]) such that when the `doc` flag
is detected, their macro is substituted with an empty one in the name of
cleaner documentation. this caused docs.rs builds of my crate to fail with
errors (pointing to the `num-derive` crate, which must be the one pulling
in `quote).

the fix it seems is to either (a) simply use a different name, e.g.
"docsrs", or (b) to switch to using the new `DOCS_RS` environment variable
as recommended at [4]. i have chosen to go with the latter.

if the issue at [1] is ever resolved, we could switch back to using
`cfg!(doc)` based detection in the build scripts to disable the check
for local doc building (unnecessary) as well as for docs.rs.

note, when re-reading 7147efa to refresh
my memory i was initially confused by the second to last paragraph, so i'm
going to briefly clarify for my own future benefit: i did not mean that the
name `docsrs` could not have been used for `rustc-args` at that time, i
just meant that i wanted to use the more official name of `doc` in this
case, whilst the `rustdoc-args` entry that uses `docsrs` could not use
`doc` since it would not have compiled with the stable compiler, and the
inconsistency was just going to be temporary since the `rustdoc-args` entry
is just there until the `doc_cfg` feature stablises.

[1]: rust-lang/cargo#8944
[2]: https://docs.rs/quote/1.0.23/src/quote/lib.rs.html#471-477
[3]: rust-num/num-derive#53
[4]: https://docs.rs/about/builds
@epage
Copy link
Contributor

epage commented Sep 30, 2023

See also #8811 (comment)

@epage
Copy link
Contributor

epage commented Sep 30, 2023

As far as I can tell, this is a dupe of #8811, I'm closing this in favor of it.

If there is a reason we need this separate issue, let us know and we can revisit!

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts C-bug Category: bug Command-doc
Projects
None yet
Development

No branches or pull requests

3 participants