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

Port tests/run-make-fulldeps/hotplug_codegen_backend to ui-fulldeps #126111

Merged
merged 2 commits into from
Jun 8, 2024

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Jun 7, 2024

This is the last remaining run-make-fulldeps test, which means I actually had to leave behind a dummy README file to prevent compiletest from complaining about a missing directory.

(Removing the run-make-fulldeps suite entirely is non-trivial, so I intend to do so in a separate PR after this one.)


I wasn't sure about adding a new kind of aux build just for this one test, so I also tried to just port this test from Makefile to rmake instead.

But I found that I couldn't get rmake to fully work for a run-make-fulldeps test, which convinced me that getting rid of run-make-fulldeps is worthwhile.

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

rustbot commented Jun 7, 2024

Some changes occurred in src/tools/compiletest

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.

Thanks, the impl looks very reasonable.

@jieyouxu
Copy link
Member

jieyouxu commented Jun 7, 2024

r=me after CI is green. @bors delegate+ rollup=iffy (this might be slightly wonky in full build CI)

@bors
Copy link
Contributor

bors commented Jun 7, 2024

✌️ @Zalathar, 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

@Zalathar
Copy link
Contributor Author

Zalathar commented Jun 7, 2024

CI is green.

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Jun 7, 2024

📌 Commit af53d00 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 Jun 7, 2024
@Zalathar
Copy link
Contributor Author

Zalathar commented Jun 7, 2024

OK, I've pushed a small update that lets us emit the “full” deps file instead of a bare-bones one (diff).

@Zalathar
Copy link
Contributor Author

Zalathar commented Jun 7, 2024

(With the update, the change to detect -o is no longer needed, but it's already written and does slightly improve a different test, so I'll leave it as-is.)

@jieyouxu
Copy link
Member

jieyouxu commented Jun 7, 2024

Looks good, r=me after CI is green as usual.

@Zalathar
Copy link
Contributor Author

Zalathar commented Jun 7, 2024

CI is green.

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Jun 7, 2024

📌 Commit 7c9b469 has been approved by jieyouxu

It is now in the queue for this repository.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 7, 2024
Improve docs for using custom paths with `--emit`

Recently I found myself concluding that this feature didn't exist (rust-lang#126111 (comment)), despite having read the documentation, because it was hidden away in the middle of a paragraph full of other information.

Giving this documentation more space of its own should make it easier to find.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 7, 2024
Improve docs for using custom paths with `--emit`

Recently I found myself concluding that this feature didn't exist (rust-lang#126111 (comment)), despite having read the documentation, because it was hidden away in the middle of a paragraph full of other information.

Giving this documentation more space of its own should make it easier to find.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 7, 2024
Improve docs for using custom paths with `--emit`

Recently I found myself concluding that this feature didn't exist (rust-lang#126111 (comment)), despite having read the documentation, because it was hidden away in the middle of a paragraph full of other information.

Giving this documentation more space of its own should make it easier to find.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2024
Rollup merge of rust-lang#126119 - Zalathar:emit-filename, r=ehuss

Improve docs for using custom paths with `--emit`

Recently I found myself concluding that this feature didn't exist (rust-lang#126111 (comment)), despite having read the documentation, because it was hidden away in the middle of a paragraph full of other information.

Giving this documentation more space of its own should make it easier to find.
@bors
Copy link
Contributor

bors commented Jun 8, 2024

⌛ Testing commit 7c9b469 with merge d8fde50...

@bors
Copy link
Contributor

bors commented Jun 8, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 8, 2024
@bors bors merged commit d8fde50 into rust-lang:master Jun 8, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 8, 2024
@Zalathar Zalathar deleted the fulldeps-hotplug branch June 8, 2024 10:13
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d8fde50): 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 (primary 1.7%, secondary 2.6%)

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)
1.7% [1.4%, 2.2%] 5
Regressions ❌
(secondary)
2.6% [2.1%, 3.0%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.7% [1.4%, 2.2%] 5

Binary size

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

Bootstrap: missing data
Artifact size: 319.64 MiB -> 319.67 MiB (0.01%)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 9, 2024
…-ozkan

Remove empty test suite `tests/run-make-fulldeps`

After rust-lang#109770, there were only a handful of tests left in the run-make-fulldeps suite.

As of rust-lang#126111, there are no longer *any* run-make-fulldeps tests, so now we can:

- Remove the directory
- Remove related bootstrap/compiletest code
- Remove various other references in CI scripts and documentation.

By removing this suite, we also no longer need to worry about discrepancies between it and ui-fulldeps, and we don't have to worry about porting tests from Makefile to [rmake](rust-lang#121876) (or whether rmake even works with fulldeps).
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 9, 2024
Rollup merge of rust-lang#126155 - Zalathar:run-make-fulldeps, r=onur-ozkan

Remove empty test suite `tests/run-make-fulldeps`

After rust-lang#109770, there were only a handful of tests left in the run-make-fulldeps suite.

As of rust-lang#126111, there are no longer *any* run-make-fulldeps tests, so now we can:

- Remove the directory
- Remove related bootstrap/compiletest code
- Remove various other references in CI scripts and documentation.

By removing this suite, we also no longer need to worry about discrepancies between it and ui-fulldeps, and we don't have to worry about porting tests from Makefile to [rmake](rust-lang#121876) (or whether rmake even works with fulldeps).
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 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
None yet
Development

Successfully merging this pull request may close these issues.

5 participants