Reorganize the `run-make-support` library #127760

jieyouxu · 2024-07-15T12:21:22Z

The run_make_support library has a kitchen sink lib.rs that make discovery/learning very difficult. Let's try to improve that by breaking up lib.rs into smaller more organized modules. This is a precursor to improving the documentation and learnability of the run_make_support library.

Changes

Breakup lib.rs into smaller modules according to functionality
Rename recursive_diff -> assert_dirs_are_equal
Rename one of the read_dir with callback interface as read_dir_entries
Coalesced fs-related stuff onto a fs module, re-exported to tests as rfs
Minor doc improvements / fixes in a few places (I have a follow-up documentation PR planned)

This PR is best reviewed commit-by-commit.

r? @Kobzol (or Mark, or T-compiler or T-bootstrap)

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

jieyouxu · 2024-07-15T12:22:56Z

I have ran ./x doc run-make-support locally, but let's also run some try jobs to be sure about other changes.
@bors try

…=<try> Reorganize the `run-make-support` library The `run_make_support` library has a kitchen sink `lib.rs` that make discovery/learning very difficult. Let's try to improve that by breaking up `lib.rs` into smaller more organized modules. This is a precursor to improving the documentation and learnability of the `run_make_support` library. ### Changes - Breakup `lib.rs` into smaller modules according to functionality - Rename `recursive_diff` -> `assert_recursive_eq` - Minor doc improvements / fixes in a few places (I have a follow-up documentation PR planned) This PR is best reviewed commit-by-commit. r? `@Kobzol` (or Mark, or T-compiler or T-bootstrap) try-job: x86_64-msvc try-job: aarch64-apple try-job: test-various try-job: armhf-gnu try-job: dist-x86_64-linux

bors · 2024-07-15T12:24:33Z

⌛ Trying commit 4769fc9 with merge e8f1965...

jieyouxu · 2024-07-15T12:26:07Z

@rustbot author (while waiting for try job to come back)

jieyouxu · 2024-07-15T12:31:16Z

This will conflict (and is prone to conflicts) with other rmake.rs PRs that touch the support library.
@bors rollup=never p=1

bors · 2024-07-15T14:15:32Z

☀️ Try build successful - checks-actions
Build commit: e8f1965 (e8f1965e243dd906e4b2561e47b263a887f5712f)

jieyouxu · 2024-07-15T14:26:38Z

Try builds passed.
@rustbot ready

fmease · 2024-07-15T16:09:51Z

src/tools/run-make-support/src/artifact_names.rs

+
+/// Construct the binary (executable) name based on the target.
+#[must_use]
+pub fn bin_name(name: &str) -> String {


(preexisting) this could use std::env::consts::EXE_EXTENSION

Thanks for pointing that out 👍, I'll address these in a follow-up PR. I want to keep this PR mostly just moving code around.

fmease · 2024-07-15T16:10:55Z

src/tools/run-make-support/src/artifact_names.rs

+
+/// Construct the dynamic library extension based on the target.
+#[must_use]
+pub fn dynamic_lib_extension() -> &'static str {


(preexisting) this could be replaced with std::env::consts::DLL_EXTENSION

Kobzol

Very nice cleanup! Left a few comments, pretty much all nits.

Btw https://github.com/tummychow/git-absorb might be useful if you want to make changes to so many commits.

src/tools/run-make-support/src/macros.rs

src/tools/run-make-support/src/env_checked.rs

src/tools/run-make-support/src/lib.rs

src/tools/run-make-support/src/path_helpers.rs

src/tools/run-make-support/src/fs_helpers.rs

src/tools/run-make-support/src/assertion_helpers.rs

src/tools/run-make-support/src/lib.rs

src/tools/run-make-support/src/assertion_helpers.rs

…dule

…scoped_run` module

jieyouxu · 2024-07-17T13:36:03Z

Going to re-run try jobs (I think force push cancels them?)
@bors try

bors · 2024-07-17T13:37:15Z

⌛ Trying commit d69cc1c with merge 1464be5...

…=<try> Reorganize the `run-make-support` library The `run_make_support` library has a kitchen sink `lib.rs` that make discovery/learning very difficult. Let's try to improve that by breaking up `lib.rs` into smaller more organized modules. This is a precursor to improving the documentation and learnability of the `run_make_support` library. ### Changes - Breakup `lib.rs` into smaller modules according to functionality - Rename `recursive_diff` -> `assert_recursive_eq` - Minor doc improvements / fixes in a few places (I have a follow-up documentation PR planned) This PR is best reviewed commit-by-commit. r? `@Kobzol` (or Mark, or T-compiler or T-bootstrap) try-job: x86_64-msvc try-job: aarch64-apple try-job: test-various try-job: armhf-gnu try-job: dist-x86_64-linux

Kobzol · 2024-07-17T13:38:05Z

I think force push cancels them?

It doesn't, we currently allow parallel try builds on PRs.

bors · 2024-07-17T15:28:06Z

☀️ Try build successful - checks-actions
Build commit: 1464be5 (1464be56c803c09ec9575ff8db7d1a83020da727)

jieyouxu · 2024-07-17T15:31:24Z

@bors r=@Kobzol

bors · 2024-07-17T15:31:27Z

📌 Commit d69cc1c has been approved by Kobzol

It is now in the queue for this repository.

Kobzol · 2024-07-17T15:31:36Z

(RIP all the open run-make PRs)

bors · 2024-07-17T15:52:06Z

⌛ Testing commit d69cc1c with merge f00f850...

bors · 2024-07-17T18:18:03Z

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing f00f850 to master...

rust-timer · 2024-07-17T20:28:23Z

Finished benchmarking commit (f00f850): 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

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

Binary size

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

Bootstrap: 769.855s -> 768.932s (-0.12%)
Artifact size: 328.79 MiB -> 328.74 MiB (-0.01%)

Cleanup dll/exe filename calculations in `run_make_support` Use `std::env::consts` constants since now we have access to them (unlike in Makefiles!) ^^ cc `@bzEq` (this is one of the places in our test suites that tries to compute e.g. dylib extension; using `std::env::consts::DLL_EXTENSION` should correctly return `a` for AIX) r? `@fmease` (thank you for the suggestion in rust-lang#127760 (comment), this also improves correctness for the support library!) try-job: aarch64-apple try-job: armhf-gnu try-job: test-various try-job: x86_64-msvc try-job: x86_64-gnu-llvm-18

…ease Cleanup dll/exe filename calculations in `run_make_support` Use `std::env::consts` constants since now we have access to them (unlike in Makefiles!) ^^ cc `@bzEq` (this is one of the places in our test suites that tries to compute e.g. dylib extension; using `std::env::consts::DLL_EXTENSION` should correctly return `a` for AIX) r? `@fmease` (thank you for the suggestion in rust-lang#127760 (comment), this also improves correctness for the support library!) try-job: aarch64-apple try-job: armhf-gnu try-job: test-various try-job: x86_64-msvc try-job: x86_64-gnu-llvm-18

Rollup merge of rust-lang#127960 - jieyouxu:minor-rmake-cleanup, r=fmease Cleanup dll/exe filename calculations in `run_make_support` Use `std::env::consts` constants since now we have access to them (unlike in Makefiles!) ^^ cc `@bzEq` (this is one of the places in our test suites that tries to compute e.g. dylib extension; using `std::env::consts::DLL_EXTENSION` should correctly return `a` for AIX) r? `@fmease` (thank you for the suggestion in rust-lang#127760 (comment), this also improves correctness for the support library!) try-job: aarch64-apple try-job: armhf-gnu try-job: test-various try-job: x86_64-msvc try-job: x86_64-gnu-llvm-18

Cleanup dll/exe filename calculations in `run_make_support` Use `std::env::consts` constants since now we have access to them (unlike in Makefiles!) ^^ cc `@bzEq` (this is one of the places in our test suites that tries to compute e.g. dylib extension; using `std::env::consts::DLL_EXTENSION` should correctly return `a` for AIX) r? `@fmease` (thank you for the suggestion in rust-lang/rust#127760 (comment), this also improves correctness for the support library!) try-job: aarch64-apple try-job: armhf-gnu try-job: test-various try-job: x86_64-msvc try-job: x86_64-gnu-llvm-18

rustbot assigned Kobzol Jul 15, 2024

rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 15, 2024

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 Jul 15, 2024

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 Jul 15, 2024

fmease reviewed Jul 15, 2024

View reviewed changes

Kobzol reviewed Jul 17, 2024

View reviewed changes

jieyouxu added 14 commits July 17, 2024 12:48

run_make_support: move impl_common_helpers into own macros module

427cf94

run_make_support: move target checks into targets module

544dda3

run_make_support: move env_var{,_os} into env_checked module

a02008e

run_make_support: move external deps to external_deps module

288c572

run_make_support: cleanup and document some lib.rs reexports

f042e72

run_make_support: move ar into own module

439c6f6

run_make_support: move artifact name helpers into artifact_names mo…

483328d

…dule

run_make_support: move path-related helpers into own module

dc95315

run_make_support: move fs helpers to own module

17212ab

run_make_support: move run_in_tmpdir and test_while_readonly to `…

66cef19

…scoped_run` module

run_make_support: move assertions and helpers into own module

f66d3d3

run_make_support: rename recursive_diff to assert_recursive_eq

230804d

tests: update rustdoc test for renamed assert_recursive_eq

56cbfa8

run_make_support: move assert_recursive_eq into assertion_helpers

88fd1df

This was referenced Jul 17, 2024

Migrate macos-fat-archive, manual-link and archive-duplicate-names run-make tests to rmake #127825

Merged

Migrate dump-ice-to-disk and panic-abort-eh_frame run-make tests to rmake #127523

Merged

This was referenced Jul 17, 2024

Migrate 8 very similar FFI run-make tests to rmake #127491

Merged

Migrate 9 more very similar FFI run-make tests to rmake #127663

Merged

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 17, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 17, 2024

bors merged commit f00f850 into rust-lang:master Jul 17, 2024
7 checks passed

rustbot added this to the 1.81.0 milestone Jul 17, 2024

jieyouxu deleted the rmake-support-reorganize branch July 17, 2024 18:21

jieyouxu mentioned this pull request Jul 19, 2024

Cleanup dll/exe filename calculations in run_make_support #127960

Merged

jieyouxu mentioned this pull request Jul 19, 2024

Use #[rustfmt::skip] on some use groups to prevent reordering. #127950

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize the `run-make-support` library #127760

Reorganize the `run-make-support` library #127760

jieyouxu commented Jul 15, 2024 •

edited

Loading

jieyouxu commented Jul 15, 2024 •

edited

Loading

bors commented Jul 15, 2024

jieyouxu commented Jul 15, 2024

jieyouxu commented Jul 15, 2024 •

edited

Loading

bors commented Jul 15, 2024

jieyouxu commented Jul 15, 2024

fmease Jul 15, 2024

jieyouxu Jul 15, 2024

fmease Jul 15, 2024

Kobzol left a comment

jieyouxu commented Jul 17, 2024

bors commented Jul 17, 2024

Kobzol commented Jul 17, 2024

bors commented Jul 17, 2024

jieyouxu commented Jul 17, 2024

bors commented Jul 17, 2024

Kobzol commented Jul 17, 2024

bors commented Jul 17, 2024

bors commented Jul 17, 2024

rust-timer commented Jul 17, 2024

Reorganize the run-make-support library #127760

Reorganize the run-make-support library #127760

Conversation

jieyouxu commented Jul 15, 2024 • edited Loading

Changes

jieyouxu commented Jul 15, 2024 • edited Loading

bors commented Jul 15, 2024

jieyouxu commented Jul 15, 2024

jieyouxu commented Jul 15, 2024 • edited Loading

bors commented Jul 15, 2024

jieyouxu commented Jul 15, 2024

fmease Jul 15, 2024

Choose a reason for hiding this comment

jieyouxu Jul 15, 2024

Choose a reason for hiding this comment

fmease Jul 15, 2024

Choose a reason for hiding this comment

Kobzol left a comment

Choose a reason for hiding this comment

jieyouxu commented Jul 17, 2024

bors commented Jul 17, 2024

Kobzol commented Jul 17, 2024

bors commented Jul 17, 2024

jieyouxu commented Jul 17, 2024

bors commented Jul 17, 2024

Kobzol commented Jul 17, 2024

bors commented Jul 17, 2024

bors commented Jul 17, 2024

rust-timer commented Jul 17, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Reorganize the `run-make-support` library #127760

Reorganize the `run-make-support` library #127760

jieyouxu commented Jul 15, 2024 •

edited

Loading

jieyouxu commented Jul 15, 2024 •

edited

Loading

jieyouxu commented Jul 15, 2024 •

edited

Loading