From cb0b9e590a6ea4479b638ee58326273a8b9d72fe Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Tue, 10 Jan 2023 12:52:06 +0900
Subject: [PATCH] cli: do not search ancestor paths specified by
 -R/--repository

If a workspace path is explicitly specified, it must point to the exact
workspace directory. This is the same behavior as 'hg -R'. OTOH, 'git -C'
is the option to chdir, so it makes sense to search .git from that directory.

This also fixes 'jj -R ../..' which would previously look up '../..', '..',
'.', ...
---
 CHANGELOG.md              |  3 +++
 src/cli_util.rs           | 17 ++++++++++-------
 tests/test_global_opts.rs | 20 ++++++++++++++++++++
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d89cf6c1d..2a5b386a8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -50,6 +50,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Commit description set by `-m`/`--message` is now terminated with a newline
   character, just like descriptions set by editor are.
 
+* The `-R`/`--repository` path must be a valid workspace directory. Its
+  ancestor directories are no longer searched.
+
 ### Contributors
 
 Thanks to the people who made this release happen!
diff --git a/src/cli_util.rs b/src/cli_util.rs
index c1fb64145..41de37c0c 100644
--- a/src/cli_util.rs
+++ b/src/cli_util.rs
@@ -977,15 +977,18 @@ fn init_workspace_loader(
     cwd: &Path,
     global_args: &GlobalArgs,
 ) -> Result<WorkspaceLoader, CommandError> {
-    let workspace_path_str = global_args.repository.as_deref().unwrap_or(".");
-    let workspace_path = cwd.join(workspace_path_str);
-    let workspace_root = workspace_path
-        .ancestors()
-        .find(|path| path.join(".jj").is_dir())
-        .unwrap_or(&workspace_path);
-    WorkspaceLoader::init(workspace_root).map_err(|err| match err {
+    let workspace_root = if let Some(path) = global_args.repository.as_ref() {
+        cwd.join(path)
+    } else {
+        cwd.ancestors()
+            .find(|path| path.join(".jj").is_dir())
+            .unwrap_or(cwd)
+            .to_owned()
+    };
+    WorkspaceLoader::init(&workspace_root).map_err(|err| match err {
         WorkspaceLoadError::NoWorkspaceHere(wc_path) => {
             // Prefer user-specified workspace_path_str instead of absolute wc_path.
+            let workspace_path_str = global_args.repository.as_deref().unwrap_or(".");
             let message = format!(r#"There is no jj repo in "{workspace_path_str}""#);
             let git_dir = wc_path.join(".git");
             if git_dir.is_dir() {
diff --git a/tests/test_global_opts.rs b/tests/test_global_opts.rs
index 1e68714ab..95da4e100 100644
--- a/tests/test_global_opts.rs
+++ b/tests/test_global_opts.rs
@@ -125,6 +125,26 @@ fn test_resolve_workspace_directory() {
     Working copy : 230dd059e1b0 (no description set)
     The working copy is clean
     "###);
+
+    // Explicit subdirectory path
+    let stderr = test_env.jj_cmd_failure(&subdir, &["status", "-R", "."]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: There is no jj repo in "."
+    "###);
+
+    // Valid explicit path
+    let stdout = test_env.jj_cmd_success(&subdir, &["status", "-R", "../.."]);
+    insta::assert_snapshot!(stdout, @r###"
+    Parent commit: 000000000000 (no description set)
+    Working copy : 230dd059e1b0 (no description set)
+    The working copy is clean
+    "###);
+
+    // "../../..".ancestors() contains "../..", but it should never be looked up.
+    let stderr = test_env.jj_cmd_failure(&subdir, &["status", "-R", "../../.."]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: There is no jj repo in "../../.."
+    "###);
 }
 
 #[test]