goto: check for other in-progress commands and bail out

Summary:
Use the new repostate::command_state logic to bail out of "goto" if the working copy is in the middle of some other multi-step operation (e.g. "graft").

This involved tweaking things wrt the "updatemergestate" and "updatestate" state files owned by "goto".

Reviewed By: quark-zju

Differential Revision: D51771500

fbshipit-source-id: 322228930994e68f567d835f39a6d75226c8a3b3

eden/scm/lib/hgcommands/src/commands/goto.rs

@@ -8,6 +8,7 @@
 use std::io;
 
 use anyhow::bail;
+use anyhow::Context;
 use anyhow::Result;
 use clidispatch::abort;
 use clidispatch::fallback;
@@ -16,6 +17,7 @@ use cliparser::define_flags;
 use configmodel::ConfigExt;
 use fs_err as fs;
 use repo::repo::Repo;
+use repostate::command_state::Operation;
 use workingcopy::workingcopy::WorkingCopy;
 
 use super::MergeToolOpts;
@@ -78,61 +80,92 @@ pub fn run(ctx: ReqCtx<GotoOpts>, repo: &mut Repo, wc: &mut WorkingCopy) -> Resu
         fallback!("checkout.use-rust is False");
     }
 
-    // Protect the various ".hg" state file checks.
-    let _wlock = wc.lock();
+    let is_eden = repo.requirements.contains("eden");
 
-    if wc.dot_hg_path().join("updatemergestate").exists() {
-        tracing::debug!(target: "checkout_info", checkout_detail="updatemergestate");
-        fallback!("updatemergestate exists");
+    if (ctx.opts.clean != is_eden) || ctx.opts.check || ctx.opts.merge || !ctx.opts.date.is_empty()
+    {
+        tracing::debug!(target: "checkout_info", checkout_detail="unsupported_args");
+        fallback!("one or more unsupported options in Rust checkout");
+    }
+
+    if ctx.opts.clean && ctx.opts.r#continue {
+        abort!("can't specify both --clean and --continue")
     }
 
     let mut dest: Vec<String> = ctx.opts.args.clone();
     if !ctx.opts.rev.is_empty() {
         dest.push(ctx.opts.rev.clone());
     }
 
+    if !dest.is_empty() && ctx.opts.r#continue {
+        abort!("can't specify a destination commit and --continue");
+    }
+
+    // Protect the various ".hg" state file checks.
+    let wc_lock = wc.lock()?;
+
+    // Clean up the "updatemergestate" file if we are done merging.
+    // We do this before try_operation since that will error on "updatemergestate".
+    // This should happen even without "--continue".
+    let cleaned_mergestate = maybe_clear_update_merge_state(wc, ctx.opts.clean)?;
+
+    let updatestate_path = wc.dot_hg_path().join("updatestate");
+
     if ctx.opts.r#continue {
-        let interrupted_dest = match fs::read_to_string(wc.dot_hg_path().join("updatestate")) {
+        if cleaned_mergestate {
+            // User ran "sl goto --continue" after resolving all "--merge" conflicts.
+            return Ok(0);
+        }
+
+        let interrupted_dest = match fs::read_to_string(&updatestate_path) {
             Ok(data) => data,
             Err(err) if err.kind() == io::ErrorKind::NotFound => {
-                bail!("not in an interrupted update state")
+                bail!("not in an interrupted goto state")
             }
             Err(err) => return Err(err.into()),
         };
         dest.push(interrupted_dest);
     }
 
