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

Pass cfg to build script #1580

Closed
dtolnay opened this issue Dec 14, 2021 · 14 comments · Fixed by #1800
Closed

Pass cfg to build script #1580

dtolnay opened this issue Dec 14, 2021 · 14 comments · Fixed by #1800

Comments

@dtolnay
Copy link
Member

dtolnay commented Dec 14, 2021

#1543 #1559 broke proc-macro2's documentation build — see dtolnay/proc-macro2#309.

As far as I can tell the difference is that cargo rustdoc -- --cfg mycfg sets mycfg during both the compilation of the build.rs and the invocation of rustdoc, whereas cargo rustdoc --config 'build.rustdocflags=["--cfg", "mycfg"]' sets it only during rustdoc and not the compilation of build.rs.

As far as I can tell there is now no way to set cfg for a build script build on docs.rs.

@willcrichton @jyn514

@willcrichton
Copy link
Contributor

Thanks for the report @dtolnay!

@jyn514 what do you think is the right fix here? We could add cfg as a new field to [package.metadata.docs.rs] if it's a special case. Or I could duplicate the rustdoc-args field between --config 'build.rustdocflags=<flags>' and rustdoc -- <flags>. Not sure if that would also cause duplicate flag CLI errors.

@jyn514
Copy link
Member

jyn514 commented Dec 14, 2021

As far as I can tell there is now no way to set cfg for a build script build on docs.rs.

Can't you set it with RUSTFLAGS? There's a lever for modifying it in the metadata with rustc-args.

Hmm, I guess that applies it to all dependencies, too ... is that a problem for your use-case?

@dtolnay
Copy link
Member Author

dtolnay commented Dec 14, 2021

Nope, cfg from rustc-args does not get applied either. That was already in the build that regressed. See https://github.com/dtolnay/proc-macro2/blob/1.0.33/Cargo.toml#L19-L20 and https://docs.rs/crate/proc-macro2/1.0.33/builds/473000.

@Nemo157
Copy link
Member

Nemo157 commented Dec 14, 2021

As far as I can tell the difference is that cargo rustdoc -- --cfg mycfg sets mycfg during both the compilation of the build.rs and the invocation of rustdoc

This seems like it would be incorrect behavior, and doesn't match what I see testing it locally. I also tried building the proc-macro2==1.0.33 docs with my script that behaves similarly to how docs.rs used to, which failed similarly to that build. That ended up running the command:

env DOCS_RS=1 RUSTCFLAGS='--cfg procmacro2_semver_exempt' cargo rustdoc --locked --target x86_64-unknown-linux-gnu -- -Z unstable-options --cfg procmacro2_semver_exempt --cfg doc_cfg --extern-html-root-url quote=https://docs.rs/quote/1.0.10 --extern-html-root-url unicode_xid=https://docs.rs/unicode-xid/0.2.2

@jyn514
Copy link
Member

jyn514 commented Dec 14, 2021

RUSTCFLAGS='--cfg procmacro2_semver_exempt'

Shouldn't that be RUSTFLAGS?

@Nemo157
Copy link
Member

Nemo157 commented Dec 14, 2021

Yep, but using RUSTFLAGS doesn't fix it, the --cfg doesn't appear to get passed to the build script build.

@Nemo157
Copy link
Member

Nemo157 commented Dec 14, 2021

I'm really confused, copying the exact command run from the previous successful build while trying to build proc-macro2==1.0.32 locally also fails in the same way, even when overriding to the nightly that was used.

"cargo" "+nightly-2021-10-26" "rustdoc" "--lib" "-Zrustdoc-map" "-Z" "unstable-options" "--config" "build.rustflags=[\"--cfg\", \"procmacro2_semver_exempt\"]" "-Zunstable-options" "--config=doc.extern-map.registries.crates-io=\"https://docs.rs/{pkg_name}/{version}/x86_64-unknown-linux-gnu\"" "-j2" "--" "--cfg" "procmacro2_semver_exempt" "--cfg" "doc_cfg" "-Z" "unstable-options" "--emit=invocation-specific" "--resource-suffix" "-20211026-1.58.0-nightly-e269e6bf4" "--static-root-path" "/" "--cap-lints" "warn" "--disable-per-crate-search"

@Nemo157
Copy link
Member

Nemo157 commented Dec 14, 2021

Ok, found the reason I was having that fail too, --config build.rustflags gets overridden by target.x86_64-unknown-linux-gnu.rustflags in my global config instead of being merged together. That shouldn't affect docs.rs since it shouldn't have a global .cargo/config.

After trying a bunch of changes I finally found the difference, the --target x86_64-unknown-linux-gnu changes whether build.rustflags gets passed to the build-script or not. That seems like a cargo bug to me.

@jyn514
Copy link
Member

jyn514 commented Dec 14, 2021

Ah, ok - that's caused by #1559 then, not #1543.

@jyn514
Copy link
Member

jyn514 commented Dec 14, 2021

cc rust-lang/cargo#7677

@Nemo157
Copy link
Member

Nemo157 commented Dec 14, 2021

And for the specific failure here: rust-lang/cargo#4423. I've always assumed build.rustflags was global arguments to apply to all builds, but it looks like it actually means "the args for the implicit host target, but only for host-only builds like proc-macros and build-scripts when the final artifact is the implicit host target"; and there's no way to explicitly set flags for "host-only non-artifact builds".

@Nemo157
Copy link
Member

Nemo157 commented Dec 14, 2021

Ah, following links a little more I found https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#host-config which might be able to do what we need; -Zhost-config --config=host.rustflags=....

@Nemo157
Copy link
Member

Nemo157 commented Dec 16, 2021

To clarify what happened and how I think we should fix this:

Prior to #1559:

  • compiling for x86_64-unknown-linux-gnu would apply the specified rustc-args to all builds, including while compiling the build-scripts
  • compiling for other targets would apply the specified rustc-args to only the builds that get linked into the final artifact, so not including the build-scripts

After #1559:

  • all (non-proc-macro) builds are now treated as cross-compiles
  • compiling for all targets would apply the specified rustc-args to only the artifact builds, not including the build-scripts

I think we should update to apply the specified rustc-args to all builds no matter what the --target is. I attempted to use -Zhost-config to implement this, but ran into rust-lang/cargo#10206; so I think we're blocked on fixing this till that is fixed in cargo.

@jyn514
Copy link
Member

jyn514 commented Aug 23, 2022

rust-lang/cargo#10206 has been fixed.

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.

4 participants