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

Add support for inputing via stdin with run-make-support #124612

Merged
merged 2 commits into from
May 3, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented May 2, 2024

This PR adds the facility to set a input bytes that will be passed via the standard input.

This is useful for testing rustc - (and soon rustdoc -).

In #124611 took the approach of having a dedicated run method but it is not very convenient to use and would necessitate many functions, one for success, one for fail, ...

Instead this PR takes a different approach and allows setting the input bytes as if it were a parameter and when calling the (now custom) output function, we write the input bytes into stdin. I think this gives us maximum flexibility in the implementation and a simple interface for users.

To test this new logic I ported tests/run-make/stdin-non-utf8/ to an rmake.rs one.

r? @jieyouxu

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 2, 2024
@rustbot
Copy link
Collaborator

rustbot commented May 2, 2024

Some changes occurred in run-make tests.

cc @jieyouxu

The run-make-support library was changed

cc @jieyouxu

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

I like how the API looks with custom stdin. One nit about the comment, after that r=me.

src/tools/run-make-support/src/lib.rs Outdated Show resolved Hide resolved
@Urgau Urgau force-pushed the run-make-stdin branch from 6716b76 to 619a569 Compare May 2, 2024 19:07
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks! r=me after CI is green.

@jieyouxu
Copy link
Member

jieyouxu commented May 2, 2024

@bors delegate=Urgau

@bors
Copy link
Contributor

bors commented May 2, 2024

✌️ @Urgau, you can now approve this pull request!

If @jieyouxu told you to "r=me" after making some further change, please make that change, then do @bors r=@jieyouxu

@Urgau
Copy link
Member Author

Urgau commented May 2, 2024

@bors r=@jieyouxu

@bors
Copy link
Contributor

bors commented May 2, 2024

📌 Commit 619a569 has been approved by jieyouxu

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-review Status: Awaiting review from the assignee but also interested parties. labels May 2, 2024
fmease added a commit to fmease/rust that referenced this pull request May 3, 2024
Add support for inputing via stdin with run-make-support

This PR adds the facility to set a input bytes that will be passed via the standard input.

This is useful for testing `rustc -` (and soon `rustdoc -`).

In rust-lang#124611 took the approach of having a dedicated `run` method but it is not very convenient to use and would necessitate many functions, one for success, one for fail, ...

Instead this PR takes a different approach and allows setting the input bytes as if it were a parameter and when calling the (now custom) `output` function, we write the input bytes into stdin. I think this gives us maximum flexibility in the implementation and a simple interface for users.

To test this new logic I ported `tests/run-make/stdin-non-utf8/` to an `rmake.rs` one.

r? ``@jieyouxu``
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#124412 (io safety: update Unix explanation to use `Arc`)
 - rust-lang#124441 (String.truncate comment microfix (greater or equal))
 - rust-lang#124594 (run-make-support: preserve tooks.mk behavior for EXTRACXXFLAGS)
 - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature)
 - rust-lang#124607 (`rustc_expand` cleanups)
 - rust-lang#124609 (variable-precision float operations can differ depending on optimization levels)
 - rust-lang#124610 (Tweak `consts_may_unify`)
 - rust-lang#124612 (Add support for inputing via stdin with run-make-support)

r? `@ghost`
`@rustbot` modify labels: rollup
@fmease
Copy link
Member

fmease commented May 3, 2024

Failed in rollup: #124643 (comment)
@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 May 3, 2024
@Urgau Urgau force-pushed the run-make-stdin branch from 619a569 to 2c4214e Compare May 3, 2024 06:10
@Urgau
Copy link
Member Author

Urgau commented May 3, 2024

auto - x86_64-msvc
2024-05-03T03:20:02.5274177Z assertion failed: out_dir.join("rust_out").try_exists().unwrap()

😮‍💨, I think I will always forget that Windows has .exe as suffix to executables. Adjusted the test for that, CI passes, let's try again.

@bors r=@jieyouxu

@bors
Copy link
Contributor

bors commented May 3, 2024

📌 Commit 2c4214e has been approved by jieyouxu

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 May 3, 2024
@ChrisDenton
Copy link
Member

Ideally this would use std::env::consts::EXE_SUFFIX so it wouldn't break if another platform would ever need an extension added.

@Urgau
Copy link
Member Author

Urgau commented May 3, 2024

Ideally this would use std::env::consts::EXE_SUFFIX so it wouldn't break if another platform would ever need an extension added.

I agree, that would be better, but I think the rmake.rs files are compiled for the current architecture (so they can be run) and so using std::env::consts::EXE_SUFFIX would break in case of cross-compilation.

bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#124461 (handle the targets that are missing in stage0)
 - rust-lang#124492 (Generalize `adjust_from_tcx` for `Allocation`)
 - rust-lang#124588 (Use `ObligationCtxt` in favor of `TraitEngine` in many more places)
 - rust-lang#124612 (Add support for inputing via stdin with run-make-support)
 - rust-lang#124613 (Allow fmt to run on rmake.rs test files)
 - rust-lang#124649 (Fix HorizonOS build broken by rust-lang#124210)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c27d3d5 into rust-lang:master May 3, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
Rollup merge of rust-lang#124612 - Urgau:run-make-stdin, r=jieyouxu

Add support for inputing via stdin with run-make-support

This PR adds the facility to set a input bytes that will be passed via the standard input.

This is useful for testing `rustc -` (and soon `rustdoc -`).

In rust-lang#124611 took the approach of having a dedicated `run` method but it is not very convenient to use and would necessitate many functions, one for success, one for fail, ...

Instead this PR takes a different approach and allows setting the input bytes as if it were a parameter and when calling the (now custom) `output` function, we write the input bytes into stdin. I think this gives us maximum flexibility in the implementation and a simple interface for users.

To test this new logic I ported `tests/run-make/stdin-non-utf8/` to an `rmake.rs` one.

r? `@jieyouxu`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants