Migrate `reproducible-build` `run-make` test to rmake by Oneirical · Pull Request #128456 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation89 Commits2 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

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

This will likely fail. Locally, rustc errors with linker 'linker' not found on line 36 while the file exists according to the dir-debug statement before it.

If this gets fixed and the test passes, further developments may include:

There may be some leftovers from each test - test_in_tmpdir may therefore be required.
Try jobs on all ignored architectures.
A potential refactor with a struct and a custom function like Migrate remap-path-prefix-dwarf run-make test to rmake #128410 so this isn't just a huge stream of rfs and rustc. This is a little bit harder to do in this test considering the variability present in each test case.

// try-job: x86_64-msvc // windows jobs passed in a prior run
// try-job: x86_64-mingw
// try-job: i686-msvc
// try-job: i686-mingw
try-job: aarch64-apple
try-job: aarch64-gnu
try-job: armhf-gnu
try-job: test-various
try-job: dist-various-1

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 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

Jul 31, 2024

This comment has been minimized.

Note to self: look at linker not found

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

Aug 3, 2024

Looks like the linker problem got fixed, but now, there are some issues with the bin copying on this test:

run_in_tmpdir(|| {
    rustc().input("reproducible-build-aux.rs").run();
    rfs::create_dir("test");
    rfs::copy("reproducible-build.rs", "test/reproducible-build.rs");
    rustc()
        .input("reproducible-build.rs")
        .crate_type("bin")
        .arg(&format!("--remap-path-prefix={}=/b", cwd().display()))
        .run();
    eprintln!("{:#?}", rfs::shallow_find_dir_entries(cwd()));
    rfs::copy(bin_name("reproducible_build"), bin_name("foo"));
    rustc()
        .input("test/reproducible-build.rs")
        .crate_type("bin")
        .arg("--remap-path-prefix=/test=/b")
        .run();
    assert_eq!(rfs::read(bin_name("reproducible_build")), rfs::read(bin_name("foo")));
});

I think it might be assuming that the bins are directories due to the lack of extension. I tried manually adding extensions with .output, but no luck, it simply causes the final assertion to fail. I'm letting this one sit for a bit.

This comment has been minimized.

I think it might be assuming that the bins are directories due to the lack of extension. I tried manually adding extensions with .output, but no luck, it simply causes the final assertion to fail. I'm letting this one sit for a bit.

Oh this one is easy. rustc performs hyphen normalization for library names, i.e. reproducible-build.rs hyphen-normalizes to libreproducible_build.rlib or whatever unless you specify output artifact names. But bin names are not hyphen-normalized, so it's actually bin_name("reproducible-build") not bin_name("reproducible_build").

Alright, I have done a big cleanup. This should be a lot nicer to look at. Note this line:

// FIXME(Oneirical): this specific case fails the final assertion. // diff_dir_test(CrateType::Bin, RemapType::Cwd { is_empty: false });

In the original test:

TODO: Builds of `bin` crate types are not deterministic with debuginfo=2 on

Windows.

See: https://github.com/rust-lang/rust/pull/87320#issuecomment-920105533

Issue: https://github.com/rust-lang/rust/issues/88982

different_source_dirs_bin \

remap_cwd_bin \

Note that despite the comment mentioning a Windows-specific problem, remap_cwd_bin fails on Linux too, and this part of the test was getting ignored on all architectures, since it's commented out. It's possible that it never worked at all.

As for different_source_dirs_bin, it succeeds, but it might fail on a different architecture. Some testing required. I'd wager it's because the original test didn't check for .exe extensions, though.

Feel free to run try jobs after CI is green.

@rustbot review

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

Aug 6, 2024

Oneirical marked this pull request as ready for review

August 6, 2024 16:44

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

Note that despite the comment mentioning a Windows-specific problem, remap_cwd_bin fails on Linux too, and this part of the test was getting ignored on all architectures, since it's commented out. It's possible that it never worked at all.

That seems like the case. If I drop the -g a.k.a. -C debuginfo=2 flag, then remap_cwd_bin passes on linux.

