Skip to content

modifed y.sh to allow for running cargo tests. #669

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

modifed y.sh to allow for running cargo tests. #669

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion build_system/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn get_runners() -> Runners {
);
runners.insert("--extended-regex-tests", ("Run extended regex tests", extended_regex_tests));
runners.insert("--mini-tests", ("Run mini tests", mini_tests));

runners.insert("--cargo-tests", ("Run cargo tests", cargo_tests));
runners
}

Expand Down Expand Up @@ -88,6 +88,8 @@ struct TestArg {
use_system_gcc: bool,
runners: Vec<String>,
flags: Vec<String>,
/// Additonal arguments, to be passed to commands like `cargo test`.
test_args: Vec<String>,
nb_parts: Option<usize>,
current_part: Option<usize>,
sysroot_panic_abort: bool,
Expand Down Expand Up @@ -144,6 +146,7 @@ impl TestArg {
show_usage();
return Ok(None);
}
"--" => test_arg.test_args.extend(&mut args),
x if runners.contains_key(x)
&& !test_arg.runners.iter().any(|runner| runner == x) =>
{
Expand Down Expand Up @@ -203,6 +206,21 @@ fn clean(_env: &Env, args: &TestArg) -> Result<(), String> {
create_dir(&path)
}

fn cargo_tests(test_env: &Env, test_args: &TestArg) -> Result<(), String> {
Copy link
Contributor

Choose a reason for hiding this comment

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

The following is not necessary before merging this PR, but would be a nice to have:
currently, this seems to always recompile rustc_codegen_gcc and the tests/lang_tests_*.rs stuff, even when there are no changes.
Do you know what causes this?
Would that be easy to fix?
Maybe that's because we have some environment variables set that affect the build, which makes it different than the normal build? (It could be worth just extract LD_LIBRARY_PATH and LIBRARY_PATH from test_env and only sending those to run_command_with_output_and_env to see if this is the case and fixes this issue).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly, that is unlikely to fix this, at least from my (rather amateur) analysis of the cargo logs.

(One of) the rebuild reasons is this:

fingerprint dirty for rustc_codegen_gcc v0.1.0 
cargo::core::compiler::fingerprint:     dirty: ProfileConfigurationChanged

Now, if it was a flag issue, I'd expect this to be the recompilation reason:

   if self.rustflags != old.rustflags {
            return DirtyReason::RustflagsChanged {
                old: old.rustflags.clone(),
                new: self.rustflags.clone(),
            };
        }

I guess I will have to ask about what ProfileConfigurationChanged means - it is compared as a 64 bit hash in the cargo code, I will have to see what it is a hash of.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, all the other rebuild reasons in the logs only tell me that the tests are getting rebuilt, because cg_gcc got rebuilt.

 target="lang_tests_common"}: cargo::core::compiler::fingerprint:     dirty: FsStatusOutdated(StaleDependency { name: "rustc_codegen_gcc",

Copy link
Contributor

Choose a reason for hiding this comment

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

I tested my idea and it does seem to help.
With it, running --cargo-tests a second time gives:

[BUILD] build system
    Finished `release` profile [optimized] target(s) in 0.01s
GCC path retrieved from `config.toml`. Using `/opt/gcc/lib` as path for libgccjit
    Finished `dev` profile [optimized + debuginfo] target(s) in 0.01s
[…]
    Finished `test` profile [optimized + debuginfo] target(s) in 0.03s
     Running unittests src/lib.rs (target/debug/deps/rustc_codegen_gcc-e4ba054637ab4819)

while without it:

[BUILD] build system
    Finished `release` profile [optimized] target(s) in 0.03s
GCC path retrieved from `config.toml`. Using `/opt/gcc/lib` as path for libgccjit
   Compiling rustc_codegen_gcc v0.1.0 (/home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc)
    Finished `dev` profile [optimized + debuginfo] target(s) in 1.37s
[…]
   Compiling rustc_codegen_gcc v0.1.0 (/home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc)
    Finished `test` profile [optimized + debuginfo] target(s) in 8.51s
     Running unittests src/lib.rs (target/debug/deps/rustc_codegen_gcc-4d02d698ebbfa187)

// First, we call `mini_tests` to build minicore for us. This ensures we are testing with a working `minicore`,
// and that any changes we have made affect `minicore`(since it would get rebuilt).
mini_tests(test_env, test_args)?;
// Then, we copy the env vars from `test_env`
// We don't want to pass things like `RUSTFLAGS`, since they contain the -Zcodegen-backend flag.
// That would force `cg_gcc` to *rebuild itself* and only then run tests, which is undesirable.
let mut env = test_env.clone();
env.remove("RUSTFLAGS");
// Pass all the default args + the user-specified ones.
let mut args: Vec<&dyn AsRef<OsStr>> = vec![&"cargo", &"test"];
args.extend(test_args.test_args.iter().map(|s| s as &dyn AsRef<OsStr>));
run_command_with_output_and_env(&args, None, Some(&env))?;
Ok(())
}
fn mini_tests(env: &Env, args: &TestArg) -> Result<(), String> {
// FIXME: create a function "display_if_not_quiet" or something along the line.
println!("[BUILD] mini_core");
Expand Down Expand Up @@ -1218,6 +1236,7 @@ fn run_all(env: &Env, args: &TestArg) -> Result<(), String> {
test_libcore(env, args)?;
extended_sysroot_tests(env, args)?;
test_rustc(env, args)?;
cargo_tests(env, args)?;
Ok(())
}

Expand Down