Skip to content

[COMPILETEST-UNTANGLE 4/N] Improve compiletest config documentation #143447

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
Merged
merged 1 commit into from
Jul 5, 2025
Merged

[COMPILETEST-UNTANGLE 4/N] Improve compiletest config documentation #143447

merged 1 commit into from
Jul 5, 2025
+353 −93

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Jul 4, 2025 •
edited
Loading

This is part of a patch series to untangle compiletest to hopefully nudge it towards being more maintainable.

This PR should contain no functional changes.

This is pulled out to its own PR to make follow-up changes easier to review.

There are intentionally a lot of FIXME comments, intended to be gradually addressed in follow-ups.

r? @Kobzol

@rustbot rustbot added A-compiletest Area: The compiletest test runner 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 4, 2025
jieyouxu
jieyouxu commented Jul 4, 2025
View reviewed changes
src/tools/compiletest/src/debuggers.rs
@@ -51,6 +51,7 @@ pub(crate) fn configure_gdb(config: &Config) -> Option<Arc<Config>> {
pub(crate) fn configure_lldb(config: &Config) -> Option<Arc<Config>> {
config.lldb_python_dir.as_ref()?;

// FIXME: this is super old
Copy link
Member Author

Choose a reason for hiding this comment

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

Remark: intended to remove this warning in a follow-up.

Kobzol
Kobzol reviewed Jul 4, 2025
View reviewed changes
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Left one question, otherwise you can r=me.

Good luck with untangling this =D

@bors rollup

src/tools/compiletest/src/common.rs
pub compile_lib_path: Utf8PathBuf,

/// The library paths required for running compiled programs.
/// Path to libraries needed to run the compiled executable for the **target** platform. This
Copy link
Member

Choose a reason for hiding this comment

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

If we actually run any binaries compiled by compiletest, they will need to run on the host, right? So these should be host runtime libraries (i.e. stdlib), or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question, let me double-check.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe this is actually accurate:

  • The host == target case is trivial.
  • When host != target, there are several ways for compiletest to actually run the target platform executable under the target platform environment:
    • The remote-test-client scheme (generally, and for run-make target executables): e.g. Reintroduce remote-test support in run-make tests #138652.

      rust/src/tools/compiletest/src/runtest.rs

      Lines 1055 to 1070 in 733b47e

      let proc_res = match &*self.config.target {
      // This is pretty similar to below, we're transforming:
      //
      // program arg1 arg2
      //
      // into
      //
      // remote-test-client run program 2 support-lib.so support-lib2.so arg1 arg2
      //
      // The test-client program will upload `program` to the emulator
      // along with all other support libraries listed (in this case
      // `support-lib.so` and `support-lib2.so`. It will then execute
      // the program on the emulator with the arguments specified
      // (in the environment we give the process) and then report back
      // the same result.
      _ if self.config.remote_test_client.is_some() => {
    • The RUNNER scheme (for run-make target executables): forward the bootstrap runner to run-make #141856

Copy link
Member Author

Choose a reason for hiding this comment

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

That is, these are in fact target runtime libraries (e.g. target std), and not host std or host runtime libraries.

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed an update to include a FIXME to better document these schemes, as I had to figure that out from the implementation as well.

https://github.com/rust-lang/rust/compare/e330c9c2613cabda20afa2db3bce118b7209db3b..7405e2a9154268955de216840ef4dd7e975b0cd5

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok, makes sense. Feel free to r=me then.

@jieyouxu
Improve compiletest config documentation
7405e2a 
Including a bunch of FIXMEs.
@jieyouxu jieyouxu force-pushed the compiletest-maintenance-4 branch from e330c9c to 7405e2a Compare July 5, 2025 07:20
@jieyouxu
Copy link
Member Author

jieyouxu commented Jul 5, 2025

@bors r=Kobzol rollup

@bors
Copy link
Collaborator

bors commented Jul 5, 2025

📌 Commit 7405e2a has been approved by Kobzol

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 5, 2025
@saethlin saethlin mentioned this pull request Jul 5, 2025
Merged
bors added a commit that referenced this pull request Jul 5, 2025
@bors
Auto merge of #143496 - saethlin:rollup-4u43rgj, r=saethlin
175e043 
Rollup of 3 pull requests

Successful merges:

 - #143291 (codegen_ssa: replace a Result by an Either)
 - #143445 (move `va_copy`, `va_arg` and `va_end` to `core::intrinsics`)
 - #143447 ([COMPILETEST-UNTANGLE 4/N] Improve compiletest config documentation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 53757e6 into rust-lang:master Jul 5, 2025
11 checks passed
rust-timer added a commit that referenced this pull request Jul 5, 2025
@rust-timer
Unrolled build for #143447
312b13a 
Rollup merge of #143447 - jieyouxu:compiletest-maintenance-4, r=Kobzol

[COMPILETEST-UNTANGLE 4/N] Improve compiletest config documentation

This is part of a patch series to untangle `compiletest` to hopefully nudge it towards being more maintainable.

This PR should contain **no functional changes**.

This is pulled out to its own PR to make follow-up changes easier to review.

There are *intentionally* a *lot* of FIXME comments, intended to be gradually addressed in follow-ups.

r? `@Kobzol`
@rustbot rustbot added this to the 1.90.0 milestone Jul 5, 2025
@jieyouxu jieyouxu deleted the compiletest-maintenance-4 branch July 6, 2025 02:16
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

Assignees

@Kobzol Kobzol

Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc 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)
Projects
None yet
Milestone
1.90.0
Development

Successfully merging this pull request may close these issues.
4 participants
@jieyouxu @bors @Kobzol @rustbot