git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCHv2 4/5] submodule--helper, module_clone: always operate on absolute paths

From
Stefan Beller <sbeller@google.com>
Date
Mar 31, 2016, 21:04 UTC
Message-ID
<1459458280-17619-5-git-send-email-sbeller@google.com>
In-Reply-To
<1459458280-17619-1-git-send-email-sbeller@google.com>

When giving relative paths to `relative_path` to compute a relative path from one directory to another, this may fail in `relative_path`. Make sure both arguments to `relative_path` are always absolute.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
Notes:
    Notice how the 2 relative path calls
      relative_path(sm_gitdir, path, &rel_path)
      relative_path(path, sm_gitdir, &rel_path)
    now have the same arguments, just switched?
    The symmetry there looks nice to read.
    
    This proposal is slightly more code than the previous patch,
    but looking at the end result
 builtin/submodule--helper.c | 36 +++++++++++++++++++++++-------------
 t/t7400-submodule-basic.sh  |  2 +-
 2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0b9f546..56c3033 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -157,8 +157,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	const char *reference = NULL, *depth = NULL;
 	int quiet = 0;
 	FILE *submodule_dot_git;
-	char *sm_gitdir, *cwd, *p;
-	struct strbuf rel_path = STRBUF_INIT;
+	char *sm_gitdir, *p;
+	struct strbuf rel_path = STRBUF_INIT; /* for relative_path */
 	struct strbuf sb = STRBUF_INIT;
 
 	struct option module_clone_options[] = {
@@ -200,6 +200,25 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
 	sm_gitdir = strbuf_detach(&sb, NULL);
 
+
+	if (!is_absolute_path(sm_gitdir)) {
+		char *cwd = xgetcwd();
+		strbuf_addf(&sb, "%s/%s", cwd, sm_gitdir);
+		sm_gitdir = strbuf_detach(&sb, 0);
+		free(cwd);
+	}
+
+	if (!is_absolute_path(path)) {
+		/*
+		 * TODO: add prefix here once we allow calling from non root
+		 * directory?
+		 */
+		strbuf_addf(&sb, "%s/%s",
+			    get_git_work_tree(),
+			    path);
+		path = strbuf_detach(&sb, 0);
+	}
+
 	if (!file_exists(sm_gitdir)) {
 		if (safe_create_leading_directories_const(sm_gitdir) < 0)
 			die(_("could not create directory '%s'"), sm_gitdir);
@@ -221,7 +240,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	submodule_dot_git = fopen(sb.buf, "w");
 	if (!submodule_dot_git)
 		die_errno(_("cannot open file '%s'"), sb.buf);
-
 	fprintf(submodule_dot_git, "gitdir: %s\n",
 		relative_path(sm_gitdir, path, &rel_path));
 	if (fclose(submodule_dot_git))
@@ -229,24 +247,16 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	strbuf_reset(&sb);
 	strbuf_reset(&rel_path);
 
-	cwd = xgetcwd();
 	/* Redirect the worktree of the submodule in the superproject's config */
-	if (!is_absolute_path(sm_gitdir)) {
-		strbuf_addf(&sb, "%s/%s", cwd, sm_gitdir);
-		free(sm_gitdir);
-		sm_gitdir = strbuf_detach(&sb, NULL);
-	}
-
-	strbuf_addf(&sb, "%s/%s", cwd, path);
 	p = git_pathdup_submodule(path, "config");
 	if (!p)
 		die(_("could not get submodule directory for '%s'"), path);
 	git_config_set_in_file(p, "core.worktree",
-			       relative_path(sb.buf, sm_gitdir, &rel_path));
+			       relative_path(path, sm_gitdir, &rel_path));
 	strbuf_release(&sb);
 	strbuf_release(&rel_path);
 	free(sm_gitdir);
-	free(cwd);
+
 	free(p);
 	return 0;
 }
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index fc11809..ea3fabb 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -818,7 +818,7 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano
 	)
 '
 
-test_expect_failure 'recursive relative submodules stay relative' '
+test_expect_success 'recursive relative submodules stay relative' '
 	test_when_finished "rm -rf super clone2 subsub sub3" &&
 	mkdir subsub &&
 	(
-- 
2.5.0.264.g39f00fe
Previous: Junio C HamanoNext: Junio C Hamano
Message 8 of 12 in “[PATCHv2 0/5] Fix relative path issues in recursive submodules.”
  1. Stefan BellerMar 31, 2016
  2. 1/5 recursive submodules: test for relative pathsStefan Beller, Mar 31, 2016
  3. 2/5 submodule--helper clone: simplify path checkStefan Beller, Mar 31, 2016
  4. Eric SunshineMar 31, 2016
  5. 3/5 submodule--helper clone: remove double path checkingStefan Beller, Mar 31, 2016
  6. Eric SunshineMar 31, 2016
  7. Junio C HamanoMar 31, 2016
  8. 4/5 submodule--helper, module_clone: always operate on absolute pathsStefan Beller, Mar 31, 2016
  9. Junio C HamanoMar 31, 2016
  10. Stefan BellerMar 31, 2016
  11. Stefan BellerMar 31, 2016
  12. 5/5 submodule--helper, module_clone: catch fprintf failureStefan Beller, Mar 31, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.