Fix #459, #781: Remove worktree code that changes the CWD

Add tests for simple use of submodules and worktrees together with editors from subdirectories. Closes #783 Closes #508
jonas · May 19, 2018 · d1bf123 · d1bf123
1 parent 0ace5cd
commit d1bf123
Show file tree

Hide file tree

Showing 15 changed files with 1,314 additions and 33 deletions.
diff --git a/NEWS.adoc b/NEWS.adoc
@@ -10,6 +10,7 @@ Improvements:
  - Update make config defaults for Cygwin to ncurses6. (GH #792)
  - Build against netbsd-curses. (GH #789)
  - Change the blame view to render more like `git blame`. (GH #812)
+ - Improve worktree and submodule support. (GH #459, #781, #783)
 
 Bug fixes:
 

diff --git a/doc/manual.adoc b/doc/manual.adoc
@@ -190,6 +190,7 @@ following variables.
 |%(repo:prefix)		|The path prefix of the current work directory,
 			 e.g `subdir/`.
 |%(repo:git-dir)	|The path to the Git directory, e.g. `/src/repo/.git`.
+|%(repo:worktree)	|The worktree path, if defined.
 |%(repo:is-inside-work-tree)
 			|Whether Tig is running inside a work tree,
 			 either `true` or `false`.

diff --git a/doc/tigrc.5.adoc b/doc/tigrc.5.adoc
@@ -705,6 +705,7 @@ following variable names, which are substituted before commands are run:
 |%(repo:prefix)		|The path prefix of the current work directory,
 			 e.g `subdir/`.
 |%(repo:git-dir)	|The path to the Git directory, e.g. `/src/repo/.git`.
+|%(repo:worktree)	|The worktree path, if defined.
 |%(repo:is-inside-work-tree)
 			|Whether Tig is running inside a work tree,
 			 either `true` or `false`.

diff --git a/include/tig/repo.h b/include/tig/repo.h
@@ -27,6 +27,7 @@ typedef char repo_str[SIZEOF_STR];
 	_(repo_str, cdup) \
 	_(repo_str, prefix) \
 	_(repo_str, git_dir) \
+	_(repo_str, worktree) \
 	_(bool,     is_inside_work_tree)
 
 #define REPO_INFO_FIELDS(type, name)	type name;

diff --git a/src/options.c b/src/options.c
@@ -1334,30 +1334,6 @@ set_repo_config_option(char *name, char *value, enum status_code (*cmd)(int, con
 		warn("Option 'tig.%s': %s", name, get_status_message(code));
 }
 
-static void
-set_work_tree(const char *value)
-{
-	char cwd[SIZEOF_STR];
-
-	if (!getcwd(cwd, sizeof(cwd)))
-		die("Failed to get cwd path: %s", strerror(errno));
-	if (chdir(cwd) < 0)
-		die("Failed to chdir(%s): %s", cwd, strerror(errno));
-	if (chdir(repo.git_dir) < 0)
-		die("Failed to chdir(%s): %s", repo.git_dir, strerror(errno));
-	if (!getcwd(repo.git_dir, sizeof(repo.git_dir)))
-		die("Failed to get git path: %s", strerror(errno));
-	if (chdir(value) < 0)
-		die("Failed to chdir(%s): %s", value, strerror(errno));
-	if (!getcwd(cwd, sizeof(cwd)))
-		die("Failed to get cwd path: %s", strerror(errno));
-	if (setenv("GIT_WORK_TREE", cwd, true))
-		die("Failed to set GIT_WORK_TREE to '%s'", cwd);
-	if (setenv("GIT_DIR", repo.git_dir, true))
-		die("Failed to set GIT_DIR to '%s'", repo.git_dir);
-	repo.is_inside_work_tree = true;
-}
-
 static struct line_info *
 parse_git_color_option(struct line_info *info, char *value)
 {
@@ -1449,7 +1425,7 @@ read_repo_config_option(char *name, size_t namelen, char *value, size_t valuelen
 		string_ncopy(opt_editor, value, valuelen);
 
 	else if (!strcmp(name, "core.worktree"))
-		set_work_tree(value);
+		string_ncopy(repo.worktree, value, valuelen);
 
 	else if (!strcmp(name, "core.abbrev"))
 		parse_int(&opt_id_width, value, 0, SIZEOF_REV - 1);

diff --git a/test/diff/editor-test b/test/diff/editor-test
@@ -54,6 +54,82 @@ test_case default \
 [diff] Changes to 'project/build.sbt' - line 224 of 367                    61%
 EOF
 
+test_case sub-directory \
+ --args='show ee912870202200a0b9cf4fd86ba57243212d341e' \
+ --before='mkdir -p some/subdir && cd some/subdir' \
+ --script="$script" <<EOF
+   | -  )
+   | -
+   |    lazy val parent: Project = Project(
+200|        id = "parent",
+   |        base = file("."),
+   | @@ -53,10 +48,10 @@ object ScalaJSBenchmarks extends Build {
+   |    )
+   |
+205|    lazy val common = project("Common", defaultSettings)
+   | -  lazy val deltablue = project("DeltaBlue", benchmarkSettings).dependsOn(c
+   | -  lazy val richards = project("Richards", benchmarkSettings).dependsOn(com
+   | -  lazy val sudoku = project("Sudoku", benchmarkSettings).dependsOn(common)
+   | -  lazy val tracer = project("Tracer", benchmarkSettings).dependsOn(common)
+210| +  lazy val deltablue = project("DeltaBlue", defaultSettings).dependsOn(com
+   | +  lazy val richards = project("Richards", defaultSettings).dependsOn(commo
+   | +  lazy val sudoku = project("Sudoku", defaultSettings).dependsOn(common)
+   | +  lazy val tracer = project("Tracer", defaultSettings).dependsOn(common)
+   |
+215|    def project(id: String, settings: Seq[sbt.Def.Setting[_]]) = Project(
+   |        id = id.toLowerCase,
+   | diff --git a/project/build.sbt b/project/build.sbt
+   | index 87c2e66..122abd0 100644
+   | --- a/project/build.sbt
+220| +++ b/project/build.sbt
+   | @@ -1 +1,4 @@
+   | -addSbtPlugin("org.scala-lang.modules.scalajs" % "scalajs-sbt-plugin" % "0.
+   | +resolvers += Resolver.url("scala-js-snapshots",
+   | +    url("http://repo.scala-js.org/repo/snapshots/"))(Resolver.ivyStylePatt
+[diff] Changes to 'project/build.sbt' - line 224 of 367                    61%
+EOF
+
+test_case sub-directory-diffstat \
+ --args='show ee912870202200a0b9cf4fd86ba57243212d341e' \
+ --before='mkdir -p some/subdir && cd some/subdir' \
+ --script='
+	:10
+	:edit
+	:26
+	:exec !vim %(file)
+	<C-p>
+' <<EOF
+  1| commit ee912870202200a0b9cf4fd86ba57243212d341e
+   | Refs: [master]
+   | Author:     Jonas Fonseca <jonas.fonseca@gmail.com>
+   | AuthorDate: Sat Mar 1 17:26:01 2014 -0500
+  5| Commit:     Jonas Fonseca <jonas.fonseca@gmail.com>
+   | CommitDate: Sat Mar 1 17:26:01 2014 -0500
+   |
+   |     WIP: Upgrade to 0.4-SNAPSHOT and DCE
+   | ---
+ 10|  common/benchmark-runner.sh                                  |  5 +++--
+   |  common/src/main/scala/org/scalajs/benchmark/Benchmark.scala | 11 +++++----
+   |  .../src/main/scala/org/scalajs/benchmark/BenchmarkApp.scala |  2 +-
+   |  common/start-benchmark.js                                   |  9 +++++++--
+   |  deltablue/exports.js                                        | 13 ---------
+ 15|  .../scala/org/scalajs/benchmark/deltablue/DeltaBlue.scala   |  7 +++++++
+   |  project/Build.scala                                         | 13 ++++-----
+   |  project/build.sbt                                           |  5 ++++-
+   |  richards/exports.js                                         | 13 ---------
+   |  .../scala/org/scalajs/benchmark/richards/Richards.scala     |  3 +++
+ 20|  run.sh                                                      |  2 +-
+   |  sudoku/exports.js                                           | 13 ---------
+   |  .../main/scala/org/scalajs/benchmark/sudoku/Sudoku.scala    |  2 ++
+   |  tracer/exports.js                                           | 13 ---------
+   |  tracer/index-dev.html                                       |  2 +-
+ 25|  tracer/index.html                                           |  2 +-
+   |  .../main/scala/org/scalajs/benchmark/tracer/Tracer.scala    |  3 +++
+   |  17 files changed, 42 insertions(+), 76 deletions(-)
+   |
+[diff] Press '<Enter>' to jump to file diff - line 25 of 367                  7%
+EOF
+
 # This settings doesn't seem to apply to `git-show`
 test_case mnemonic \
  --args='show ee912870202200a0b9cf4fd86ba57243212d341e' \
@@ -217,6 +293,22 @@ assert_equals 'editor.log' <<EOF
   lazy val deltablue = project("DeltaBlue", defaultSettings).dependsOn(common)
 +2 project/build.sbt
     url("http://repo.scala-js.org/repo/snapshots/"))(Resolver.ivyStylePatterns)
++51 project/Build.scala
+  lazy val deltablue = project("DeltaBlue", defaultSettings).dependsOn(common)
++2 project/build.sbt
+    url("http://repo.scala-js.org/repo/snapshots/"))(Resolver.ivyStylePatterns)
+common/benchmark-runner.sh
+#!/bin/sh
+#                     __
+#     ________ ___   / /  ___      __ ____  Scala.js Benchmarks
+#    / __/ __// _ | / /  / _ | __ / // __/  (c) 2013, Jonas Fonseca
+#  __\\ \\/ /__/ __ |/ /__/ __ |/_// /_\\ \\
+tracer/src/main/scala/org/scalajs/benchmark/tracer/Tracer.scala
+/*                     __                                               *\\
+**     ________ ___   / /  ___      __ ____  Scala.js Benchmarks        **
+**    / __/ __// _ | / /  / _ | __ / // __/  Adam Burmister             **
+**  __\\ \\/ /__/ __ |/ /__/ __ |/_// /_\\ \\    2012, Google, Inc          **
+** /____/\\___/_/ |_/____/_/ | |__/ /____/    2013, Jonas Fonseca        **
 +51 project/Build.scala
   lazy val deltablue = project("DeltaBlue", defaultSettings).dependsOn(common)
 +2 project/build.sbt

diff --git a/test/diff/submodule-editor-diffstat-test b/test/diff/submodule-editor-diffstat-test
@@ -0,0 +1,81 @@
+#!/bin/sh
+
+. libtest.sh
+. libgit.sh
+
+export LINES=22
+
+executable exec-env <<EOF
+#!/bin/sh
+
+{
+	printf ' - pwd='; pwd
+	for arg in git-dir show-cdup is-inside-work-tree show-superproject-working-tree; do
+		echo " - \$arg=\$(git rev-parse --\$arg)"
+	done
+} | sed "s,$output_dir,ROOT," >> "$output_dir/exec-env"
+EOF
+
+tigrc <<EOF
+set line-graphics = ascii
+set diff-view-line-number = yes
+bind diff <C-p> !exec-env
+EOF
+
+steps '
+	:9
+	:edit
+	:save-display diff.screen
+	<C-p>
+'
+
+setup_submodule_project()
+{
+	create_repo_from_tgz "$base_dir/files/repo-two.tgz"
+	git submodule update
+	mkdir -p repo-two-a/src/subdir
+}
+
+test_exec_work_dir setup_submodule_project
+work_dir="$work_dir/repo-two-a/src/subdir"
+
+test_tig show 8cc26dd6e80e2fa7fa98d12f4670d8983bc83678
+
+assert_equals 'diff.screen' <<EOF
+  1| commit 8cc26dd6e80e2fa7fa98d12f4670d8983bc83678
+   | Author:     A. U. Thor <a.u.thor@example.com>
+   | AuthorDate: Thu Jun 4 16:19:40 2009 +0000
+   | Commit:     Committer <c.ommitter@example.net>
+  5| CommitDate: Thu Jun 4 16:19:40 2009 +0000
+   |
+   |     [repo-two-a] Commit 3
+   | ---
+   |  include/api.h | 1 +
+ 10|  1 file changed, 1 insertion(+)
+   |
+   | diff --git a/include/api.h b/include/api.h
+   | index f2eb956..1da0f3f 100644
+   | --- a/include/api.h
+ 15| +++ b/include/api.h
+   | @@ -1 +1,2 @@
+   |  include/api.h
+   | +include/api.h
+ 
+ 
+[diff] Press '<Enter>' to jump to file diff - line 9 of 18                  100%
+EOF
+
+# The file is called include/api.h and contains its name on two lines
+assert_equals 'editor.log' <<EOF
+include/api.h
+include/api.h
+include/api.h
+EOF
+
+assert_equals 'exec-env' <<EOF
+ - pwd=ROOT/work dir/repo-two-a
+ - git-dir=ROOT/work dir/.git/modules/repo-two-a
+ - show-cdup=
+ - is-inside-work-tree=true
+ - show-superproject-working-tree=ROOT/work dir
+EOF