Don't use process::exit in bootstrap tests

[jyn514]

Bootstrap has a lot of places where it calls process::exit. This makes it hard to figure out why tests fail. We should use unique exit codes so it's easier to tell where something wrong (and maybe consider using resume_unwind instead? not sure).

$ rg exit'\(' src/bootstrap/*.rs
src/bootstrap/util.rs
338:        std::process::exit(1);
377:        std::process::exit(1);
467:    std::process::exit(1);

src/bootstrap/toolstate.rs
96:    std::process::exit(3);
111:            std::process::exit(1);
182:            std::process::exit(1);
228:            std::process::exit(1);

src/bootstrap/tool.rs
207:                exit(1);

src/bootstrap/setup.rs
97:        std::process::exit(1);
290:            std::process::exit(1);

src/bootstrap/sanity.rs
107:            std::process::exit(1);

src/bootstrap/test.rs
676:            std::process::exit(1);
1024:                std::process::exit(1);
1254:            std::process::exit(1);

src/bootstrap/lib.rs
719:            process::exit(1);
1617:                std::process::exit(1);

src/bootstrap/format.rs
35:            std::process::exit(1);
117:        std::process::exit(1);

src/bootstrap/flags.rs
257:                process::exit(exit_code);
343:            process::exit(exit_code);
375:            process::exit(1);
596:                        std::process::exit(1);
610:                process::exit(1);
800:            process::exit(1);

src/bootstrap/compile.rs
1380:        exit(1);

src/bootstrap/config.rs
820:                    process::exit(2);
1490:        exit(1);

src/bootstrap/builder.rs
350:            std::process::exit(1);
1010:            std::process::exit(1);
1439:                    std::process::exit(1);

[Mark-Simulacrum]

I don't think unique exit codes are a good idea -- too much hassle; we should probably replace the exit's with a call to something like crate::exit which can panic! in cfg(test) and resume_unwind in regular code; the goal of the exits is really just to more cleanly exit than the big panic message you get otherwise.

[kons-9]

@rustbot claim

[kons-9]

Thank you for mentering!
From the above exchange, I thought I could write code like this.

// exit.rs
fn detail_exit(message:String){
    if (test) {
        panic!(message)
    } else (not test) {
        resume_unwind(Box::new(message))
    }
}

Is this understanding correct or incorrect?
(I've never used resume_unwind that way, so I'm not sure. but it can stop running without panic message like std::process::exit and if needed, we can get imformation about error message.)

[jyn514]

@kons-9 exit is fine to keep using when we're not running tests. Otherwise that looks broadly correct. The magic built-in to replace (test) with is cfg!(test): https://doc.rust-lang.org/stable/std/macro.cfg.html

I would put this in lib.rs or utils.rs, no need to add a new file just for one function.

[jyn514]

Ah, I missed that Mark suggested resume_unwind earlier - ok, that's fine to use, it will make sure tempfiles are deleted and that sort of thing.

[kons-9]

Thank you for your reply!

no need to add a new file just for one function.

Copy that!

I am going to use #[cfg(test)] in that code, but would cfg!(test) be better? (you might have suggested this macro because of my code. )

[jyn514]

@kons-9 I think cfg!(test) is better so you don't have to run x test src/bootstrap before you get type errors.