Skip to content

Commit

Permalink
No warning and only on build
Browse files Browse the repository at this point in the history 
- Removing the warning for all the commands besides build
- Cleaning up the three tests
  • Loading branch information
@MBerguer
MBerguer committed Jan 6, 2025
1 parent 44a04dc commit cb3a76a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 135 deletions.
20 changes: 0 additions & 20 deletions crates/cli/src/opts/build/zksync.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,6 @@ pub struct ZkSyncArgs {
)]
pub fallback_oz: Option<bool>,

/// Detect missing libraries, instead of erroring
///
/// Currently unused
#[clap(
long = "zk-detect-missing-libraries-deprecated",
id = "zk-detect-missing-libraries-deprecated",
group = "depracated"
)]
pub detect_missing_libraries: bool,

/// Set the LLVM optimization parameter `-O[0 | 1 | 2 | 3 | s | z]`.
/// Use `3` for best performance and `z` for minimal size.
#[clap(
Expand Down Expand Up @@ -168,16 +158,6 @@ impl ZkSyncArgs {
set_if_some!(self.llvm_options.clone(), zksync.llvm_options);
set_if_some!(self.force_evmla, zksync.force_evmla);
set_if_some!(self.fallback_oz, zksync.fallback_oz);
set_if_some!(
self.detect_missing_libraries.then_some(true),
zksync.detect_missing_libraries
);

if zksync.detect_missing_libraries {
// warn saying that The `detect_missing_libraries` option should not be used in toml
// config, but as an argument in the build command
warn!("Ignoring the `detect_missing_libraries` flag; it should not be used in toml config file, but as an argument in the build command");
}

set_if_some!(self.optimizer.then_some(true), zksync.optimizer);
set_if_some!(
Expand Down
4 changes: 4 additions & 0 deletions crates/forge/bin/cmd/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ impl Provider for BuildArgs {
dict.insert("ignore_eip_3860".to_string(), true.into());
}

if self.detect_missing_libraries {
dict.insert("detect_missing_libraries".to_string(), true.into());
}

Ok(Map::from([(Config::selected_profile(), dict)]))
}
}
129 changes: 14 additions & 115 deletions crates/forge/tests/cli/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,125 +193,24 @@ contract ValidContract {}
cmd.args(["build"]).assert_success();
});

