diff --git a/crates/cli/src/utils/mod.rs b/crates/cli/src/utils/mod.rs index faf80d1cff095..e5ccb4612211b 100644 --- a/crates/cli/src/utils/mod.rs +++ b/crates/cli/src/utils/mod.rs @@ -409,6 +409,11 @@ impl<'a> Git<'a> { .map(drop) } + /// Returns the current HEAD commit hash of the current branch. + pub fn head(self) -> Result { + self.cmd().args(["rev-parse", "HEAD"]).get_stdout_lossy() + } + pub fn checkout_at(self, tag: impl AsRef, at: &Path) -> Result<()> { self.cmd_at(at).arg("checkout").arg(tag).exec().map(drop) } diff --git a/crates/forge/src/cmd/update.rs b/crates/forge/src/cmd/update.rs index dfb821aedc9d6..9c483f2a65fb4 100644 --- a/crates/forge/src/cmd/update.rs +++ b/crates/forge/src/cmd/update.rs @@ -4,10 +4,10 @@ use clap::{Parser, ValueHint}; use eyre::{Context, Result}; use foundry_cli::{ opts::Dependency, - utils::{Git, LoadConfig}, + utils::{CommandUtils, Git, LoadConfig}, }; use foundry_config::{Config, impl_figment_convert_basic}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use yansi::Paint; /// CLI arguments for `forge update`. @@ -60,9 +60,23 @@ impl UpdateArgs { let rel_path = dep_path .strip_prefix(&root) .wrap_err("Dependency path is not relative to the repository root")?; - if let Ok(dep_id) = DepIdentifier::resolve_type(&git, dep_path, override_tag) { - let prev = foundry_lock.override_dep(rel_path, dep_id)?; - prev_dep_ids.insert(rel_path.to_owned(), prev); + + if let Ok(mut dep_id) = DepIdentifier::resolve_type(&git, dep_path, override_tag) { + // Store the previous state before overriding + let prev = foundry_lock.get(rel_path).cloned(); + + // If it's a branch, mark it as overridden so it gets updated below + if let DepIdentifier::Branch { .. } = dep_id { + dep_id.mark_override(); + } + + // Update the lockfile + foundry_lock.override_dep(rel_path, dep_id)?; + + // Only track as updated if there was a previous dependency + if let Some(prev) = prev { + prev_dep_ids.insert(rel_path.to_owned(), prev); + } } else { sh_warn!( "Could not r#override submodule at {} with tag {}, try using forge install", @@ -94,54 +108,53 @@ impl UpdateArgs { } } - // Branches should get updated to their latest commit on `forge update`. - // i.e if previously submodule was tracking branch `main` at rev `1234567` and now the - // remote `main` branch is at `7654321`, then submodule should also be updated to `7654321`. - // This tracking is automatically handled by git, but we need to update the lockfile entry - // to reflect the latest commit. - if dep_overrides.is_empty() { - let branch_overrides = foundry_lock - .iter_mut() - .filter_map(|(path, dep_id)| { - if dep_id.is_branch() && dep_id.overridden() { - return Some((path, dep_id)); - } - None - }) - .collect::>(); - - for (path, dep_id) in branch_overrides { - let (curr_rev, curr_branch) = git.current_rev_branch(&root.join(path))?; - let name = dep_id.name(); - // This can occur when the submodule is manually checked out to a different branch. - if curr_branch != name { - let warn_msg = format!( - r#"Lockfile sync warning - Lockfile is tracking branch {name} for submodule at {path:?}, but the submodule is currently on {curr_branch}. - Checking out branch {name} for submodule at {path:?}."#, - ); - let _ = sh_warn!("{}", warn_msg); - git.checkout_at(name, &root.join(path)).wrap_err(format!( - "Could not checkout branch {name} for submodule at {}", - path.display() - ))?; + // Update branches to their latest commit from origin + // This handles both explicit updates (forge update dep@branch) and + // general updates (forge update) for branch-tracked dependencies + let branch_overrides = foundry_lock + .iter_mut() + .filter_map(|(path, dep_id)| { + if dep_id.is_branch() && dep_id.overridden() { + return Some((path, dep_id)); } - - // Update the lockfile entry to reflect the latest commit - let prev = std::mem::replace( - dep_id, - DepIdentifier::Branch { - name: name.to_string(), - rev: curr_rev, - r#override: true, - }, - ); + None + }) + .collect::>(); + + for (path, dep_id) in branch_overrides { + let submodule_path = root.join(path); + let name = dep_id.name(); + + // Fetch and checkout the latest commit from the remote branch + Self::fetch_and_checkout_branch(&git, &submodule_path, name)?; + + // Now get the updated revision after syncing with origin + let (updated_rev, _) = git.current_rev_branch(&submodule_path)?; + + // Update the lockfile entry to reflect the latest commit + let prev = std::mem::replace( + dep_id, + DepIdentifier::Branch { + name: name.to_string(), + rev: updated_rev, + r#override: true, + }, + ); + + // Only insert if we don't already have a previous state for this path + // (e.g., from explicit overrides where we converted tag to branch) + if !prev_dep_ids.contains_key(path) { prev_dep_ids.insert(path.to_owned(), prev); } } // checkout the submodules at the correct tags + // Skip branches that were already updated above to avoid reverting to local branch for (path, dep_id) in foundry_lock.iter() { + // Skip branches that were already updated + if dep_id.is_branch() && dep_id.overridden() { + continue; + } git.checkout_at(dep_id.checkout_id(), &root.join(path))?; } @@ -177,6 +190,29 @@ impl UpdateArgs { }) .collect() } + + /// Fetches and checks out the latest version of a branch from origin + fn fetch_and_checkout_branch(git: &Git<'_>, path: &Path, branch: &str) -> Result<()> { + // Fetch the latest changes from origin for the branch + git.cmd_at(path).args(["fetch", "origin", branch]).exec().wrap_err(format!( + "Could not fetch latest changes for branch {} in submodule at {}", + branch, + path.display() + ))?; + + // Checkout and track the remote branch to ensure we have the latest commit + // Using checkout -B ensures the local branch tracks origin/branch + git.cmd_at(path) + .args(["checkout", "-B", branch, &format!("origin/{branch}")]) + .exec() + .wrap_err(format!( + "Could not checkout and track origin/{} for submodule at {}", + branch, + path.display() + ))?; + + Ok(()) + } } /// Returns `(root, paths, overridden_deps_with_abosolute_paths)` where `root` is the root of the diff --git a/crates/forge/tests/cli/install.rs b/crates/forge/tests/cli/install.rs index c842224cb41af..75c1b6dde949a 100644 --- a/crates/forge/tests/cli/install.rs +++ b/crates/forge/tests/cli/install.rs @@ -590,3 +590,42 @@ async fn correctly_sync_dep_with_multiple_version() { assert!(matches!(solday_v_245, DepIdentifier::Rev { .. })); assert_eq!(solday_v_245.rev(), submod_solday_v_245.rev()); } + +forgetest_init!(sync_on_forge_update, |prj, cmd| { + let git = Git::new(prj.root()); + + let submodules = git.submodules().unwrap(); + assert!(submodules.0.iter().any(|s| s.rev() == FORGE_STD_REVISION)); + + let mut lockfile = Lockfile::new(prj.root()); + lockfile.read().unwrap(); + + let forge_std = lockfile.get(&PathBuf::from("lib/forge-std")).unwrap(); + assert!(forge_std.rev() == FORGE_STD_REVISION); + + // cd into the forge-std submodule and reset the master branch + let forge_std_path = prj.root().join("lib/forge-std"); + let git = Git::new(&forge_std_path); + git.checkout(false, "master").unwrap(); + // Get the master head commit + let origin_master_head = git.head().unwrap(); + // Reset the master branch to HEAD~1 + git.reset(true, "HEAD~1").unwrap(); + let local_master_head = git.head().unwrap(); + assert_ne!(origin_master_head, local_master_head, "Master head should have changed"); + // Now checkout back to the release tag + git.checkout(false, forge_std.name()).unwrap(); + assert!(git.head().unwrap() == forge_std.rev(), "Forge std should be at the release tag"); + + let expected_output = format!( + r#"Updated dep at 'lib/forge-std', (from: tag={}@{}, to: branch=master@{}) +"#, + forge_std.name(), + forge_std.rev(), + origin_master_head + ); + cmd.forge_fuse() + .args(["update", "foundry-rs/forge-std@master"]) + .assert_success() + .stdout_eq(expected_output); +});