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(cli): Add --target-dir option #7350

Merged
merged 5 commits into from
Feb 12, 2025
Merged
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
3 changes: 3 additions & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"envrc",
"EXPONENTIATE",
"Flamegraph",
"flamegraphs",
"flate",
"fmtstr",
"foldl",
Expand Down Expand Up @@ -120,6 +121,7 @@
"impls",
"indexmap",
"injective",
"interners",
"Inlines",
"instrumenter",
"interner",
Expand Down Expand Up @@ -227,6 +229,7 @@
"tempdir",
"tempfile",
"termcolor",
"termion",
"thiserror",
"tslog",
"turbofish",
Expand Down
1 change: 1 addition & 0 deletions tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ pub(crate) fn resolve_workspace_for_source_path(file_path: &Path) -> Result<Work
members: vec![assumed_package],
selected_package_index: Some(0),
is_assumed: true,
target_dir: None,
};
Ok(workspace)
}
Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,14 +271,14 @@
signature_help_provider: Some(lsp_types::OneOf::Right(
lsp_types::SignatureHelpOptions {
trigger_characters: Some(vec!["(".to_string(), ",".to_string()]),
retrigger_characters: None,

Check warning on line 274 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (retrigger)
work_done_progress_options: WorkDoneProgressOptions {
work_done_progress: None,
},
},
)),
code_action_provider: Some(lsp_types::OneOf::Right(lsp_types::CodeActionOptions {
code_action_kinds: Some(vec![CodeActionKind::QUICKFIX]),

Check warning on line 281 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (QUICKFIX)
work_done_progress_options: WorkDoneProgressOptions {
work_done_progress: None,
},
Expand Down Expand Up @@ -598,7 +598,7 @@
));
}