Choose a reason for hiding this comment

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

Thanks, this is now much easier to follow!

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

Aug 7, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 7, 2024

…

Migrate reproducible-build run-make test to rmake

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

This will likely fail. Locally, rustc errors with linker 'linker' not found on line 36 while the file exists according to the dir-debug statement before it.

If this gets fixed and the test passes, further developments may include:

There may be some leftovers from each test - test_in_tmpdir may therefore be required.
Try jobs on all ignored architectures.
A potential refactor with a struct and a custom function like rust-lang#128410 so this isn't just a huge stream of rfs and rustc. This is a little bit harder to do in this test considering the variability present in each test case.

try-job: aarch64-apple try-job: test-various try-job: armhf-gnu try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw try-job: x86_64-gnu-llvm-17 try-job: dist-various-1

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

Aug 15, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 15, 2024

…

Migrate reproducible-build run-make test to rmake

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

This will likely fail. Locally, rustc errors with linker 'linker' not found on line 36 while the file exists according to the dir-debug statement before it.

If this gets fixed and the test passes, further developments may include:

There may be some leftovers from each test - test_in_tmpdir may therefore be required.
Try jobs on all ignored architectures.
A potential refactor with a struct and a custom function like rust-lang#128410 so this isn't just a huge stream of rfs and rustc. This is a little bit harder to do in this test considering the variability present in each test case.

// try-job: aarch64-apple // try-job: aarch64-gnu // try-job: armhf-gnu // try-job: test-various // try-job: dist-various-1 try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw

☀️ Try build successful - checks-actions
Build commit: f3c4813 (f3c48134e356f29e7db8f2d40855e48d467749f1)

Choose a reason for hiding this comment

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

Thanks! A nit for an outdated FIXME, then r=me after a rebase and running the second batch of non-Windows try-jobs and if they come back green.

Comment on lines 22 to 27

// FIXME(Oneirical): ignore-windows
// # FIXME: Builds of `bin` crate types are not deterministic with debuginfo=2 on
// # Windows.
// # See: https://github.com/rust-lang/rust/pull/87320#issuecomment-920105533
// # Issue: https://github.com/rust-lang/rust/issues/88982

Choose a reason for hiding this comment

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

Suggestion: this comment is moved to specific cases, we should remove this top-level ignore-windows FIXME comment.

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

Aug 16, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 16, 2024

…

Migrate reproducible-build run-make test to rmake

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

This will likely fail. Locally, rustc errors with linker 'linker' not found on line 36 while the file exists according to the dir-debug statement before it.

If this gets fixed and the test passes, further developments may include:

There may be some leftovers from each test - test_in_tmpdir may therefore be required.
Try jobs on all ignored architectures.
A potential refactor with a struct and a custom function like rust-lang#128410 so this isn't just a huge stream of rfs and rustc. This is a little bit harder to do in this test considering the variability present in each test case.

// try-job: x86_64-msvc // windows jobs passed in a prior run // try-job: x86_64-mingw // try-job: i686-msvc // try-job: i686-mingw try-job: aarch64-apple try-job: aarch64-gnu try-job: armhf-gnu try-job: test-various try-job: dist-various-1

☀️ Try build successful - checks-actions
Build commit: 3d3742d (3d3742d2207d10c64423b2425b4840ef23eb55c4)

📌 Commit e752410 has been approved by jieyouxu

It is now in the queue for this repository.

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

Aug 16, 2024

@bors rollup=iffy (reproducible-build isn't always reproducible)

bors mentioned this pull request

Aug 16, 2024

Finished benchmarking commit (569d7e3): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-1.8%, -0.2%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 2.5%, secondary 0.8%)

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)	2.5%	[2.5%, 2.5%]	1
Regressions ❌ (secondary)	1.7%	[1.1%, 2.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.8%	[-0.8%, -0.8%]	1
All ❌✅ (primary)	2.5%	[2.5%, 2.5%]	1

Cycles

Results (secondary -3.2%)

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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.2%	[-4.5%, -2.1%]	14
All ❌✅ (primary)	-	-	0