cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Published 20 hours ago • verified publisher icon rust-lang

feat: support `target.<triple>.rustdocflags` officially

Open weihanglo opened this issue 1 year ago • 3 comments

What does this PR try to resolve?

CARGO_TARGET_<TRIPLE>_RUSTDOCFLAGS was accidentally accepted somehow, so support both config and env officially, given that removing env support would be a breaking change.

Fixes #13189

How should we test and review this PR?

Commit by commit.

Additional information

I leave target.<cfg>.rustdocflags for future possibilities, as it might incur the same convergence issue target.<cfg>.rustflags has.

weihanglo avatar Dec 23 '23 05:12 weihanglo

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

rustbot avatar Dec 23 '23 05:12 rustbot

Can you add a test for cargo test --doc, too, since it goes through a very different code path?

ehuss avatar Dec 24 '23 00:12 ehuss

Added a new test target_triple_rustdocflags_works_through_cargo_test.

Let me know if there is anything missing.

weihanglo avatar Dec 24 '23 14:12 weihanglo

When combined with -Zhost-config does this also support host.rustdocflags? (I've just found out we actually need to use this for docs.rs to fix issues with combining -Zhost-config and --config build.rustdocflags).

Nemo157 avatar Dec 31 '23 16:12 Nemo157

I didn't include that in this PR, but can probably do that if there is a use case.

weihanglo avatar Dec 31 '23 17:12 weihanglo

:umbrella: The latest upstream changes (presumably #13409) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Feb 20 '24 19:02 bors

@bors r+

Seems reasonable. Fleshing this out isn't the required way to go (could also deprecate it) but I think its a reasonable to do. This does slightly expand our compatibility guarantees but its in such alignment with what we already do that I don't think its worth holding a straw vote on this. If someone disagrees, we can revert before March 21 and re-evaluate.

epage avatar Feb 21 '24 01:02 epage

:pushpin: Commit 6131e991af7dfd14b7641c0b09664d8c81d9af8d has been approved by epage

It is now in the queue for this repository.

bors avatar Feb 21 '24 01:02 bors

:hourglass: Testing commit 6131e991af7dfd14b7641c0b09664d8c81d9af8d with merge 3d47dd412e4c342ca5cb28cf9944ed9de9462b83...

bors avatar Feb 21 '24 01:02 bors

:sunny: Test successful - checks-actions Approved by: epage Pushing 3d47dd412e4c342ca5cb28cf9944ed9de9462b83 to master...

bors avatar Feb 21 '24 01:02 bors

When combined with -Zhost-config does this also support host.rustdocflags? (I've just found out we actually need to use this for docs.rs to fix issues with combining -Zhost-config and --config build.rustdocflags).

@Nemo157 could you elaborate on this more and create a new issue, so we won't lose track of it?

weihanglo avatar Feb 21 '24 02:02 weihanglo