feat(forge): don't commit installations by default (foundry-rs#9884)

* feat(`forge`)!: flip `no_commit` to `commit` + don't commit installations by default

* fix tests + always commit when initialized via template

* fix

* nit

Author: yash-atreya

Cargo.lock

@@ -818,7 +818,7 @@ type                 2
 blobGasPrice         
 blobGasUsed          
 to                   0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45
-revertReason         Transaction too old, data: "0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000135472616e73616374696f6e20746f6f206f6c6400000000000000000000000000"
+revertReason         [..]Transaction too old, data: "0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000135472616e73616374696f6e20746f6f206f6c6400000000000000000000000000"
 
 "#]]);
 });

crates/cast/tests/cli/main.rs

@@ -460,7 +460,7 @@ and it requires clean working and staging areas, including no untracked files.
 
 Check the current git repository's status with `git status`.
 Then, you can track files with `git add ...` and then commit them with `git commit`,
-ignore them in the `.gitignore` file, or run this command again with the `--no-commit` flag."
+ignore them in the `.gitignore` file."
             ))
         }
     }

crates/cli/src/utils/mod.rs

@@ -130,7 +130,7 @@ impl CloneArgs {
         Self::collect_compilation_metadata(&meta, chain, address, &root, &client).await?;
 
         // step 5. git add and commit the changes if needed
-        if !install.no_commit {
+        if install.commit {
             let git = Git::new(&root);
             git.add(Some("--all"))?;
             let msg = format!("chore: forge clone {address}");

crates/forge/bin/cmd/clone.rs

@@ -44,7 +44,7 @@ pub struct InitArgs {
 impl InitArgs {
     pub fn run(self) -> Result<()> {
         let Self { root, template, branch, install, offline, force, vscode } = self;
-        let DependencyInstallOpts { shallow, no_git, no_commit } = install;
+        let DependencyInstallOpts { shallow, no_git, commit } = install;
 
         // create the root dir if it does not exist
         if !root.exists() {
@@ -70,17 +70,15 @@ impl InitArgs {
             // collapsed. gitmodules will be initialized after the template is fetched
             git.fetch(true, &template, branch)?;
 
-            if !no_commit {
-                // reset git history to the head of the template
-                // first get the commit hash that was fetched
-                let commit_hash = git.commit_hash(true, "FETCH_HEAD")?;
-                // format a commit message for the new repo
-                let commit_msg = format!("chore: init from {template} at {commit_hash}");
-                // get the hash of the FETCH_HEAD with the new commit message
-                let new_commit_hash = git.commit_tree("FETCH_HEAD^{tree}", Some(commit_msg))?;
-                // reset head of this repo to be the head of the template repo
-                git.reset(true, new_commit_hash)?;
-            }
+            // reset git history to the head of the template
+            // first get the commit hash that was fetched
+            let commit_hash = git.commit_hash(true, "FETCH_HEAD")?;
+            // format a commit message for the new repo
+            let commit_msg = format!("chore: init from {template} at {commit_hash}");
+            // get the hash of the FETCH_HEAD with the new commit message
+            let new_commit_hash = git.commit_tree("FETCH_HEAD^{tree}", Some(commit_msg))?;
+            // reset head of this repo to be the head of the template repo
+            git.reset(true, new_commit_hash)?;
 
             // if shallow, just initialize submodules
             if shallow {
@@ -102,7 +100,7 @@ impl InitArgs {
             }
 
             // ensure git status is clean before generating anything
-            if !no_git && !no_commit && !force && git.is_in_repo()? {
+            if !no_git && commit && !force && git.is_in_repo()? {
                 git.ensure_clean()?;
             }
 
@@ -141,7 +139,7 @@ impl InitArgs {
 
             // set up the repo
             if !no_git {
-                init_git_repo(git, no_commit)?;
+                init_git_repo(git, commit)?;
             }
 
             // install forge-std
@@ -170,8 +168,8 @@ impl InitArgs {
 ///
 /// Creates `.gitignore` and `.github/workflows/test.yml`, if they don't exist already.
 ///
-/// Commits everything in `root` if `no_commit` is false.
-fn init_git_repo(git: Git<'_>, no_commit: bool) -> Result<()> {
+/// Commits everything in `root` if `commit` is true.
+fn init_git_repo(git: Git<'_>, commit: bool) -> Result<()> {
     // git init
     if !git.is_in_repo()? {
         git.init()?;
@@ -191,7 +189,7 @@ fn init_git_repo(git: Git<'_>, no_commit: bool) -> Result<()> {
     }
 
     // commit everything
-    if !no_commit {
+    if commit {
         git.add(Some("--all"))?;
         git.commit("chore: forge init")?;
     }

crates/forge/bin/cmd/init.rs

@@ -76,9 +76,9 @@ pub struct DependencyInstallOpts {
     #[arg(long)]
     pub no_git: bool,
 
-    /// Do not create a commit.
+    /// Create a commit after installing the dependencies.
     #[arg(long)]
-    pub no_commit: bool,
+    pub commit: bool,
 }
 
 impl DependencyInstallOpts {
@@ -91,12 +91,11 @@ impl DependencyInstallOpts {
     /// See also [`Self::install`].
     ///
     /// Returns true if any dependency was installed.
-    pub fn install_missing_dependencies(mut self, config: &mut Config) -> bool {
+    pub fn install_missing_dependencies(self, config: &mut Config) -> bool {
         let lib = config.install_lib_dir();
         if self.git(config).has_missing_dependencies(Some(lib)).unwrap_or(false) {
             // The extra newline is needed, otherwise the compiler output will overwrite the message
             let _ = sh_println!("Missing dependencies found. Installing now...\n");
-            self.no_commit = true;
             if self.install(config, Vec::new()).is_err() {
                 let _ =
                     sh_warn!("Your project has missing dependencies that could not be installed.");
@@ -109,7 +108,7 @@ impl DependencyInstallOpts {
 
     /// Installs all dependencies
     pub fn install(self, config: &mut Config, dependencies: Vec<Dependency>) -> Result<()> {
-        let Self { no_git, no_commit, .. } = self;
+        let Self { no_git, commit, .. } = self;
 
         let git = self.git(config);
 
@@ -138,7 +137,7 @@ impl DependencyInstallOpts {
 
         fs::create_dir_all(&libs)?;
 
-        let installer = Installer { git, no_commit };
+        let installer = Installer { git, commit };
         for dep in dependencies {
             let path = libs.join(dep.name());
             let rel_path = path
@@ -157,7 +156,7 @@ impl DependencyInstallOpts {
             if no_git {
                 installed_tag = installer.install_as_folder(&dep, &path)?;
             } else {
-                if !no_commit {
+                if commit {
                     git.ensure_clean()?;
                 }
                 installed_tag = installer.install_as_submodule(&dep, &path)?;
@@ -173,14 +172,16 @@ impl DependencyInstallOpts {
                             .exec()?;
                     }
 
-                    // update .gitmodules which is at the root of the repo,
-                    // not necessarily at the root of the current Foundry project
-                    let root = Git::root_of(git.root)?;
-                    git.root(&root).add(Some(".gitmodules"))?;
+                    if commit {
+                        // update .gitmodules which is at the root of the repo,
+                        // not necessarily at the root of the current Foundry project
+                        let root = Git::root_of(git.root)?;
+                        git.root(&root).add(Some(".gitmodules"))?;
+                    }
                 }
 
                 // commit the installation
-                if !no_commit {
+                if commit {
                     let mut msg = String::with_capacity(128);
                     msg.push_str("forge install: ");
                     msg.push_str(dep.name());
@@ -216,7 +217,7 @@ pub fn install_missing_dependencies(config: &mut Config) -> bool {
 #[derive(Clone, Copy, Debug)]
 struct Installer<'a> {
     git: Git<'a>,
-    no_commit: bool,
+    commit: bool,
 }
 
 impl Installer<'_> {
@@ -275,7 +276,7 @@ impl Installer<'_> {
             std::iter::empty::<PathBuf>(),
         )?;
 
-        if !self.no_commit {
+        if self.commit {
             self.git.add(Some(path))?;
         }
 
@@ -522,7 +523,7 @@ mod tests {
     fn get_oz_tags() {
         let tmp = tempdir().unwrap();
         let git = Git::new(tmp.path());
-        let installer = Installer { git, no_commit: true };
+        let installer = Installer { git, commit: false };
 
         git.init().unwrap();
 

crates/forge/bin/cmd/install.rs

@@ -295,13 +295,13 @@ forgetest!(can_detect_dirty_git_status_on_init, |prj, cmd| {
     fs::create_dir_all(&nested).unwrap();
 
     cmd.current_dir(&nested);
-    cmd.arg("init").assert_failure().stderr_eq(str![[r#"
+    cmd.args(["init", "--commit"]).assert_failure().stderr_eq(str![[r#"
 Error: The target directory is a part of or on its own an already initialized git repository,
 and it requires clean working and staging areas, including no untracked files.
 
 Check the current git repository's status with `git status`.
 Then, you can track files with `git add ...` and then commit them with `git commit`,
-ignore them in the `.gitignore` file, or run this command again with the `--no-commit` flag.
+ignore them in the `.gitignore` file.
 
 "#]]);
 
@@ -584,10 +584,10 @@ Error: git fetch exited with code 128
 "#]]);
 });
 
-// checks that `forge init --template [template] work with --no-commit
+// checks that `forge init --template [template] works by default i.e without committing
 forgetest!(can_init_template_with_no_commit, |prj, cmd| {
     prj.wipe();
-    cmd.args(["init", "--template", "foundry-rs/forge-template", "--no-commit"])
+    cmd.args(["init", "--template", "foundry-rs/forge-template"])
         .arg(prj.root())
         .assert_success()
         .stdout_eq(str![[r#"
@@ -1308,14 +1308,13 @@ forgetest!(can_install_and_remove, |prj, cmd| {
     let forge_std_mod = git_mod.join("forge-std");
 
     let install = |cmd: &mut TestCommand| {
-        cmd.forge_fuse()
-            .args(["install", "foundry-rs/forge-std", "--no-commit"])
-            .assert_success()
-            .stdout_eq(str![[r#"
+        cmd.forge_fuse().args(["install", "foundry-rs/forge-std"]).assert_success().stdout_eq(
+            str![[r#"
 Installing forge-std in [..] (url: Some("https://github.com/foundry-rs/forge-std"), tag: None)
     Installed forge-std[..]
 
-"#]]);
+"#]],
+        );
 
         assert!(forge_std.exists());
         assert!(forge_std_mod.exists());
@@ -1376,14 +1375,13 @@ forgetest!(can_reinstall_after_manual_remove, |prj, cmd| {
     let forge_std_mod = git_mod.join("forge-std");
 
     let install = |cmd: &mut TestCommand| {
-        cmd.forge_fuse()
-            .args(["install", "foundry-rs/forge-std", "--no-commit"])
-            .assert_success()
-            .stdout_eq(str![[r#"
+        cmd.forge_fuse().args(["install", "foundry-rs/forge-std"]).assert_success().stdout_eq(
+            str![[r#"
 Installing forge-std in [..] (url: Some("https://github.com/foundry-rs/forge-std"), tag: None)
     Installed forge-std[..]
 
-"#]]);
+"#]],
+        );
 
         assert!(forge_std.exists());
         assert!(forge_std_mod.exists());
@@ -1446,7 +1444,7 @@ forgetest!(
 
         // install main dependency
         cmd.forge_fuse()
-            .args(["install", "evalir/forge-5980-test", "--no-commit"])
+            .args(["install", "evalir/forge-5980-test"])
             .assert_success()
             .stdout_eq(str![[r#"
 Installing forge-5980-test in [..] (url: Some("https://github.com/evalir/forge-5980-test"), tag: None)

crates/forge/tests/cli/cmd.rs

@@ -268,13 +268,11 @@ forgetest_init!(can_parse_remappings_correctly, |prj, cmd| {
     assert_eq!(expected, output);
 
     let install = |cmd: &mut TestCommand, dep: &str| {
-        cmd.forge_fuse().args(["install", dep, "--no-commit"]).assert_success().stdout_eq(str![[
-            r#"
+        cmd.forge_fuse().args(["install", dep]).assert_success().stdout_eq(str![[r#"
 Installing solmate in [..] (url: Some("https://github.com/transmissions11/solmate"), tag: None)
     Installed solmate[..]
 
-"#
-        ]]);
+"#]]);
     };
 
     install(&mut cmd, "transmissions11/solmate");
@@ -694,24 +692,19 @@ forgetest!(can_update_libs_section, |prj, cmd| {
     // explicitly set gas_price
     prj.update_config(|config| config.libs = vec!["node_modules".into()]);
 
-    cmd.args(["install", "foundry-rs/forge-std", "--no-commit"]).assert_success().stdout_eq(str![
-        [r#"
+    cmd.args(["install", "foundry-rs/forge-std"]).assert_success().stdout_eq(str![[r#"
 Installing forge-std in [..] (url: Some("https://github.com/foundry-rs/forge-std"), tag: None)
     Installed forge-std[..]
 
-"#]
-    ]);
+"#]]);
 
     let config = cmd.forge_fuse().config();
     // `lib` was added automatically
     let expected = vec![PathBuf::from("node_modules"), PathBuf::from("lib")];
     assert_eq!(config.libs, expected);
 
     // additional install don't edit `libs`
-    cmd.forge_fuse()
-        .args(["install", "dapphub/ds-test", "--no-commit"])
-        .assert_success()
-        .stdout_eq(str![[r#"
+    cmd.forge_fuse().args(["install", "dapphub/ds-test"]).assert_success().stdout_eq(str![[r#"
 Installing ds-test in [..] (url: Some("https://github.com/dapphub/ds-test"), tag: None)
     Installed ds-test
 
@@ -726,13 +719,11 @@ Installing ds-test in [..] (url: Some("https://github.com/dapphub/ds-test"), tag
 forgetest!(config_emit_warnings, |prj, cmd| {
     cmd.git_init();
 
-    cmd.args(["install", "foundry-rs/forge-std", "--no-commit"]).assert_success().stdout_eq(str![
-        [r#"
+    cmd.args(["install", "foundry-rs/forge-std"]).assert_success().stdout_eq(str![[r#"
 Installing forge-std in [..] (url: Some("https://github.com/foundry-rs/forge-std"), tag: None)
     Installed forge-std[..]
 
-"#]
-    ]);
+"#]]);
 
     let faulty_toml = r"[default]
     src = 'src'