+    // We either consumed "updatestate" above, or are goto'ing someplace else,
+    // so clear it out.
+    util::file::unlink_if_exists(updatestate_path)?;
+
+    let op = if ctx.opts.clean {
+        Operation::GotoClean
+    } else {
+        Operation::Other
+    };
+
+    // This aborts if there are unresolved conflicts, or have some other
+    // operation (e.g. "graft") in progress.
+    repostate::command_state::try_operation(&wc_lock, op)?;
+
     if dest.len() > 1 {
         abort!(
-            "checkout requires exactly one destination commit but got: {:?}",
+            "goto requires exactly one destination commit but got: {:?}",
             dest
         );
     }
 
     if dest.is_empty() {
-        abort!(r#"You must specify a destination to update to, for example "@prog@ goto main"."#);
+        abort!(r#"you must specify a destination to update to, for example "@prog@ goto main"."#);
     }
 
     let dest = dest.remove(0);
-    let is_eden = repo.requirements.contains("eden");
-
-    if (ctx.opts.clean != is_eden) || ctx.opts.check || ctx.opts.merge || !ctx.opts.date.is_empty()
-    {
-        tracing::debug!(target: "checkout_info", checkout_detail="unsupported_args");
-        fallback!("one or more unsupported options in Rust checkout");
-    }
 
     let target = match repo.resolve_commit(Some(&wc.treestate().lock()), &dest) {
         Ok(target) => target,
-        Err(_) => {
+        Err(err) => {
             tracing::debug!(target: "checkout_info", checkout_detail="resolve_commit");
+            tracing::debug!(?err, dest, "unable to resolve checkout destination");
             fallback!("unable to resolve checkout destination");
         }
     };
 
     tracing::debug!(target: "checkout_info", checkout_mode="rust");
 
-    let _lock = repo.lock();
+    let _lock = repo.lock()?;
     let update_result = checkout::checkout(ctx.io(), repo, wc, target)?;
 
     if !ctx.global_opts().quiet {
@@ -149,6 +182,23 @@ pub fn run(ctx: ReqCtx<GotoOpts>, repo: &mut Repo, wc: &mut WorkingCopy) -> Resu
     Ok(0)
 }
 
+// Clear use out of the "updatemergestate" state if there are no unresolved
+// files or user specified "--clean". Returns whether state was cleared.
+fn maybe_clear_update_merge_state(wc: &WorkingCopy, clean: bool) -> Result<bool> {
+    let ums_path = wc.dot_hg_path().join("updatemergestate");
+
+    if !ums_path.try_exists().context("updatemergestate")? {
+        return Ok(false);
+    }
+
+    if clean || !wc.read_merge_state()?.unwrap_or_default().is_unresolved() {
+        fs_err::remove_file(&ums_path)?;
+        Ok(true)
+    } else {
+        Ok(false)
+    }
+}
+
 pub fn aliases() -> &'static str {
     "goto|go|update|up|checkout|co|upd|upda|updat|che|chec|check|checko|checkou"
 }

eden/scm/lib/workingcopy/repostate/src/command_state.rs

@@ -78,7 +78,9 @@ impl Conflict {
 
 impl Display for Conflict {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.0.description)?;
+        write!(f, "{}", self.description())?;
+        write!(f, "\n({})", self.hint())?;
+
         Ok(())
     }
 }
@@ -220,7 +222,7 @@ mod test {
         let err = try_operation(&locked_path, Operation::GotoClean).unwrap_err();
         let err: Conflict = err.downcast().unwrap();
         assert_eq!(
-            format!("{err}\n({})", err.hint()),
+            format!("{err}"),
             "goto --merge in progress
 (use 'sl goto --continue' to continue or
      'sl goto --clean' to abort - WARNING: will destroy uncommitted changes)"

eden/scm/tests/test-rust-checkout.t

@@ -33,19 +33,20 @@ Respect merge marker file:
   [1]
 
   $ hg go $B
-  abort: outstanding merge conflicts
-  (use 'hg resolve --list' to list, 'hg resolve --mark FILE' to mark resolved)
+  abort: goto --merge in progress
+  (use 'hg goto --continue' to continue or
+       'hg goto --clean' to abort - WARNING: will destroy uncommitted changes)
   [255]
 
 Run it again to make sure we didn't clear out state file:
   $ hg go $B
-  abort: outstanding merge conflicts
-  (use 'hg resolve --list' to list, 'hg resolve --mark FILE' to mark resolved)
+  abort: goto --merge in progress
+  (use 'hg goto --continue' to continue or
+       'hg goto --clean' to abort - WARNING: will destroy uncommitted changes)
   [255]
 
   $ hg go --continue
-  abort: outstanding merge conflicts
-  (use 'hg resolve --list' to list, 'hg resolve --mark FILE' to mark resolved)
+  abort: not in an interrupted goto state
   [255]
 
   $ hg resolve --mark foo
@@ -74,7 +75,7 @@ Can continue interrupted checkout:
   0000000000000000000000000000000000000000
 
   $ hg go --continue $A --rev $A
-  abort: checkout requires exactly one destination commit but got: ["a19fc4bcafede967b22a29cd9af839765fff19b7", "a19fc4bcafede967b22a29cd9af839765fff19b7", "a19fc4bcafede967b22a29cd9af839765fff19b7"]
+  abort: can't specify a destination commit and --continue
   [255]
 
   $ LOG=checkout=debug hg go -q --continue 2>&1 | grep skipped_count
@@ -84,7 +85,7 @@ Can continue interrupted checkout:
   @  a19fc4bcafed 'A'
 
   $ hg go --continue
-  abort: not in an interrupted update state
+  abort: not in an interrupted goto state
   [255]
 
 
@@ -107,3 +108,37 @@ Don't fail with open files that can't be deleted:
       update failed to remove foo: Can't remove file "*foo": The process cannot access the file because it is being used by another process. (os error 32)! (glob) (windows !)
       2 files updated, 0 files merged, 1 files removed, 0 files unresolved
 
+
+Respect other repo states:
+  $ newclientrepo
+  $ drawdag <<'EOS'
+  > B   # B/foo = two
+  > 
+  > A   # A/foo = one
+  > EOS
+
+  $ hg go -q $A
+  $ hg graft -r $B
+  grafting e57212eac5db "B"
+  merging foo
+  warning: 1 conflicts while merging foo! (edit, then use 'hg resolve --mark')
+  abort: unresolved conflicts, can't continue
+  (use 'hg resolve' and 'hg graft --continue')
+  [255]
+
+  $ hg go $B
+  abort: graft in progress
+  (use 'hg graft --continue' to continue or
+       'hg graft --abort' to abort)
+  [255]
+
+Various invalid arg combos:
+
+  $ newclientrepo
+  $ hg go foo --rev bar
+  abort: goto requires exactly one destination commit but got: ["foo", "bar"]
+  [255]
+
+  $ hg go
+  abort: you must specify a destination to update to, for example "hg goto main".
+  [255]