-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Stabilize doctest-xcompile #15462
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
base: master
Are you sure you want to change the base?
Stabilize doctest-xcompile #15462
Conversation
r? @weihanglo rustbot has assigned @weihanglo. Use |
@rust-lang/cargo I'm proposing to stabilize this, though I think it should wait until 1.89. I figured I'd get this process started in case anyone had any concerns or needs time to look at it. @rfcbot fcp merge |
Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
2603268
to
fccb97d
Compare
🔔 This is now entering its final comment period, as per the review above. 🔔 |
fccb97d
to
3e7aaf4
Compare
This comment has been minimized.
This comment has been minimized.
This stabilizes the doctest-xcompile feature by unconditionally enabling it.
3e7aaf4
to
83a6acd
Compare
This stabilizes the doctest-xcompile feature by unconditionally enabling it.
Closes #7040
Closes #12118
What is being stabilized?
This changes it so that cargo will run doctests when using the
--target
flag for a target that is not the host. Previously, cargo would ignore doctests (and show a note if passing--verbose
).A wrapper for running the doctest can be specified with the
target.*.runner
configuration option (which is powered by the--test-runtool
rustdoc flag). This would typically be something like qemu to run under emulation. It is my understanding that this should work just like running other kinds of tests.Additionally, the
target.*.linker
config option is honored for using a custom linker.Already stabilized in rustdoc is the ability to ignore tests per-target.
Motivation
The lack of doctest cross-compile support has always been simply due to the lack of functionality in rustdoc to support this. Rustdoc gained the ability to cross-compile doctests some time ago, but there were additional flags like the test runner that were not stabilized until just recently.
This is intended to ensure that projects have full test coverage even when doing cross-compilation. It can be surprising to some that this was not happening, particularly since cargo is silent about it.
Risks
The cargo team had several conversations about how to roll out this feature. Ultimately we decided to enable it unconditionally with the understanding that most projects will probably want to have their doctests covered, and that any breakage will be a local concern that can be resolved by either fixing the test or ignoring the target.
Tests in rust-lang/rust run into this issue, particularly on android, and those will need to be fixed before this reaches beta. This is something I am looking into.
Some cross-compiling scenarios may need codegen flags that are not supported. It's not clear how common this will be, or if ignoring will be a solution, or how difficult it would be to update rustdoc and cargo to support these. Additionally, the split between RUSTFLAGS and RUSTDOCFLAGS can be cumbersome.
Implementation history
--target
flag and runtool and per-target-ignores.--target=HOST
not working on stable.--verbose
).--test-run-directory
interaction with--test-runtool
.--test-runtool
and--test-runtool-arg
CLI args, and drops--enable-per-target-ignores
unconditionally enabling it.Test coverage
Cargo tests:
.json
targets work with rustdoc cross tests.ignore-*
syntax works.Rustdoc tests:
--test-run-directory
with relative paths of the runner.--test-runtool
and--test-runtool-arg
.Future concerns
There have been some discussions (rust-lang/testing-devex-team#5) about changing how doctests are driven. My understanding is that stabilizing this should not affect those plans, since if cargo becomes the driver, it will simply need to build things with
--target
and use the appropriate runner.Change notes
This PR changed tests a little:
-Zdoctest-xcompile
since-Zbuild-std
no longer uses a target.I think this should probably wait until the next release cutoff, moving this to 1.89 (will update the PR accordingly if that happens).