Skip to content

Commit 1b4972b

Browse files
committed
compiletest/rmake: adjust docs for rmake.rs setup and add FIXMEs
1 parent 11e5724 commit 1b4972b

File tree

1 file changed

+27
-7
lines changed

1 file changed

+27
-7
lines changed

src/tools/compiletest/src/runtest.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3433,14 +3433,19 @@ impl<'test> TestCx<'test> {
34333433

34343434
fn run_rmake_v2_test(&self) {
34353435
// For `run-make` V2, we need to perform 2 steps to build and run a `run-make` V2 recipe
3436-
// (`rmake.rs`) to run the actual tests. The support library is already built as a tool
3437-
// dylib and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`.
3436+
// (`rmake.rs`) to run the actual tests. The support library is already built as a tool rust
3437+
// library and is available under `build/$TARGET/stageN-tools-bin/librun_make_support.rlib`.
34383438
//
3439-
// 1. We need to build the recipe `rmake.rs` and link in the support library.
3440-
// 2. We need to run the recipe to build and run the tests.
3439+
// 1. We need to build the recipe `rmake.rs` as a binary and link in the `run_make_support`
3440+
// library.
3441+
// 2. We need to run the recipe binary.
3442+
3443+
// FIXME(jieyouxu): hm, cwd doesn't look right here?
34413444
let cwd = env::current_dir().unwrap();
3445+
// FIXME(jieyouxu): is there a better way to get `src_root`?
34423446
let src_root = self.config.src_base.parent().unwrap().parent().unwrap();
34433447
let src_root = cwd.join(&src_root);
3448+
// FIXME(jieyouxu): is there a better way to get `build_root`?
34443449
let build_root = self.config.build_base.parent().unwrap().parent().unwrap();
34453450
let build_root = cwd.join(&build_root);
34463451

@@ -3450,11 +3455,13 @@ impl<'test> TestCx<'test> {
34503455
// rmake.exe
34513456
// rmake_out/
34523457
// ```
3453-
// having the executable separate from the output artifacts directory allows the recipes to
3454-
// `remove_dir_all($TMPDIR)` without running into permission denied issues because
3455-
// the executable is not under the `rmake_out/` directory.
3458+
// having the recipe executable separate from the output artifacts directory allows the
3459+
// recipes to `remove_dir_all($TMPDIR)` without running into issues related trying to remove
3460+
// a currently running executable because the recipe executable is not under the
3461+
// `rmake_out/` directory.
34563462
//
34573463
// This setup intentionally diverges from legacy Makefile run-make tests.
3464+
// FIXME(jieyouxu): is there a better way to compute `base_dir`?
34583465
let base_dir = cwd.join(self.output_base_name());
34593466
if base_dir.exists() {
34603467
self.aggressive_rm_rf(&base_dir).unwrap();
@@ -3477,10 +3484,13 @@ impl<'test> TestCx<'test> {
34773484
}
34783485
}
34793486

3487+
// FIXME(jieyouxu): is there a better way to get the stage number or otherwise compute the
3488+
// required stage-specific build directories?
34803489
// HACK: assume stageN-target, we only want stageN.
34813490
let stage = self.config.stage_id.split('-').next().unwrap();
34823491

34833492
// First, we construct the path to the built support library.
3493+
// FIXME(jieyouxu): explain what the hecc we are doing here.
34843494
let mut support_lib_path = PathBuf::new();
34853495
support_lib_path.push(&build_root);
34863496
support_lib_path.push(format!("{}-tools-bin", stage));
@@ -3492,19 +3502,22 @@ impl<'test> TestCx<'test> {
34923502
stage_std_path.push("lib");
34933503

34943504
// Then, we need to build the recipe `rmake.rs` and link in the support library.
3505+
// FIXME(jieyouxu): use `std::env::consts::EXE_EXTENSION`.
34953506
let recipe_bin = base_dir.join(if self.config.target.contains("windows") {
34963507
"rmake.exe"
34973508
} else {
34983509
"rmake"
34993510
});
35003511

3512+
// FIXME(jieyouxu): explain what the hecc we are doing here.
35013513
let mut support_lib_deps = PathBuf::new();
35023514
support_lib_deps.push(&build_root);
35033515
support_lib_deps.push(format!("{}-tools", stage));
35043516
support_lib_deps.push(&self.config.host);
35053517
support_lib_deps.push("release");
35063518
support_lib_deps.push("deps");
35073519

3520+
// FIXME(jieyouxu): explain what the hecc we are doing here.
35083521
let mut support_lib_deps_deps = PathBuf::new();
35093522
support_lib_deps_deps.push(&build_root);
35103523
support_lib_deps_deps.push(format!("{}-tools", stage));
@@ -3514,6 +3527,7 @@ impl<'test> TestCx<'test> {
35143527
debug!(?support_lib_deps);
35153528
debug!(?support_lib_deps_deps);
35163529

3530+
// FIXME(jieyouxu): explain what the hecc we are doing here.
35173531
let orig_dylib_env_paths =
35183532
Vec::from_iter(env::split_paths(&env::var(dylib_env_var()).unwrap()));
35193533

@@ -3522,6 +3536,8 @@ impl<'test> TestCx<'test> {
35223536
host_dylib_env_paths.extend(orig_dylib_env_paths.iter().cloned());
35233537
let host_dylib_env_paths = env::join_paths(host_dylib_env_paths).unwrap();
35243538

3539+
// FIXME(jieyouxu): explain what the hecc we are doing here.
3540+
// FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc!
35253541
let mut cmd = Command::new(&self.config.rustc_path);
35263542
cmd.arg("-o")
35273543
.arg(&recipe_bin)
@@ -3563,16 +3579,20 @@ impl<'test> TestCx<'test> {
35633579
// Finally, we need to run the recipe binary to build and run the actual tests.
35643580
debug!(?recipe_bin);
35653581

3582+
// FIXME(jieyouxu): explain what the hecc we are doing here.
35663583
let mut dylib_env_paths = orig_dylib_env_paths.clone();
35673584
dylib_env_paths.push(support_lib_path.parent().unwrap().to_path_buf());
35683585
dylib_env_paths.push(stage_std_path.join("rustlib").join(&self.config.host).join("lib"));
35693586
let dylib_env_paths = env::join_paths(dylib_env_paths).unwrap();
35703587

3588+
// FIXME(jieyouxu): explain what the hecc we are doing here.
35713589
let mut target_rpath_env_path = Vec::new();
35723590
target_rpath_env_path.push(&rmake_out_dir);
35733591
target_rpath_env_path.extend(&orig_dylib_env_paths);
35743592
let target_rpath_env_path = env::join_paths(target_rpath_env_path).unwrap();
35753593

3594+
// FIXME(jieyouxu): explain what the hecc we are doing here.
3595+
// FIXME(jieyouxu): audit these env vars. some of them only makes sense for make, not rustc!
35763596
let mut cmd = Command::new(&recipe_bin);
35773597
cmd.current_dir(&rmake_out_dir)
35783598
.stdout(Stdio::piped())

0 commit comments

Comments
 (0)