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!: Add semver checks for the compiler version in Nargo.toml #3336

Merged
merged 38 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
721b7b4
recompile artifacts from a different noir version
guipublic Oct 23, 2023
12eb3f0
noir version also for wasm
guipublic Oct 23, 2023
3bfa955
chore: move empty circuits to `compile_success_empty` (#3247)
TomAFrench Oct 23, 2023
dd19890
chore: add constructor formatter (#3243)
Oct 23, 2023
b519e28
chore(ci): Generate circuit benchmark summaries on PRs (#3250)
TomAFrench Oct 23, 2023
07ca181
add git commit to noir_version
guipublic Oct 23, 2023
36ba1a6
feat: Cache debug artifacts (#3133)
TomAFrench Oct 23, 2023
6dd738f
feat: Cache debug artifacts (#3133)
TomAFrench Oct 23, 2023
5ff3cbe
feat: Cache debug artifacts (#3133)
TomAFrench Oct 23, 2023
5604bdd
format
guipublic Oct 23, 2023
7caf94f
Merge branch 'master' into gd/issue_3130
guipublic Oct 23, 2023
1c4a0f7
code reivew: remove tmp file
guipublic Oct 23, 2023
edad212
move noirc_version to noirc_driver
kevaundray Oct 23, 2023
dc40dc6
cargo fmt
kevaundray Oct 23, 2023
561036f
modify versioning format
kevaundray Oct 23, 2023
661c0e3
change regex in test
kevaundray Oct 23, 2023
8a52786
remove outdated comment
kevaundray Oct 23, 2023
8dd44cd
add required semver version
kevaundray Oct 28, 2023
61970b4
Add initial code for checking the compiler version
kevaundray Oct 28, 2023
db95016
use + for build data
kevaundray Oct 28, 2023
bbee6ce
cargo
kevaundray Oct 28, 2023
3488099
temp: resolve workspace without checking semver initially
kevaundray Oct 28, 2023
1268fb8
rename fields of error variant to be clearer
kevaundray Oct 28, 2023
0042909
semver check on compile_cmd
kevaundray Oct 28, 2023
923321d
missed argument
kevaundray Oct 28, 2023
d66e25c
remove redundant import
kevaundray Oct 28, 2023
74c19d5
Merge remote-tracking branch 'origin/master' into kw/semver-compiler-…
kevaundray Oct 28, 2023
475fea5
clippy
kevaundray Oct 28, 2023
77fab01
cargo fmt
kevaundray Oct 28, 2023
f062e3b
modify init compiler_version to include this compiler version and above.
kevaundray Oct 29, 2023
e99d5e6
modify compiler_version in test to be any
kevaundray Oct 29, 2023
4e587b7
remove compiler_version from examples/test files
kevaundray Oct 29, 2023
78291c3
fix typo
kevaundray Oct 29, 2023
726e430
fix more grep typos
kevaundray Oct 29, 2023
2e2ae60
update error message
kevaundray Oct 29, 2023
a3de7ef
chore(docs): updating docs to reflect the changes in compiler_version
signorecello Oct 29, 2023
fb9bca5
cargo fmt
kevaundray Oct 29, 2023
7e830df
semver check on all function calls that resolve a workspace
kevaundray Oct 30, 2023
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
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tooling/lsp/src/notifications/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub(super) fn on_did_save_text_document(
return ControlFlow::Continue(());
}
};
let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) {
let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All, None) {
Ok(workspace) => workspace,
Err(err) => {
// If we found a manifest, but the workspace is invalid, we raise an error about it
Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/code_lens_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ fn on_code_lens_request_inner(
}
};
let workspace =
resolve_workspace_from_toml(&toml_path, PackageSelection::All).map_err(|err| {
resolve_workspace_from_toml(&toml_path, PackageSelection::All, None).map_err(|err| {
// If we found a manifest, but the workspace is invalid, we raise an error about it
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;
Expand Down
15 changes: 9 additions & 6 deletions tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@ fn on_test_run_request_inner(
let crate_name = params.id.crate_name();
let function_name = params.id.function_name();

let workspace =
resolve_workspace_from_toml(&toml_path, PackageSelection::Selected(crate_name.clone()))
.map_err(|err| {
// If we found a manifest, but the workspace is invalid, we raise an error about it
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;
let workspace = resolve_workspace_from_toml(
&toml_path,
PackageSelection::Selected(crate_name.clone()),
None,
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
)
.map_err(|err| {
// If we found a manifest, but the workspace is invalid, we raise an error about it
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;

// Since we filtered on crate name, this should be the only item in the iterator
match workspace.into_iter().next() {
Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn on_tests_request_inner(
};

let workspace =
resolve_workspace_from_toml(&toml_path, PackageSelection::All).map_err(|err| {
resolve_workspace_from_toml(&toml_path, PackageSelection::All, None).map_err(|err| {
// If we found a manifest, but the workspace is invalid, we raise an error about it
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;
Expand Down
2 changes: 2 additions & 0 deletions tooling/nargo/src/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ impl Dependency {

#[derive(Clone)]
pub struct Package {
// A semver string which specifies the compiler version required to compile this package
pub compiler_required_version: Option<String>,
pub root_dir: PathBuf,
pub package_type: PackageType,
pub entry_path: PathBuf,
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub(crate) fn run(
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;
let workspace = resolve_workspace_from_toml(&toml_path, selection, None)?;

for package in &workspace {
check_package(package, &args.compile_options)?;
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub(crate) fn run(
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;
let workspace = resolve_workspace_from_toml(&toml_path, selection, None)?;

let (np_language, opcode_support) = backend.get_backend_info()?;
for package in &workspace {
Expand Down
7 changes: 6 additions & 1 deletion tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ pub(crate) fn run(
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;

let workspace = resolve_workspace_from_toml(
&toml_path,
selection,
Some(NOIR_ARTIFACT_VERSION_STRING.to_owned()),
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
)?;
let circuit_dir = workspace.target_directory_path();

let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/debug_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub(crate) fn run(
) -> Result<(), CliError> {
let toml_path = get_package_manifest(&config.program_dir)?;
let selection = args.package.map_or(PackageSelection::DefaultOrAll, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;
let workspace = resolve_workspace_from_toml(&toml_path, selection, None)?;
let target_dir = &workspace.target_directory_path();
let (np_language, opcode_support) = backend.get_backend_info()?;

Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub(crate) fn run(
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;
let workspace = resolve_workspace_from_toml(&toml_path, selection, None)?;
let target_dir = &workspace.target_directory_path();

let (np_language, opcode_support) = backend.get_backend_info()?;
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/fmt_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub(crate) struct FormatCommand {}

pub(crate) fn run(_args: FormatCommand, config: NargoConfig) -> Result<(), CliError> {
let toml_path = get_package_manifest(&config.program_dir)?;
let workspace = resolve_workspace_from_toml(&toml_path, PackageSelection::All)?;
let workspace = resolve_workspace_from_toml(&toml_path, PackageSelection::All, None)?;

let config = nargo_fmt::Config::read(&config.program_dir)
.map_err(|err| CliError::Generic(err.to_string()))?;
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/info_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(crate) fn run(
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;
let workspace = resolve_workspace_from_toml(&toml_path, selection, None)?;

let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace
.into_iter()
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/prove_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) fn run(
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;
let workspace = resolve_workspace_from_toml(&toml_path, selection, None)?;

let (np_language, opcode_support) = backend.get_backend_info()?;
for package in &workspace {
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) fn run(
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;
let workspace = resolve_workspace_from_toml(&toml_path, selection, None)?;

let pattern = match &args.test_name {
Some(name) => {
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/verify_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub(crate) fn run(
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
let selection = args.package.map_or(default_selection, PackageSelection::Selected);
let workspace = resolve_workspace_from_toml(&toml_path, selection)?;
let workspace = resolve_workspace_from_toml(&toml_path, selection, None)?;

let (np_language, opcode_support) = backend.get_backend_info()?;
for package in &workspace {
Expand Down
1 change: 1 addition & 0 deletions tooling/nargo_toml/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ serde.workspace = true
thiserror.workspace = true
toml.workspace = true
url.workspace = true
semver = "1.0.20"

[dev-dependencies]
18 changes: 17 additions & 1 deletion tooling/nargo_toml/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use nargo::package::PackageType;
use noirc_frontend::graph::CrateName;
use thiserror::Error;

/// Errors covering situations where a package is either missing or malformed.
/// Errors covering situations where a package is either missing, malformed or does not pass semver
/// validation checks.
#[derive(Debug, Error)]
pub enum ManifestError {
/// Package doesn't have a manifest file
Expand Down Expand Up @@ -65,4 +66,19 @@ pub enum ManifestError {

#[error("No common ancestor between {root} and {current}")]
NoCommonAncestor { root: PathBuf, current: PathBuf },

#[error(transparent)]
SemverError(SemverError),
}

#[derive(Error, Debug, PartialEq, Eq, Clone)]
pub enum SemverError {
#[error("Incompatible compiler version in package {package_name}. Required version is {required_compiler_version} but the compiler version is {compiler_version_found}")]
IncompatibleVersion {
package_name: CrateName,
required_compiler_version: String,
compiler_version_found: String,
},
#[error("Could not parse the required compiler version for package {package_name} in Nargo.toml. Error: {error}")]
CouldNotParseRequiredVersion { package_name: String, error: String },
}
12 changes: 9 additions & 3 deletions tooling/nargo_toml/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use serde::Deserialize;

mod errors;
mod git;
mod semver;

pub use errors::ManifestError;
use git::clone_git_repo;
Expand Down Expand Up @@ -163,6 +164,7 @@ impl PackageConfig {
};

Ok(Package {
compiler_required_version: self.package.compiler_version.clone(),
root_dir: root_dir.to_path_buf(),
entry_path,
package_type,
Expand Down Expand Up @@ -228,7 +230,7 @@ struct PackageMetadata {
entry: Option<PathBuf>,
description: Option<String>,
authors: Option<Vec<String>>,
// If not compiler version is supplied, the latest is used
// If no compiler version is supplied, the latest is used
// For now, we state that all packages must be compiled under the same
// compiler version.
// We also state that ACIR and the compiler will upgrade in lockstep.
Expand Down Expand Up @@ -391,10 +393,14 @@ pub enum PackageSelection {
pub fn resolve_workspace_from_toml(
toml_path: &Path,
package_selection: PackageSelection,
current_compiler_version: Option<String>,
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<Workspace, ManifestError> {
let nargo_toml = read_toml(toml_path)?;

toml_to_workspace(nargo_toml, package_selection)
let workspace = toml_to_workspace(nargo_toml, package_selection)?;
if let Some(current_compiler_version) = current_compiler_version {
semver::semver_check_workspace(workspace.clone(), current_compiler_version)?;
}
Ok(workspace)
}

#[test]
Expand Down
Loading
Loading