// The LSP client usually removes duplicate loctions, but we do it here just in case they don't
// The LSP client usually removes duplicate locations, but we do it here just in case they don't
locations.sort_by_key(|location| {
(
location.uri.to_string(),
Expand Down
4 changes: 3 additions & 1 deletion tooling/nargo/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ use crate::{
#[derive(Clone)]
pub struct Workspace {
pub root_dir: PathBuf,
/// Optional target directory override.
pub target_dir: Option<PathBuf>,
pub members: Vec<Package>,
// If `Some()`, the `selected_package_index` is used to select the only `Package` when iterating a Workspace
pub selected_package_index: Option<usize>,
Expand All @@ -34,7 +36,7 @@ impl Workspace {
}

pub fn target_directory_path(&self) -> PathBuf {
self.root_dir.join(TARGET_DIR)
self.target_dir.as_ref().cloned().unwrap_or_else(|| self.root_dir.join(TARGET_DIR))
}

pub fn export_directory_path(&self) -> PathBuf {
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/benches/criterion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ fn criterion_test_execution(c: &mut Criterion, test_program_dir: &Path, force_br
});
}

/// Go through all the selected tests and executem with and without Brillig.
/// Go through all the selected tests and execute them with and without Brillig.
fn criterion_selected_tests_execution(c: &mut Criterion) {
for test_program_dir in get_selected_tests() {
for force_brillig in [false, true] {
Expand Down
52 changes: 39 additions & 13 deletions tooling/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ struct NargoCli {
#[derive(Args, Clone, Debug)]
pub(crate) struct NargoConfig {
// REMINDER: Also change this flag in the LSP test lens if renamed
#[arg(long, hide = true, global = true, default_value = "./")]
#[arg(long, hide = true, global = true, default_value = "./", value_parser = parse_path)]
program_dir: PathBuf,

/// Override the default target directory.
#[arg(long, hide = true, global = true, value_parser = parse_path)]
target_dir: Option<PathBuf>,
}

/// Options for commands that work on either workspace or package scope.
Expand Down Expand Up @@ -130,14 +134,7 @@ enum LockType {
#[cfg(not(feature = "codegen-docs"))]
#[tracing::instrument(level = "trace")]
pub(crate) fn start_cli() -> eyre::Result<()> {
use fm::NormalizePath;

let NargoCli { command, mut config } = NargoCli::parse();

// If the provided `program_dir` is relative, make it absolute by joining it to the current directory.
if !config.program_dir.is_absolute() {
config.program_dir = std::env::current_dir().unwrap().join(config.program_dir).normalize();
}
let NargoCli { command, config } = NargoCli::parse();

match command {
NargoCommand::New(args) => new_cmd::run(args, config),
Expand Down Expand Up @@ -194,14 +191,17 @@ where
// or a specific package; if that's the case then parse the package name to select it in the workspace.
let selection = match cmd.package_selection() {
PackageSelection::DefaultOrAll if workspace_dir != package_dir => {
let workspace = read_workspace(&package_dir, PackageSelection::DefaultOrAll)?;
let package = workspace.into_iter().next().expect("there should be exactly 1 package");
let package = read_workspace(&package_dir, PackageSelection::DefaultOrAll)?;
let package = package.into_iter().next().expect("there should be exactly 1 package");
PackageSelection::Selected(package.name.clone())
}
other => other,
};
// Parse the top level workspace with the member selected.
let workspace = read_workspace(&workspace_dir, selection)?;
let mut workspace = read_workspace(&workspace_dir, selection)?;
// Optionally override the target directory. It's only done here because most commands like the LSP and DAP
// don't read or write artifacts, so they don't use the target directory.
workspace.target_dir = config.target_dir.clone();
// Lock manifests if the command needs it.
let _locks = match cmd.lock_type() {
LockType::None => None,
Expand Down Expand Up @@ -249,18 +249,44 @@ fn lock_workspace(workspace: &Workspace, exclusive: bool) -> Result<Vec<impl Dro
Ok(locks)
}

/// Parses a path and turns it into an absolute one by joining to the current directory.
fn parse_path(path: &str) -> Result<PathBuf, String> {
use fm::NormalizePath;
let mut path: PathBuf = path.parse().map_err(|e| format!("failed to parse path: {e}"))?;
if !path.is_absolute() {
path = std::env::current_dir().unwrap().join(path).normalize();
}
Ok(path)
}

#[cfg(test)]
mod tests {
use super::NargoCli;
use clap::Parser;

#[test]
fn test_parse_invalid_expression_width() {
let cmd = "nargo --program-dir . compile --expression-width 1";
let res = super::NargoCli::try_parse_from(cmd.split_ascii_whitespace());
let res = NargoCli::try_parse_from(cmd.split_ascii_whitespace());

let err = res.expect_err("should fail because of invalid width");
assert!(err.to_string().contains("expression-width"));
assert!(err
.to_string()
.contains(acvm::compiler::MIN_EXPRESSION_WIDTH.to_string().as_str()));
}

#[test]
fn test_parse_target_dir() {
let cmd = "nargo --program-dir . --target-dir ../foo/bar execute";
let cli = NargoCli::try_parse_from(cmd.split_ascii_whitespace()).expect("should parse");

let target_dir = cli.config.target_dir.expect("should parse target dir");
assert!(target_dir.is_absolute(), "should be made absolute");
assert!(target_dir.ends_with("foo/bar"));

let cmd = "nargo --program-dir . execute";
let cli = NargoCli::try_parse_from(cmd.split_ascii_whitespace()).expect("should parse");
assert!(cli.config.target_dir.is_none());
}
}
5 changes: 4 additions & 1 deletion tooling/nargo_toml/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ fn toml_to_workspace(
selected_package_index: Some(0),
members: vec![member],
is_assumed: false,
target_dir: None,
},
}
}
Expand Down Expand Up @@ -448,6 +449,7 @@ fn toml_to_workspace(
members,
selected_package_index,
is_assumed: false,
target_dir: None,
}
}
};
Expand Down Expand Up @@ -514,14 +516,15 @@ pub enum PackageSelection {
}

/// Resolves a Nargo.toml file into a `Workspace` struct as defined by our `nargo` core.
///
/// As a side effect it downloads project dependencies as well.
pub fn resolve_workspace_from_toml(
toml_path: &Path,
package_selection: PackageSelection,
current_compiler_version: Option<String>,
) -> Result<Workspace, ManifestError> {
let nargo_toml = read_toml(toml_path)?;
let workspace = toml_to_workspace(nargo_toml, package_selection)?;

if let Some(current_compiler_version) = current_compiler_version {
semver::semver_check_workspace(&workspace, current_compiler_version)?;
}
Expand Down
Loading