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/sysroot-crates-are-unstable from Python to rmake #126231

Closed
wants to merge 1 commit into from

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Jun 10, 2024

EDIT: See #129071 for a much simpler migration that doesn't try to get rid of the Python script.

(I would still like to finish porting the Python script at some point, but that doesn't have to be tied to the Makefile migration.)


Part of the run-make Makefile purge (#121876), and also the Python purge (#110479).

This is a relatively complex script by run-make standards, and it's not a direct 1:1 port, though I did try to retain all the functionality of the old script (such as reporting all errors instead of just the first one).

It's a bit annoying that reporting all errors requires so much extra plumbing.


While reviewing this just before posting, I realised that I neglected to port the set_ld_lib_path stuff (#89033), so marking this as draft until I've had a chance to look into that.

@rustbot
Copy link
Collaborator

rustbot commented Jun 10, 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-run-make Area: port run-make Makefiles to rmake.rs 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 Jun 10, 2024
@Zalathar
Copy link
Contributor Author

Oh, it looks like rustc() already takes care of that automatically, so maybe this is fine.

@Zalathar
Copy link
Contributor Author

Like a lot of run-make tests, this one is frustratingly close to just being a UI test.

The trick is that it would need to run arbitrary code ahead of time, to generate a dynamic list of revisions (based on the sysroot contents), which is probably non-trivial to support in compiletest.

Alternatively, we could hardcode the list of sysroot crates, and have some kind of separate check to ensure that the hardcoded list matches the actual sysroot.

@jieyouxu
Copy link
Member

jieyouxu commented Jun 11, 2024

I'm fine with it being a run-make test tbh, run-make tests are exactly for the situation where it's impossible or otherwise very awkward to try to shoehorn into other test suites. It's great if we can reuse other test suites that are more specialized, but imo we shouldn't be trying super hard to avoid run-make just because it's run-make.

BTW, I'm trying to get 3 run-make-support infra PRs merged, I'll take a look at this PR after the infra PRs are merged

@Zalathar Zalathar force-pushed the sysroot-crates branch 2 times, most recently from 591e2e3 to 516495e Compare June 11, 2024 13:04
@bors
Copy link
Contributor

bors commented Jun 11, 2024

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

@Zalathar Zalathar changed the title Port tests/run-make/sysroot-crates-are-unstable to rmake Port tests/run-make/sysroot-crates-are-unstable from Python to rmake Aug 14, 2024
@Zalathar
Copy link
Contributor Author

Closing this, partly to prevent confusion with #129071, and partly because I want to have another go at it without the complexity of trying to report all failures.

@Zalathar Zalathar closed this Aug 14, 2024
@Zalathar Zalathar deleted the sysroot-crates branch August 14, 2024 03:38
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 14, 2024
Port `run-make/sysroot-crates-are-unstable` to rmake

I already have a more elaborate draft at rust-lang#126231 that tries to port the underlying Python script to rmake, but there's no need for the removal of Makefiles to be held up on complex tasks like that, so this PR simply takes the trivial Makefile and converts it into a trivial rmake recipe.

Part of rust-lang#121876.

r? `@jieyouxu`
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Aug 14, 2024
Port `run-make/sysroot-crates-are-unstable` to rmake

I already have a more elaborate draft at rust-lang#126231 that tries to port the underlying Python script to rmake, but there's no need for the removal of Makefiles to be held up on complex tasks like that, so this PR simply takes the trivial Makefile and converts it into a trivial rmake recipe.

Part of rust-lang#121876.

r? ``@jieyouxu``
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2024
Rollup merge of rust-lang#129071 - Zalathar:sysroot-unstable, r=jieyouxu

Port `run-make/sysroot-crates-are-unstable` to rmake

I already have a more elaborate draft at rust-lang#126231 that tries to port the underlying Python script to rmake, but there's no need for the removal of Makefiles to be held up on complex tasks like that, so this PR simply takes the trivial Makefile and converts it into a trivial rmake recipe.

Part of rust-lang#121876.

r? ``@jieyouxu``
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 15, 2024
Port the `sysroot-crates-are-unstable` Python script to rmake

New version of rust-lang#126231, and a follow-up to rust-lang#129071.

One major difference is that the new version no longer tries to report *all* accidentally-stable crates, because the `run_make_support` helpers tend to halt the test as soon as something goes wrong. That's unfortunate, but I think it won't matter much in practice, and preserving the old behaviour doesn't seem worth the extra effort.

---

Part of rust-lang#110479 (Python purge), with this being one of the non-trivial Python scripts that actually seems feasible and worthwhile to remove.

This is *not* part of rust-lang#121876 (Makefile purge), because the underlying test is already using rmake; this PR just modifies the existing rmake recipe to do all the work itself instead of delegating to Python. So there's no particular urgency here.

r? `@jieyouxu`

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: i686-mingw
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 15, 2024
Port the `sysroot-crates-are-unstable` Python script to rmake

New version of rust-lang#126231, and a follow-up to rust-lang#129071.

One major difference is that the new version no longer tries to report *all* accidentally-stable crates, because the `run_make_support` helpers tend to halt the test as soon as something goes wrong. That's unfortunate, but I think it won't matter much in practice, and preserving the old behaviour doesn't seem worth the extra effort.

---

Part of rust-lang#110479 (Python purge), with this being one of the non-trivial Python scripts that actually seems feasible and worthwhile to remove.

This is *not* part of rust-lang#121876 (Makefile purge), because the underlying test is already using rmake; this PR just modifies the existing rmake recipe to do all the work itself instead of delegating to Python. So there's no particular urgency here.

r? `@jieyouxu`

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: i686-mingw
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
Port the `sysroot-crates-are-unstable` Python script to rmake

New version of rust-lang#126231, and a follow-up to rust-lang#129071.

One major difference is that the new version no longer tries to report *all* accidentally-stable crates, because the `run_make_support` helpers tend to halt the test as soon as something goes wrong. That's unfortunate, but I think it won't matter much in practice, and preserving the old behaviour doesn't seem worth the extra effort.

---

Part of rust-lang#110479 (Python purge), with this being one of the non-trivial Python scripts that actually seems feasible and worthwhile to remove.

This is *not* part of rust-lang#121876 (Makefile purge), because the underlying test is already using rmake; this PR just modifies the existing rmake recipe to do all the work itself instead of delegating to Python. So there's no particular urgency here.

r? `@jieyouxu`

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: i686-mingw
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
Port the `sysroot-crates-are-unstable` Python script to rmake

New version of rust-lang#126231, and a follow-up to rust-lang#129071.

One major difference is that the new version no longer tries to report *all* accidentally-stable crates, because the `run_make_support` helpers tend to halt the test as soon as something goes wrong. That's unfortunate, but I think it won't matter much in practice, and preserving the old behaviour doesn't seem worth the extra effort.

---

Part of rust-lang#110479 (Python purge), with this being one of the non-trivial Python scripts that actually seems feasible and worthwhile to remove.

This is *not* part of rust-lang#121876 (Makefile purge), because the underlying test is already using rmake; this PR just modifies the existing rmake recipe to do all the work itself instead of delegating to Python. So there's no particular urgency here.

r? ``@jieyouxu``

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: i686-mingw
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
Port the `sysroot-crates-are-unstable` Python script to rmake

New version of rust-lang#126231, and a follow-up to rust-lang#129071.

One major difference is that the new version no longer tries to report *all* accidentally-stable crates, because the `run_make_support` helpers tend to halt the test as soon as something goes wrong. That's unfortunate, but I think it won't matter much in practice, and preserving the old behaviour doesn't seem worth the extra effort.

---

Part of rust-lang#110479 (Python purge), with this being one of the non-trivial Python scripts that actually seems feasible and worthwhile to remove.

This is *not* part of rust-lang#121876 (Makefile purge), because the underlying test is already using rmake; this PR just modifies the existing rmake recipe to do all the work itself instead of delegating to Python. So there's no particular urgency here.

r? ```@jieyouxu```

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: i686-mingw
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
Port the `sysroot-crates-are-unstable` Python script to rmake

New version of rust-lang#126231, and a follow-up to rust-lang#129071.

One major difference is that the new version no longer tries to report *all* accidentally-stable crates, because the `run_make_support` helpers tend to halt the test as soon as something goes wrong. That's unfortunate, but I think it won't matter much in practice, and preserving the old behaviour doesn't seem worth the extra effort.

---

Part of rust-lang#110479 (Python purge), with this being one of the non-trivial Python scripts that actually seems feasible and worthwhile to remove.

This is *not* part of rust-lang#121876 (Makefile purge), because the underlying test is already using rmake; this PR just modifies the existing rmake recipe to do all the work itself instead of delegating to Python. So there's no particular urgency here.

r? ````@jieyouxu````

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: i686-mingw
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2024
Rollup merge of rust-lang#129111 - Zalathar:python-sysroot, r=jieyouxu

Port the `sysroot-crates-are-unstable` Python script to rmake

New version of rust-lang#126231, and a follow-up to rust-lang#129071.

One major difference is that the new version no longer tries to report *all* accidentally-stable crates, because the `run_make_support` helpers tend to halt the test as soon as something goes wrong. That's unfortunate, but I think it won't matter much in practice, and preserving the old behaviour doesn't seem worth the extra effort.

---

Part of rust-lang#110479 (Python purge), with this being one of the non-trivial Python scripts that actually seems feasible and worthwhile to remove.

This is *not* part of rust-lang#121876 (Makefile purge), because the underlying test is already using rmake; this PR just modifies the existing rmake recipe to do all the work itself instead of delegating to Python. So there's no particular urgency here.

r? ````@jieyouxu````

try-job: aarch64-gnu
try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: i686-mingw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs 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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants