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.

Sign up for GitHub

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

Jump to bottom

Use rust_out{exe_suffix} for doctests #95836

Merged
merged 2 commits into from
Nov 27, 2022
Merged

Use rust_out{exe_suffix} for doctests #95836

merged 2 commits into from
Nov 27, 2022

Conversation

workingjubilee
Copy link
Member

This was mentioned as an issue to me by @bruxisma. There are 3 separate instances where "rust_out" can become part of the name of a Rust executable, so I am mostly just hoping that this fixes the problem, given that the other sites which it can slip in seem to be well-behaved.

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Apr 9, 2022
@rust-highfive
Copy link
Collaborator

r? @notriddle

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 9, 2022
@ChrisDenton
Copy link
Member

For maximum portability and future proofing you may want to use one of the consts in std::env::consts

@workingjubilee
Copy link
Member Author

Oh nice, I didn't even know those existed. You learn something new every day etc.
Also a very good point since on e.g. wasi this should be .wasm, and so on.

@workingjubilee workingjubilee changed the title Use rust_out.exe for doctests on Windows Use rust_out{,.exe,.wasm} for doctests Apr 9, 2022
@notriddle
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 9, 2022

📌 Commit d483220 has been approved by notriddle

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 9, 2022
@Dylan-DPC
Rollup merge of rust-lang#95836 - workingjubilee:doctest-exe, r=notri…
fcc2c10 
…ddle

Use rust_out{,.exe,.wasm} for doctests

This was mentioned as an issue to me by `@bruxisma.` There are 3 separate instances where "rust_out" can become part of the name of a Rust executable, so I am mostly just hoping that this fixes the problem, given that the other sites which it can slip in seem to be well-behaved.
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 9, 2022
Closed
bjorn3
bjorn3 reviewed Apr 11, 2022
View reviewed changes
@notriddle
Copy link
Contributor

You’re right, of course. Cross-compiling matters.

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 11, 2022
@workingjubilee
Copy link
Member Author

workingjubilee commented Apr 11, 2022 •
edited
Loading

Honestly, I'm surprised we don't have a single unified interface for handling emitting binaries in the compiler that everyone is forced to go through. Or maybe I'm just missing something.

@workingjubilee workingjubilee changed the title Use rust_out{,.exe,.wasm} for doctests Use rust_out{target_exe_suffix} for doctests Apr 11, 2022
@workingjubilee
Copy link
Member Author

workingjubilee commented Apr 12, 2022 •
edited
Loading

I was going to investigate adding a relatively thorough test but having done more digging, persisting doctests and runtool are both unstable features, as are target spec jsons (the case this doesn't directly handle now), of course. I can investigate a further improvement in a later PR but this is already starting to get a bit afield of rustdoc's specialties and is in any case more correct than what was there before.

@rustbot label: -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 12, 2022
@notriddle
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 22, 2022

📌 Commit 7882cf7ff44e5faa8bda9e159d03116c7e092434 has been approved by notriddle

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2022
@bors
Copy link
Contributor

bors commented Apr 23, 2022

⌛ Testing commit 7882cf7ff44e5faa8bda9e159d03116c7e092434 with merge 69aab9ae8baa2490718c54ae633f447beaaa16bf...

@bors
Copy link
Contributor

bors commented Apr 23, 2022

💔 Test failed - checks-actions

@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Nov 27, 2022
@jyn514
Copy link
Member

jyn514 commented Nov 27, 2022 

  $( for file in /d/a/rust/rust/build/x86_64-pc-windows-msvc/test/run-make-fulldeps/coverage-reports/coverage-reports/rustdoc-abort/*/rust_out; do [ -x "$file" ] && printf "%s %s " -object $file; done ) \

this probably just needs to use rust_out* instead of rust_out.

@workingjubilee
Copy link
Member Author

Oh, sorry! I had forgotten about this. ^^;

@workingjubilee workingjubilee force-pushed the doctest-exe branch 2 times, most recently from 670a69a to 9808534 Compare November 27, 2022 03:27
@workingjubilee
Use target exe_suffix for doctests
353cef9 
This will use rust_out.exe for doctests on Windows,
rust_out.wasm for doctests in the wasm case, and
also handles cross-compiling or user-provided targets.
@workingjubilee
Attempt to solve problem with globs
47ddca6
@workingjubilee workingjubilee changed the title Use rust_out{target_exe_suffix} for doctests Use rust_out{exe_suffix} for doctests Nov 27, 2022
@workingjubilee
Copy link
Member Author

Okay. New version should handle every single case, including user-provided targets, smoothly.

@notriddle
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 27, 2022

📌 Commit 47ddca6 has been approved by notriddle

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 27, 2022
@workingjubilee workingjubilee removed the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Nov 27, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 27, 2022
@matthiaskrgr
Rollup merge of rust-lang#95836 - workingjubilee:doctest-exe, r=notri…
72ae08d 
…ddle

Use `rust_out{exe_suffix}` for doctests

This was mentioned as an issue to me by `@bruxisma.` There are 3 separate instances where "rust_out" can become part of the name of a Rust executable, so I am mostly just hoping that this fixes the problem, given that the other sites which it can slip in seem to be well-behaved.
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 27, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 27, 2022
@matthiaskrgr
Rollup merge of rust-lang#95836 - workingjubilee:doctest-exe, r=notri…
db71df3 
…ddle

Use `rust_out{exe_suffix}` for doctests

This was mentioned as an issue to me by ``@bruxisma.`` There are 3 separate instances where "rust_out" can become part of the name of a Rust executable, so I am mostly just hoping that this fixes the problem, given that the other sites which it can slip in seem to be well-behaved.
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 27, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 27, 2022
@matthiaskrgr
Rollup merge of rust-lang#95836 - workingjubilee:doctest-exe, r=notri…
c3bbceb 
…ddle

Use `rust_out{exe_suffix}` for doctests

This was mentioned as an issue to me by ```@bruxisma.``` There are 3 separate instances where "rust_out" can become part of the name of a Rust executable, so I am mostly just hoping that this fixes the problem, given that the other sites which it can slip in seem to be well-behaved.
This was referenced Nov 27, 2022
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2022
@bors
Auto merge of rust-lang#104983 - matthiaskrgr:rollup-018sk73, r=matth…
1eb62b1 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#95836 (Use `rust_out{exe_suffix}` for doctests)
 - rust-lang#104882 (notify lcnr on changes to `ObligationCtxt`)
 - rust-lang#104892 (Explain how to get the discriminant out of a `#[repr(T)] enum` with payload)
 - rust-lang#104917 (Allow non-org members to label `requires-debug-assertions`)
 - rust-lang#104931 (Pretty-print generators with their `generator_kind`)
 - rust-lang#104934 (Remove redundant `all` in cfg)
 - rust-lang#104944 (Support unit tests for jsondoclint)
 - rust-lang#104946 (rustdoc: improve popover focus handling JS)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7ce937f into rust-lang:master Nov 27, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 27, 2022
@workingjubilee
Copy link
Member Author

Awesome! Thank you for reminding me about this!

@workingjubilee workingjubilee deleted the doctest-exe branch September 19, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

Assignees

@notriddle notriddle

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.67.0
Development

Successfully merging this pull request may close these issues.
10 participants
@workingjubilee @rust-highfive @ChrisDenton @notriddle @bors @rust-log-analyzer @JohnCSimon @jyn514 @bjorn3 @rustbot