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

Use --config build.rustdocflags instead of cargo rustdoc -- <flags> #1543

Merged
merged 1 commit into from
Nov 4, 2021

Conversation

willcrichton
Copy link
Contributor

For the --scrape-examples feature, it's important that rustdoc flags like --static-root-path be passed to all rustdoc invocations. This way, sources generated for reverse dependencies have the correct root path.

To fix this issue, this PR changes the call to cargo rustdoc to move flags that were previously passed via -- <flags> to RUSTDOCFLAGS=<flags>.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing this to all invocations of rustdoc is fine, but using RUSTDOCFLAGS is not the right approach, because it mangles whitespace. Please use -Zconfig, like rustcflags uses. This should also have the benefit that you don't need to change the API, just the implementation.

@willcrichton willcrichton changed the title Use RUSTDOCFLAGS instead of cargo rustdoc -- <flags> Use --config build.rustdocflags instead of cargo rustdoc -- <flags> Nov 4, 2021
@willcrichton
Copy link
Contributor Author

Ohh I understand. @jyn514 does the latest commit look like what you expect then?

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willcrichton
Copy link
Contributor Author

Done.

@jyn514 jyn514 merged commit e52460f into rust-lang:master Nov 4, 2021
@jyn514 jyn514 added the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Nov 4, 2021
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Nov 30, 2021
@nagisa
Copy link
Member

nagisa commented Jan 14, 2022

I believe this has resulted in all dependencies receiving the flags specified in package.metadata.docs.rs.rustc-args.

Most notably a commonly used --cfg docsrs does not necessarily will allow a dependency to compile (but the code remains valid enough to document). And cargo rustdoc compiles the dependencies for some reason.

@willcrichton
Copy link
Contributor Author

Perhaps the solution is to carve off a whitelist of flags like --static-root-path that are passed in via build.rustdocflags and pass the remaining flags as they were before?

@nagisa
Copy link
Member

nagisa commented Jan 14, 2022

…Actually, nevermind. I missed the distinction between build.rustdocflags and build.rustflags. Builds with build.rustdocflags still work fine!

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 this pull request may close these issues.

4 participants