Merge pull request #4686 from tiennou/fix/more-worktree-from-bare

Fix git_worktree_validate failing on bare repositories

6 files changed, 117 insertions, 68 deletions

diff --git a/src/worktree.c b/src/worktree.c
index 591df1c34..610fd7ee3 100644
--- a/src/worktree.c
+++ b/src/worktree.c
@@ -234,40 +234,30 @@ void git_worktree_free(git_worktree *wt)
 
 int git_worktree_validate(const git_worktree *wt)
 {
-	git_buf buf = GIT_BUF_INIT;
-	int err = 0;
-
 	assert(wt);
 
-	git_buf_puts(&buf, wt->gitdir_path);
-	if (!is_worktree_dir(buf.ptr)) {
+	if (!is_worktree_dir(wt->gitdir_path)) {
 		giterr_set(GITERR_WORKTREE,
 			"Worktree gitdir ('%s') is not valid",
 			wt->gitlink_path);
-		err = -1;
-		goto out;
+		return GIT_ERROR;
 	}
 
-	if (!git_path_exists(wt->parent_path)) {
+	if (wt->parent_path && !git_path_exists(wt->parent_path)) {
 		giterr_set(GITERR_WORKTREE,
 			"Worktree parent directory ('%s') does not exist ",
 			wt->parent_path);
-		err = -2;
-		goto out;
+		return GIT_ERROR;
 	}
 
 	if (!git_path_exists(wt->commondir_path)) {
 		giterr_set(GITERR_WORKTREE,
 			"Worktree common directory ('%s') does not exist ",
 			wt->commondir_path);
-		err = -3;
-		goto out;
+		return GIT_ERROR;
 	}
 
-out:
-	git_buf_dispose(&buf);
-
-	return err;
+	return 0;
 }
 
 int git_worktree_add_init_options(git_worktree_add_options *opts,
diff --git a/tests/clar_libgit2.c b/tests/clar_libgit2.c
index 7ffa015a4..740264ccc 100644
--- a/tests/clar_libgit2.c
+++ b/tests/clar_libgit2.c
@@ -319,6 +319,35 @@ const char* cl_git_path_url(const char *path)
 	return url;
 }
 
+const char *cl_git_sandbox_path(int is_dir, ...)
+{
+	const char *path = NULL;
+	static char _temp[GIT_PATH_MAX];
+	git_buf buf = GIT_BUF_INIT;
+	va_list arg;
+
+	cl_git_pass(git_buf_sets(&buf, clar_sandbox_path()));
+
+	va_start(arg, is_dir);
+
+	while ((path = va_arg(arg, const char *)) != NULL) {
+		cl_git_pass(git_buf_joinpath(&buf, buf.ptr, path));
+	}
+	va_end(arg);
+
+	cl_git_pass(git_path_prettify(&buf, buf.ptr, NULL));
+	if (is_dir)
+		git_path_to_dir(&buf);
+
+	/* make sure we won't truncate */
+	cl_assert(git_buf_len(&buf) < sizeof(_temp));
+	git_buf_copy_cstr(_temp, sizeof(_temp), &buf);
+
+	git_buf_dispose(&buf);
+
+	return _temp;
+}
+
 typedef struct {
 	const char *filename;
 	size_t filename_len;
diff --git a/tests/clar_libgit2.h b/tests/clar_libgit2.h
index c72d37db3..618aed02d 100644
--- a/tests/clar_libgit2.h
+++ b/tests/clar_libgit2.h
@@ -181,6 +181,13 @@ git_repository *cl_git_sandbox_init_new(const char *name);
 void cl_git_sandbox_cleanup(void);
 git_repository *cl_git_sandbox_reopen(void);
 
+/*
+ * build a sandbox-relative from path segments
+ * is_dir will add a trailing slash
+ * vararg must be a NULL-terminated char * list
+ */
+const char *cl_git_sandbox_path(int is_dir, ...);
+
 /* Local-repo url helpers */
 const char* cl_git_fixture_url(const char *fixturename);
 const char* cl_git_path_url(const char *path);
diff --git a/tests/worktree/bare.c b/tests/worktree/bare.c
new file mode 100644
index 000000000..2a0e88239
--- /dev/null
+++ b/tests/worktree/bare.c
@@ -0,0 +1,72 @@
+#include "clar_libgit2.h"
+#include "worktree_helpers.h"
+#include "submodule/submodule_helpers.h"
+
+#define COMMON_REPO "testrepo.git"
+#define WORKTREE_REPO "worktree"
+
+static git_repository *g_repo;
+
+void test_worktree_bare__initialize(void)
+{
+	g_repo = cl_git_sandbox_init(COMMON_REPO);
+
+	cl_assert_equal_i(1, git_repository_is_bare(g_repo));
+	cl_assert_equal_i(0, git_repository_is_worktree(g_repo));
+}
+
+void test_worktree_bare__cleanup(void)
+{
+	cl_fixture_cleanup(WORKTREE_REPO);
+	cl_git_sandbox_cleanup();
+}
+
+void test_worktree_bare__list(void)
+{
+	git_strarray wts;
+
+	cl_git_pass(git_worktree_list(&wts, g_repo));
+	cl_assert_equal_i(wts.count, 0);
+
+	git_strarray_free(&wts);
+}
+
+void test_worktree_bare__add(void)
+{
+	git_worktree *wt;
+	git_repository *wtrepo;
+	git_strarray wts;
+
+	cl_git_pass(git_worktree_add(&wt, g_repo, "name", WORKTREE_REPO, NULL));
+
+	cl_git_pass(git_worktree_list(&wts, g_repo));
+	cl_assert_equal_i(wts.count, 1);
+
+	cl_git_pass(git_worktree_validate(wt));
+
+	cl_git_pass(git_repository_open(&wtrepo, WORKTREE_REPO));
+	cl_assert_equal_i(0, git_repository_is_bare(wtrepo));
+	cl_assert_equal_i(1, git_repository_is_worktree(wtrepo));
+
+	git_strarray_free(&wts);
+	git_worktree_free(wt);
+	git_repository_free(wtrepo);
+}
+
+void test_worktree_bare__repository_path(void)
+{
+	git_worktree *wt;
+	git_repository *wtrepo;
+
+	cl_git_pass(git_worktree_add(&wt, g_repo, "name", WORKTREE_REPO, NULL));
+	cl_assert_equal_s(git_worktree_path(wt), cl_git_sandbox_path(0, WORKTREE_REPO, NULL));
+
+	cl_git_pass(git_repository_open(&wtrepo, WORKTREE_REPO));
+	cl_assert_equal_s(git_repository_path(wtrepo), cl_git_sandbox_path(1, COMMON_REPO, "worktrees", "name", NULL));
+
+	cl_assert_equal_s(git_repository_commondir(g_repo), git_repository_commondir(wtrepo));
+	cl_assert_equal_s(git_repository_workdir(wtrepo), cl_git_sandbox_path(1, WORKTREE_REPO, NULL));
+
+	git_repository_free(wtrepo);
+	git_worktree_free(wt);
+}
diff --git a/tests/worktree/open.c b/tests/worktree/open.c
index 8ea43b2fa..52d47334c 100644
--- a/tests/worktree/open.c
+++ b/tests/worktree/open.c
@@ -11,28 +11,11 @@ static worktree_fixture fixture =
 
 static void assert_worktree_valid(git_repository *wt, const char *parentdir, const char *wtdir)
 {
-	git_buf path = GIT_BUF_INIT;
-
 	cl_assert(wt->is_worktree);
 
-	cl_git_pass(git_buf_joinpath(&path, clar_sandbox_path(), wtdir));
-	cl_git_pass(git_path_prettify(&path, path.ptr, NULL));
-	cl_git_pass(git_path_to_dir(&path));
-	cl_assert_equal_s(wt->workdir, path.ptr);
-
-	cl_git_pass(git_buf_joinpath(&path, path.ptr, ".git"));
-	cl_git_pass(git_path_prettify(&path, path.ptr, NULL));
-	cl_assert_equal_s(wt->gitlink, path.ptr);
-
-	cl_git_pass(git_buf_joinpath(&path, clar_sandbox_path(), parentdir));
-	cl_git_pass(git_buf_joinpath(&path, path.ptr, ".git"));
-	cl_git_pass(git_buf_joinpath(&path, path.ptr, "worktrees"));
-	cl_git_pass(git_buf_joinpath(&path, path.ptr, wtdir));
-	cl_git_pass(git_path_prettify(&path, path.ptr, NULL));
-	cl_git_pass(git_path_to_dir(&path));
-	cl_assert_equal_s(wt->gitdir, path.ptr);
-
-	git_buf_dispose(&path);
+	cl_assert_equal_s(wt->workdir, cl_git_sandbox_path(1, wtdir, NULL));
+	cl_assert_equal_s(wt->gitlink, cl_git_sandbox_path(0, wtdir, ".git", NULL));
+	cl_assert_equal_s(wt->gitdir, cl_git_sandbox_path(1, parentdir, ".git", "worktrees", wtdir, NULL));
 }
 
 void test_worktree_open__initialize(void)
diff --git a/tests/worktree/worktree.c b/tests/worktree/worktree.c
index 83be09760..6935f6005 100644
--- a/tests/worktree/worktree.c
+++ b/tests/worktree/worktree.c
@@ -84,18 +84,6 @@ void test_worktree_worktree__list_in_worktree_repo(void)
 	git_strarray_free(&wts);
 }
 
-void test_worktree_worktree__list_bare(void)
-{
-	git_repository *repo;
-	git_strarray wts;
-
-	repo = cl_git_sandbox_init("testrepo.git");
-	cl_git_pass(git_worktree_list(&wts, repo));
-	cl_assert_equal_i(wts.count, 0);
-
-	git_repository_free(repo);
-}
-
 void test_worktree_worktree__list_without_worktrees(void)
 {
 	git_repository *repo;
@@ -228,26 +216,6 @@ void test_worktree_worktree__init(void)
 	git_repository_free(repo);
 }
 
-void test_worktree_worktree__add_from_bare(void)
-{
-	git_worktree *wt;
-	git_repository *repo, *wtrepo;
-
-	repo = cl_git_sandbox_init("short_tag.git");
-
-	cl_assert_equal_i(1, git_repository_is_bare(repo));
-	cl_assert_equal_i(0, git_repository_is_worktree(repo));
-
-	cl_git_pass(git_worktree_add(&wt, repo, "worktree-frombare", "worktree-frombare", NULL));
-	cl_git_pass(git_repository_open(&wtrepo, "worktree-frombare"));
-	cl_assert_equal_i(0, git_repository_is_bare(wtrepo));
-	cl_assert_equal_i(1, git_repository_is_worktree(wtrepo));
-
-	git_worktree_free(wt);
-	git_repository_free(repo);
-	git_repository_free(wtrepo);
-}
-
 void test_worktree_worktree__add_locked(void)
 {
 	git_worktree *wt;