From 6e10a015944f0610020844288c90f6c9cea2abf2 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 9 Feb 2025 16:15:01 +0000 Subject: [PATCH 1/5] fix: lock git dependencies folder when resolving workspace --- Cargo.lock | 1 + tooling/nargo_toml/Cargo.toml | 1 + tooling/nargo_toml/src/git.rs | 18 +++++++++++++++++- tooling/nargo_toml/src/lib.rs | 6 +++++- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d22780e66a..f263d11b386 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3082,6 +3082,7 @@ version = "1.0.0-beta.1" dependencies = [ "dirs", "fm", + "fs2", "nargo", "noirc_driver", "noirc_frontend", diff --git a/tooling/nargo_toml/Cargo.toml b/tooling/nargo_toml/Cargo.toml index f5f7d7cd595..a984dc2c576 100644 --- a/tooling/nargo_toml/Cargo.toml +++ b/tooling/nargo_toml/Cargo.toml @@ -23,6 +23,7 @@ toml.workspace = true url.workspace = true noirc_driver.workspace = true semver = "1.0.20" +fs2 = "0.4.3" [dev-dependencies] tempfile.workspace = true diff --git a/tooling/nargo_toml/src/git.rs b/tooling/nargo_toml/src/git.rs index efaed4fabb9..b6be441145f 100644 --- a/tooling/nargo_toml/src/git.rs +++ b/tooling/nargo_toml/src/git.rs @@ -1,4 +1,8 @@ -use std::path::PathBuf; +use fs2::FileExt; +use std::{ + fs::{File, OpenOptions}, + path::PathBuf, +}; /// Creates a unique folder name for a GitHub repo /// by using its URL and tag @@ -23,6 +27,18 @@ fn git_dep_location(base: &url::Url, tag: &str) -> PathBuf { nargo_crates().join(folder_name) } +pub(crate) fn lock_git_deps() -> std::io::Result { + let file_path = nargo_crates().join(".package-cache"); + let file = OpenOptions::new().create(true).truncate(true).write(true).open(file_path)?; + if file.try_lock_exclusive().is_err() { + eprintln!("Waiting for lock on git dependencies cache..."); + } + + file.lock_exclusive()?; + + Ok(file) +} + /// XXX: I'd prefer to use a GitHub library however, there /// does not seem to be an easy way to download a repo at a specific /// tag diff --git a/tooling/nargo_toml/src/lib.rs b/tooling/nargo_toml/src/lib.rs index fa556d445a4..05f9161bdaf 100644 --- a/tooling/nargo_toml/src/lib.rs +++ b/tooling/nargo_toml/src/lib.rs @@ -10,6 +10,7 @@ use std::{ use errors::SemverError; use fm::{NormalizePath, FILE_EXTENSION}; +use fs2::FileExt; use nargo::{ package::{Dependency, Package, PackageType}, workspace::Workspace, @@ -23,7 +24,7 @@ mod git; mod semver; pub use errors::ManifestError; -use git::clone_git_repo; +use git::{clone_git_repo, lock_git_deps}; /// Searches for a `Nargo.toml` file in the current directory and all parent directories. /// For example, if the current directory is `/workspace/package/src`, then this function @@ -518,7 +519,10 @@ pub fn resolve_workspace_from_toml( current_compiler_version: Option, ) -> Result { let nargo_toml = read_toml(toml_path)?; + let lock = lock_git_deps().expect("Failed to lock git dependencies cache"); let workspace = toml_to_workspace(nargo_toml, package_selection)?; + lock.unlock().expect("Failed to unlock git dependencies cache"); + if let Some(current_compiler_version) = current_compiler_version { semver::semver_check_workspace(&workspace, current_compiler_version)?; } From d4997a45975f6f1ad26a5eb72c975f022d2bcdca Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 9 Feb 2025 16:28:47 +0000 Subject: [PATCH 2/5] . --- tooling/nargo_toml/src/git.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/nargo_toml/src/git.rs b/tooling/nargo_toml/src/git.rs index b6be441145f..af3337f2a95 100644 --- a/tooling/nargo_toml/src/git.rs +++ b/tooling/nargo_toml/src/git.rs @@ -28,6 +28,7 @@ fn git_dep_location(base: &url::Url, tag: &str) -> PathBuf { } pub(crate) fn lock_git_deps() -> std::io::Result { + std::fs::create_dir_all(nargo_crates())?; let file_path = nargo_crates().join(".package-cache"); let file = OpenOptions::new().create(true).truncate(true).write(true).open(file_path)?; if file.try_lock_exclusive().is_err() { From 84620715952320d32072a05f3bf95b6766f1872d Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 10 Feb 2025 10:40:57 +0000 Subject: [PATCH 3/5] chore: automatically remove locks when going out of scope --- Cargo.lock | 1 + tooling/nargo_toml/Cargo.toml | 1 + tooling/nargo_toml/src/flock.rs | 33 +++++++++++++++++++++++++++++++++ tooling/nargo_toml/src/git.rs | 21 +++++---------------- tooling/nargo_toml/src/lib.rs | 5 ++--- 5 files changed, 42 insertions(+), 19 deletions(-) create mode 100644 tooling/nargo_toml/src/flock.rs diff --git a/Cargo.lock b/Cargo.lock index f263d11b386..6fbe47d376f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3092,6 +3092,7 @@ dependencies = [ "test-case", "thiserror", "toml", + "tracing", "url", ] diff --git a/tooling/nargo_toml/Cargo.toml b/tooling/nargo_toml/Cargo.toml index a984dc2c576..5871df9f6f9 100644 --- a/tooling/nargo_toml/Cargo.toml +++ b/tooling/nargo_toml/Cargo.toml @@ -24,6 +24,7 @@ url.workspace = true noirc_driver.workspace = true semver = "1.0.20" fs2 = "0.4.3" +tracing.workspace = true [dev-dependencies] tempfile.workspace = true diff --git a/tooling/nargo_toml/src/flock.rs b/tooling/nargo_toml/src/flock.rs new file mode 100644 index 00000000000..20404e4d8a9 --- /dev/null +++ b/tooling/nargo_toml/src/flock.rs @@ -0,0 +1,33 @@ +use fs2::FileExt; +use std::{ + fs::{File, OpenOptions}, + path::Path, +}; + +// TODO: move this to some utils crate. + +pub(crate) struct FileLock { + file: File, +} + +impl FileLock { + pub(crate) fn new(file_path: &Path, lock_name: &str) -> std::io::Result { + std::fs::create_dir_all(file_path.parent().expect("can't create lock on filesystem root"))?; + let file = OpenOptions::new().create(true).truncate(true).write(true).open(file_path)?; + if file.try_lock_exclusive().is_err() { + eprintln!("Waiting for lock on {lock_name}..."); + } + + file.lock_exclusive()?; + + Ok(Self { file }) + } +} + +impl Drop for FileLock { + fn drop(&mut self) { + if let Err(e) = self.file.unlock() { + tracing::warn!("failed to release lock: {e:?}"); + } + } +} diff --git a/tooling/nargo_toml/src/git.rs b/tooling/nargo_toml/src/git.rs index af3337f2a95..1413dced0e5 100644 --- a/tooling/nargo_toml/src/git.rs +++ b/tooling/nargo_toml/src/git.rs @@ -1,8 +1,6 @@ -use fs2::FileExt; -use std::{ - fs::{File, OpenOptions}, - path::PathBuf, -}; +use std::path::PathBuf; + +use crate::flock::FileLock; /// Creates a unique folder name for a GitHub repo /// by using its URL and tag @@ -27,17 +25,8 @@ fn git_dep_location(base: &url::Url, tag: &str) -> PathBuf { nargo_crates().join(folder_name) } -pub(crate) fn lock_git_deps() -> std::io::Result { - std::fs::create_dir_all(nargo_crates())?; - let file_path = nargo_crates().join(".package-cache"); - let file = OpenOptions::new().create(true).truncate(true).write(true).open(file_path)?; - if file.try_lock_exclusive().is_err() { - eprintln!("Waiting for lock on git dependencies cache..."); - } - - file.lock_exclusive()?; - - Ok(file) +pub(crate) fn lock_git_deps() -> std::io::Result { + FileLock::new(&nargo_crates().join(".package-cache"), "git dependencies cache") } /// XXX: I'd prefer to use a GitHub library however, there diff --git a/tooling/nargo_toml/src/lib.rs b/tooling/nargo_toml/src/lib.rs index 05f9161bdaf..8240076fc0b 100644 --- a/tooling/nargo_toml/src/lib.rs +++ b/tooling/nargo_toml/src/lib.rs @@ -10,7 +10,6 @@ use std::{ use errors::SemverError; use fm::{NormalizePath, FILE_EXTENSION}; -use fs2::FileExt; use nargo::{ package::{Dependency, Package, PackageType}, workspace::Workspace, @@ -20,6 +19,7 @@ use noirc_frontend::graph::CrateName; use serde::Deserialize; mod errors; +mod flock; mod git; mod semver; @@ -519,9 +519,8 @@ pub fn resolve_workspace_from_toml( current_compiler_version: Option, ) -> Result { let nargo_toml = read_toml(toml_path)?; - let lock = lock_git_deps().expect("Failed to lock git dependencies cache"); + let _lock = lock_git_deps().expect("Failed to lock git dependencies cache"); let workspace = toml_to_workspace(nargo_toml, package_selection)?; - lock.unlock().expect("Failed to unlock git dependencies cache"); if let Some(current_compiler_version) = current_compiler_version { semver::semver_check_workspace(&workspace, current_compiler_version)?; From 04c20ee3ea84c8450098f3caaa1c96fd7fb9ae28 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 10 Feb 2025 11:16:08 +0000 Subject: [PATCH 4/5] chore: turn off truncation --- tooling/nargo_toml/src/flock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_toml/src/flock.rs b/tooling/nargo_toml/src/flock.rs index 20404e4d8a9..031dbcff647 100644 --- a/tooling/nargo_toml/src/flock.rs +++ b/tooling/nargo_toml/src/flock.rs @@ -13,7 +13,7 @@ pub(crate) struct FileLock { impl FileLock { pub(crate) fn new(file_path: &Path, lock_name: &str) -> std::io::Result { std::fs::create_dir_all(file_path.parent().expect("can't create lock on filesystem root"))?; - let file = OpenOptions::new().create(true).truncate(true).write(true).open(file_path)?; + let file = OpenOptions::new().create(true).truncate(false).write(true).open(file_path)?; if file.try_lock_exclusive().is_err() { eprintln!("Waiting for lock on {lock_name}..."); } From 76b447de5ca602446c16f6106bd94042c37b4a29 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 10 Feb 2025 13:57:07 +0000 Subject: [PATCH 5/5] chore: remove file lock inside of `toml_to_workspace` --- tooling/nargo_toml/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_toml/src/lib.rs b/tooling/nargo_toml/src/lib.rs index 8240076fc0b..e0b3fe19e0c 100644 --- a/tooling/nargo_toml/src/lib.rs +++ b/tooling/nargo_toml/src/lib.rs @@ -383,6 +383,7 @@ fn toml_to_workspace( package_selection: PackageSelection, ) -> Result { let mut resolved = Vec::new(); + let _lock = lock_git_deps().expect("Failed to lock git dependencies cache"); let workspace = match nargo_toml.config { Config::Package { package_config } => { let member = package_config.resolve_to_package(&nargo_toml.root_dir, &mut resolved)?; @@ -519,7 +520,6 @@ pub fn resolve_workspace_from_toml( current_compiler_version: Option, ) -> Result { let nargo_toml = read_toml(toml_path)?; - let _lock = lock_git_deps().expect("Failed to lock git dependencies cache"); let workspace = toml_to_workspace(nargo_toml, package_selection)?; if let Some(current_compiler_version) = current_compiler_version {