// tests build scenarios varying zk-detect-missing-libraries flag
// TOML config file could be used to set the flag
// also the flag should be set via command line argument
// In any of these cases, it should not build (if the flag is set)
// it should build (if the flag is not set in any of the above cases)
// case 1: [BUILD] flag in both toml config file and command line argument
forgetest_init!(test_zk_build_missing_libraries_config_and_flag, |prj, cmd| {
let zk = ZkSyncConfig { detect_missing_libraries: true, ..Default::default() };
prj.write_config(Config { zksync: zk, ..Default::default() });
cmd.args(["build", "--zksync", "--zk-detect-missing-libraries"])
.assert_success()
// .stderr_eq(str![r#"Ignoring the `detect_missing_libraries` flag; it should not be used in
// toml config file, but as an argument in the build command"#])
.stdout_eq(str![[r#"
...
Compiler run successful with warnings:
...
"#]]);
// .stderr_eq(str![r#"Ignoring the `detect_missing_libraries` flag; it should not be used in
// toml config file, but as an argument in the build command"#]);
});

// scenario 2: [BUILD] flag set via command line argument only (NO WARNINGS; THE RIGHT WAY)
forgetest_init!(test_zk_build_missing_libraries_only_flag, |prj, cmd| {
cmd.args(["build", "--zksync", "--zk-detect-missing-libraries"])
.assert_failure()
.stderr_eq(str![r#"Ignoring the `detect_missing_libraries` flag; it should not be used in toml config file, but as an argument in the build command"#]);
});

// scenario 3: [BUILD] flag set in toml config file only
forgetest_init!(test_zk_build_missing_libraries_only_config, |prj, cmd| {
let zk = ZkSyncConfig { detect_missing_libraries: true, ..Default::default() };
prj.write_config(Config { zksync: zk, ..Default::default() });
cmd.args(["build", "--zksync"])
.assert_failure()
.stderr_eq(str![r#"Ignoring the `detect_missing_libraries` flag; it should not be used in toml config file, but as an argument in the build command"#]);
});

// scenario 4: [BUILD] flag not set in either toml config file or command line argument
forgetest_init!(test_zk_build_missing_libraries_none, |prj, cmd| {
cmd.args(["build", "--zksync"]).assert_success();
forgetest_init!(test_zk_build_missing_libraries_as_arg, |prj, cmd| {
cmd.args(["build", "--zksync", "--zk-detect-missing-libraries"]).assert_success();
});

// scenario 5: [TEST] flag not set in either toml config file or command line argument
forgetest_init!(test_zk_script_missing_libraries_config_and_flag, |prj, cmd| {
forgetest_init!(test_zk_build_missing_libraries_as_config, |prj, cmd| {
let zk = ZkSyncConfig { detect_missing_libraries: true, ..Default::default() };
prj.write_config(Config { zksync: zk, ..Default::default() });
cmd.env("RUST_LOG", "warn");

// cmd.args(["test", "--zksync", "--zk-detect-missing-libraries-deprecated"])
// .assert_failure()
// .stderr_eq(str![[r#"
// ...
// Ignoring the `detect_missing_libraries` flag; it should not be used in toml config file, but
// as an argument in the build command ...
// "#]]);

let output = cmd
.args(["test", "--zksync", "--zk-detect-missing-libraries-deprecated"])
.assert_failure()
.get_output()
.stdout_lossy();
assert!(output.contains("Ignoring the `detect_missing_libraries` flag; it should not be used in toml config file, but as an argument in the build command"), "{}", output);

// println!(
// "{}",
// cmd.args(["test", "--zksync", "--zk-detect-missing-libraries"])
// .assert_success()
// .get_output()
// .stdout_lossy()
// );

// .stderr_eq(str![r#"Ignoring the `detect_missing_libraries` flag; it should not be used in
// toml config file, but as an argument in the build command"#])
// .stdout_eq(str![[r#"
// ...
// Compiler run successful with warnings:
// ...
// "#]]);
// .stderr_eq(str![r#"Ignoring the `detect_missing_libraries` flag; it should not be used in
// toml config file, but as an argument in the build command"#]);
cmd.args(["build", "--zksync", "--zk-detect-missing-libraries"]).assert_success();
});

// scenario 6: [TEST] flag not set in either toml config file or command line argument
forgetest_init!(test_zk_script_missing_libraries_config_and_flag, |prj, cmd| {
let zk = ZkSyncConfig { detect_missing_libraries: true, ..Default::default() };
prj.write_config(Config { zksync: zk, ..Default::default() });
cmd.env("RUST_LOG", "warn");

// cmd.args(["test", "--zksync", "--zk-detect-missing-libraries-deprecated"])
// .assert_failure()
// .stderr_eq(str![[r#"
// ...
// Ignoring the `detect_missing_libraries` flag; it should not be used in toml config file, but
// as an argument in the build command ...
// "#]]);

let output = cmd
.args(["test", "--zksync", "--zk-detect-missing-libraries-deprecated"])
.assert_failure()
.get_output()
.stdout_lossy();
assert!(output.contains("Ignoring the `detect_missing_libraries` flag; it should not be used in toml config file, but as an argument in the build command"), "{}", output);

// println!(
// "{}",
// cmd.args(["test", "--zksync", "--zk-detect-missing-libraries"])
// .assert_success()
// .get_output()
// .stdout_lossy()
// );

// .stderr_eq(str![r#"Ignoring the `detect_missing_libraries` flag; it should not be used in
// toml config file, but as an argument in the build command"#])
// .stdout_eq(str![[r#"
// ...
// Compiler run successful with warnings:
// ...
// "#]]);
// .stderr_eq(str![r#"Ignoring the `detect_missing_libraries` flag; it should not be used in
// toml config file, but as an argument in the build command"#]);
forgetest_init!(test_zk_missing_libraries_param_only_on_build, |prj, cmd| {
cmd.args(["script", "--zksync", "--zk-detect-missing-libraries"]).assert_failure().stderr_eq(
str![
r#"
...
error: unexpected argument '--zk-detect-missing-libraries' found
...
"#
],
);
});

0 comments on commit cb3a76a

Please sign in to comment.