Skip to content
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.

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

feat: test_programs that are invisible to git now pass #5035

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ tooling/noir_js/lib
**/target
!test_programs/acir_artifacts/*/target
!test_programs/acir_artifacts/*/target/witness.gz
!test_programs/compile_success_empty/workspace_usually_gitignored/bin/Verifier.toml
!compiler/wasm/noir-script/target

gates_report.json
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
return = ""
1 change: 1 addition & 0 deletions tooling/nargo_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ path = "src/main.rs"

[build-dependencies]
build-data.workspace = true
nargo_toml.workspace = true
toml.workspace = true

[dependencies]
Expand Down
17 changes: 17 additions & 0 deletions tooling/nargo_cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use std::io::Write;
use std::path::{Path, PathBuf};
use std::{env, fs};

use nargo_toml::{find_package_root, ManifestError};

const GIT_COMMIT: &&str = &"GIT_COMMIT";

fn main() {
Expand Down Expand Up @@ -230,6 +232,11 @@ fn compile_success_empty_{test_name}() {{
panic!("`nargo info` failed with: {{}}", String::from_utf8(output.stderr).unwrap_or_default());
}}

// skip empty test packages
if std::str::from_utf8(&output.stdout).unwrap_or("").contains("cannot find any files visible to git at the following path") {{
return ()
}}

// `compile_success_empty` tests should be able to compile down to an empty circuit.
let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap_or_else(|e| {{
panic!("JSON was not well-formatted {{:?}}\n\n{{:?}}", e, std::str::from_utf8(&output.stdout))
Expand Down Expand Up @@ -299,6 +306,16 @@ fn generate_compile_failure_tests(test_file: &mut File, test_data_dir: &Path) {
};
let test_dir = &test_dir.path();

match find_package_root(test_dir) {
Ok(_) => (),
Err(err) => {
// avoid testing when there are no files in the test_dir
return if matches!(err, ManifestError::InvisibleToGit(..)) {
continue;
};
}
}

write!(
test_file,
r#"
Expand Down
15 changes: 13 additions & 2 deletions tooling/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clap::{Args, Parser, Subcommand};
use const_format::formatcp;
use nargo_toml::find_package_root;
use nargo_toml::{find_package_root, ManifestError};
use noirc_driver::NOIR_ARTIFACT_VERSION_STRING;
use std::path::PathBuf;

Expand Down Expand Up @@ -100,7 +100,18 @@ pub(crate) fn start_cli() -> eyre::Result<()> {
| NargoCommand::Backend(_)
| NargoCommand::Dap(_)
) {
config.program_dir = find_package_root(&config.program_dir)?;
match find_package_root(&config.program_dir) {
Ok(program_dir) => config.program_dir = program_dir,
Err(err) => {
// avoid erroring when there are no files in the program_dir
return if matches!(err, ManifestError::InvisibleToGit(..)) {
println!("{}", err);
Ok(())
} else {
Err(err.into())
};
}
}
}

let active_backend = get_active_backend();
Expand Down
4 changes: 4 additions & 0 deletions tooling/nargo_toml/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ pub enum ManifestError {
#[error("cannot find a Nargo.toml for {0}")]
MissingFile(PathBuf),

/// Package doesn't contain any files that are visible to git
#[error("cannot find any files visible to git at the following path (skipping): {0}")]
InvisibleToGit(PathBuf),

#[error("Cannot read file {0} - does it exist?")]
ReadFailed(PathBuf),

Expand Down
36 changes: 35 additions & 1 deletion tooling/nargo_toml/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use std::{
collections::BTreeMap,
ffi::OsStr,
path::{Component, Path, PathBuf},
};

Expand Down Expand Up @@ -92,14 +93,47 @@ pub fn find_package_manifest(
}

// Return the shallowest Nargo.toml, which will be the last in the list
found_toml_paths.pop().ok_or_else(|| ManifestError::MissingFile(current_path.to_path_buf()))
found_toml_paths.pop().ok_or_else(|| {
if is_package_visible_to_git(current_path) {
ManifestError::MissingFile(current_path.to_path_buf())
} else {
ManifestError::InvisibleToGit(current_path.to_path_buf())
}
})
} else {
Err(ManifestError::NoCommonAncestor {
root: root_path.to_path_buf(),
current: current_path.to_path_buf(),
})
}
}

/// Check whether the given path is visible to git.
/// A package can become invisible to git when its visible contents are deleted,
/// but its folders remain, as can happen from git merges.
fn is_package_visible_to_git(current_path: &Path) -> bool {
if current_path.is_file() {
// **/Verifier.toml
if current_path.file_name() != Some(OsStr::new("Verifier.toml")) {
return true;
}
} else {
// **/target
// !test_programs/acir_artifacts/*/target
if current_path.file_name() == Some(OsStr::new("target")) {
return current_path.to_str().unwrap_or("").contains("acir_artifacts");
}

for sub_path_entry in current_path.read_dir().expect("path to exist during compilation") {
let sub_path = sub_path_entry.expect("path to exist during compilation").path();
if is_package_visible_to_git(&sub_path) {
return true;
}
}
}
false
}

/// Returns the [PathBuf] of the `Nargo.toml` file in the `current_path` directory.
///
/// Returns a [ManifestError] if `current_path` does not contain a manifest file.
Expand Down
Loading