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

Migrate compiler-lookup-paths, dump-mono-stats and prune-link-args run-make tests to rmake or ui format #126208

Merged
merged 4 commits into from
Jul 17, 2024
Merged

Migrate compiler-lookup-paths, dump-mono-stats and prune-link-args run-make tests to rmake or ui format #126208

merged 4 commits into from
Jul 17, 2024

Conversation

Oneirical
Copy link
Contributor

@Oneirical Oneirical commented Jun 9, 2024 •
edited by jieyouxu
Loading

Part of #121876 and the associated Google Summer of Code project.

try-job: x86_64-msvc
try-job: aarch64-apple
try-job: dist-x86_64-linux
try-job: armhf-gnu

@rustbot
Copy link
Collaborator

rustbot commented Jun 9, 2024

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 9, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 9, 2024

Some changes occurred in run-make tests.

cc @jieyouxu

The run-make-support library was changed

cc @jieyouxu

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Kobzol Kobzol mentioned this pull request Jun 10, 2024
Open
@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 10, 2024
@Oneirical Oneirical force-pushed the one-flew-over-the-cuckoo's-test branch from 5d9d76d to 4484eb1 Compare June 10, 2024 14:42
@rustbot rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Jun 10, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Oneirical Oneirical force-pushed the one-flew-over-the-cuckoo's-test branch from 4484eb1 to 890a85e Compare June 10, 2024 14:48
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Oneirical Oneirical force-pushed the one-flew-over-the-cuckoo's-test branch from 890a85e to 640c0f0 Compare June 10, 2024 14:58
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Oneirical Oneirical force-pushed the one-flew-over-the-cuckoo's-test branch from 640c0f0 to 6bf6241 Compare June 10, 2024 15:17
@Oneirical
Copy link
Contributor Author

Oneirical commented Jun 10, 2024 •
edited
Loading

error: multiple input filenames provided (first two filenames are /checkout/tests/ui/invalid-compile-flags/prune-link-args.rs and ")

Amusingly, I appear to be running into the same issue which the test author experienced 11 years ago:

Ok, I tried that [making a run-pass test instead of a run-make test]. Guess who has a problem with quoted command line parameters: the test runner. I'm not sure if it's worth it to fix that or if the current makefile test suffices.

So... to ask the question again, over a decade later - is it worth it to fix that? Maybe just adding backslashes would suffice, but I am not sure.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Oneirical Oneirical force-pushed the one-flew-over-the-cuckoo's-test branch from 6bf6241 to 16c0be8 Compare June 11, 2024 13:57
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jieyouxu
Copy link
Member

So... to ask the question again, over a decade later - is it worth it to fix that? Maybe just adding backslashes would suffice, but I am not sure.

If it's a hassle then it's fine to just keep it as a run-make test (would be helpful including this info in the test description).

@Oneirical Oneirical force-pushed the one-flew-over-the-cuckoo's-test branch 2 times, most recently from 115d7d3 to 51903b5 Compare June 11, 2024 14:40
@Oneirical
Copy link
Contributor Author

If it's a hassle then it's fine to just keep it as a run-make test (would be helpful including this info in the test description).

Done, it would be nice to run a Windows try job afterwards, as there was an unexplained ignore flag.

@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 16, 2024
@Oneirical Oneirical force-pushed the one-flew-over-the-cuckoo's-test branch from b934542 to a3f9d13 Compare July 16, 2024 15:27
@Oneirical
Copy link
Contributor Author

Looks like that fails for what the test is actually trying to test (the space -lc is passed as an empty argument). Ignore added and reason added, at least it's less restrictive than the old test (only windows-gnu, not all windows).

@bors r=@jieyouxu rollup=iffy (link args, llvm-ar, etc.)

@bors
Copy link
Contributor

bors commented Jul 16, 2024

📌 Commit a3f9d13 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 Jul 16, 2024
@bors
Copy link
Contributor

bors commented Jul 16, 2024

☔ The latest upstream changes (presumably #127819) made this pull request unmergeable. Please resolve the merge conflicts.

@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 Jul 16, 2024
@Oneirical Oneirical force-pushed the one-flew-over-the-cuckoo's-test branch from a3f9d13 to ea2b699 Compare July 16, 2024 19:34
@Oneirical
Copy link
Contributor Author

@bors r=@jieyouxu rollup=iffy (link args, llvm-ar, etc.)

@bors
Copy link
Contributor

bors commented Jul 16, 2024

📌 Commit ea2b699 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 Jul 16, 2024
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 16, 2024
@tgross35
Rollup merge of rust-lang#126208 - Oneirical:one-flew-over-the-cuckoo…
a0f91da 
…'s-test, r=jieyouxu

Migrate `compiler-lookup-paths`, `dump-mono-stats` and `prune-link-args` `run-make` tests to `rmake` or `ui` format

Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html).

try-job: x86_64-msvc
try-job: aarch64-apple
try-job: dist-x86_64-linux
try-job: armhf-gnu
@tgross35 tgross35 mentioned this pull request Jul 16, 2024
Closed
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 16, 2024
@tgross35
Rollup merge of rust-lang#126208 - Oneirical:one-flew-over-the-cuckoo…
5080c9f 
…'s-test, r=jieyouxu

Migrate `compiler-lookup-paths`, `dump-mono-stats` and `prune-link-args` `run-make` tests to `rmake` or `ui` format

Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html).

try-job: x86_64-msvc
try-job: aarch64-apple
try-job: dist-x86_64-linux
try-job: armhf-gnu
@tgross35 tgross35 mentioned this pull request Jul 16, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
@bors
Auto merge of rust-lang#127837 - tgross35:rollup-bq4ffc3, r=tgross35
6b12e7e 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#125206 (Simplify environment variable examples)
 - rust-lang#126208 (Migrate `compiler-lookup-paths`, `dump-mono-stats` and `prune-link-args` `run-make` tests to `rmake` or `ui` format)
 - rust-lang#126271 (Skip fast path for dec2flt when optimize_for_size)
 - rust-lang#126776 (Clean up more comments near use declarations)
 - rust-lang#127444 (`impl Send + Sync` and override `count` for the `CStr::bytes` iterator)
 - rust-lang#127792 (std: Use `read_unaligned` for reads from DWARF)
 - rust-lang#127807 (Use futex.rs for Windows thread parking)
 - rust-lang#127810 (Rename `tcx` to `cx` in `rustc_type_ir`)
 - rust-lang#127816 (Update method name to reflect changes to its internals)
 - rust-lang#127833 (zkvm: add `#[forbid(unsafe_op_in_unsafe_fn)]` in `stdlib`)
 - rust-lang#127836 (std: Forbid unwrapped unsafe ops in xous and uefi modules)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Jul 17, 2024

⌛ Testing commit ea2b699 with merge 1a6e777...

@bors
Copy link
Contributor

bors commented Jul 17, 2024

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing 1a6e777 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 17, 2024
@bors bors merged commit 1a6e777 into rust-lang:master Jul 17, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 17, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1a6e777): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (secondary 2.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 2.9% [2.1%, 3.8%] 13
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 768.585s -> 769.376s (0.10%)
Artifact size: 328.66 MiB -> 328.66 MiB (0.00%)

@Oneirical Oneirical mentioned this pull request Jul 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kobzol Kobzol Kobzol left review comments

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. 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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Oxidizing run-make tests
Status: Done
Milestone
1.81.0
Development

Successfully merging this pull request may close these issues.
7 participants
@Oneirical @rustbot @rust-log-analyzer @jieyouxu @Kobzol @bors @rust-timer