{"thread":{"id":"41892","subject":"[PATCHv3 0/2] Fix relative path issues in recursive submodules.","startedAt":"2016-04-01T00:17:27Z","lastAt":"2016-04-13T22:45:31Z","messageCount":15,"participants":["Stefan Beller","Ramsay Jones","Junio C Hamano","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"282414","messageId":"1459469849-9643-1-git-send-email-sbeller@google.com","threadId":"41892","inReplyTo":null,"subject":"[PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-01T00:17:27Z","receivedAt":"2016-04-01T00:17:27Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Thanks Junio for review!\n\nv3:\n * This is a resend of the last two patches of the series, i.e. it replaces\n 44859d6626d4 and efdef1e2e in sb/submodule-helper-clone-regression-fix\n \n * use absolute_path for sm_gitdir\n * removed todo\n * we need to free the intermediate sm_gitdir, so rename that to sm_gitdir_rel\n and free it afterwards.\n * while we currently do not support an absolute `path`, we eventually might.\n If `path` is absolute it would be a pointer to `argv`, which would lead to a\n crash. Duplicate the path and the crash is prevented.\n (* I thought we could use it as well for `path`, but we cannot; as \n get_git_work_tree() != cwd)\n * diff to sb/submodule-helper-clone-regression-fix:\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 89cbbda..be7bf5f 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -153,12 +153,13 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \n static int module_clone(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *path = NULL, *name = NULL, *url = NULL;\n+\tconst char *name = NULL, *url = NULL;\n \tconst char *reference = NULL, *depth = NULL;\n \tint quiet = 0;\n \tFILE *submodule_dot_git;\n-\tchar *sm_gitdir, *p;\n-\tstruct strbuf rel_path = STRBUF_INIT; /* for relative_path */\n+\tchar *sm_gitdir_rel, *p, *path = NULL;\n+\tconst char *sm_gitdir;\n+\tstruct strbuf rel_path = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tstruct option module_clone_options[] = {\n@@ -198,26 +199,14 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"submodule--helper: unspecified or empty --path\"));\n \n \tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n-\tsm_gitdir = strbuf_detach(&sb, NULL);\n-\n-\n-\tif (!is_absolute_path(sm_gitdir)) {\n-\t\tchar *cwd = xgetcwd();\n-\t\tstrbuf_addf(&sb, \"%s/%s\", cwd, sm_gitdir);\n-\t\tsm_gitdir = strbuf_detach(&sb, 0);\n-\t\tfree(cwd);\n-\t}\n+\tsm_gitdir_rel = strbuf_detach(&sb, NULL);\n+\tsm_gitdir = absolute_path(sm_gitdir_rel);\n \n \tif (!is_absolute_path(path)) {\n-\t\t/*\n-\t\t * TODO: add prefix here once we allow calling from non root\n-\t\t * directory?\n-\t\t */\n-\t\tstrbuf_addf(&sb, \"%s/%s\",\n-\t\t\t get_git_work_tree(),\n-\t\t\t path);\n+\t\tstrbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n \t\tpath = strbuf_detach(&sb, 0);\n-\t}\n+\t} else\n+\t\tpath = xstrdup(path);\n \n \tif (!file_exists(sm_gitdir)) {\n \t\tif (safe_create_leading_directories_const(sm_gitdir) < 0)\n@@ -240,6 +229,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tsubmodule_dot_git = fopen(sb.buf, \"w\");\n \tif (!submodule_dot_git)\n \t\tdie_errno(_(\"cannot open file '%s'\"), sb.buf);\n+\n \tfprintf_or_die(submodule_dot_git, \"gitdir: %s\\n\",\n \t\t relative_path(sm_gitdir, path, &rel_path));\n \tif (fclose(submodule_dot_git))\n@@ -255,8 +245,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\t\t relative_path(path, sm_gitdir, &rel_path));\n \tstrbuf_release(&sb);\n \tstrbuf_release(&rel_path);\n-\tfree(sm_gitdir);\n-\n+\tfree(sm_gitdir_rel);\n+\tfree(path);\n \tfree(p);\n \treturn 0;\n }\n\nv2:\n * reworded commit message for test (Thanks Junio!)\n * instead of \"simplifying\" the path handling in patch 2, we are prepared\n for anything the user throws at us (i.e. instead of segfault we\n die(_(\"submodule--helper: unspecified or empty --path\"));\n (Thanks Eric, Jacob for pushing back here!)\n * reword commit message for patch 3 (safe_create_leading_directories_const is\n not a check, Thanks Junio!)\n * patch 4 (the fix): That got reworked completely. No flow controlling\n conditions in the write out phase!\n * patch 5 is a bonus! (replace fprintf by fprintf_or die) When implementing\n that I wondered if we also have close_or_die and open_or_die, but that doesn't\n seem to be the case.\n\nThanks,\nStefan\n\nv1:\n\nIt took me longer than expected to fix this bug.\n\nIt comes with a test and minor refactoring and applies on top of\norigin/sb/submodule-helper, such that it can be merged into 2.7, 2.8 as well\nas master.\n\nPatch 1 is a test which fails; it has a similar layout as the\nreal failing repository Norio Nomura pointed out, but simplified to the\nbare essentials for reproduction of the bug.\n\nPatch 2&3 are not strictly necessary for fixing the isseu, but it removes\nstupid code I wrote, so the resulting code looks a little better.\n\nPatch 4 fixes the issue by giving more information to relative_path,\nsuch that a relative path can be found in all cases.\n\nThanks,\nStefan\n\nStefan Beller (2):\n submodule--helper, module_clone: always operate on absolute paths\n submodule--helper, module_clone: catch fprintf failure\n\n builtin/submodule--helper.c | 32 ++++++++++++++------------------\n t/t7400-submodule-basic.sh | 2 +-\n 2 files changed, 15 insertions(+), 19 deletions(-)\n\n-- \n2.5.0.262.g0ef869b.dirty\n"},{"id":"282415","messageId":"1459469849-9643-2-git-send-email-sbeller@google.com","threadId":"41892","inReplyTo":"1459469849-9643-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/2] submodule--helper, module_clone: always operate on absolute paths","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-01T00:17:28Z","receivedAt":"2016-04-01T00:17:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When giving relative paths to `relative_path` to compute a relative path\nfrom one directory to another, this may fail in `relative_path`.\nMake sure both arguments to `relative_path` are always absolute.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 28 ++++++++++++++--------------\n t/t7400-submodule-basic.sh | 2 +-\n 2 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 0b9f546..b099817 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -153,11 +153,12 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \n static int module_clone(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *path = NULL, *name = NULL, *url = NULL;\n+\tconst char *name = NULL, *url = NULL;\n \tconst char *reference = NULL, *depth = NULL;\n \tint quiet = 0;\n \tFILE *submodule_dot_git;\n-\tchar *sm_gitdir, *cwd, *p;\n+\tchar *sm_gitdir_rel, *p, *path = NULL;\n+\tconst char *sm_gitdir;\n \tstruct strbuf rel_path = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \n@@ -198,7 +199,14 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"submodule--helper: unspecified or empty --path\"));\n \n \tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n-\tsm_gitdir = strbuf_detach(&sb, NULL);\n+\tsm_gitdir_rel = strbuf_detach(&sb, NULL);\n+\tsm_gitdir = absolute_path(sm_gitdir_rel);\n+\n+\tif (!is_absolute_path(path)) {\n+\t\tstrbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n+\t\tpath = strbuf_detach(&sb, 0);\n+\t} else\n+\t\tpath = xstrdup(path);\n \n \tif (!file_exists(sm_gitdir)) {\n \t\tif (safe_create_leading_directories_const(sm_gitdir) < 0)\n@@ -229,24 +237,16 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tstrbuf_reset(&sb);\n \tstrbuf_reset(&rel_path);\n \n-\tcwd = xgetcwd();\n \t/* Redirect the worktree of the submodule in the superproject's config */\n-\tif (!is_absolute_path(sm_gitdir)) {\n-\t\tstrbuf_addf(&sb, \"%s/%s\", cwd, sm_gitdir);\n-\t\tfree(sm_gitdir);\n-\t\tsm_gitdir = strbuf_detach(&sb, NULL);\n-\t}\n-\n-\tstrbuf_addf(&sb, \"%s/%s\", cwd, path);\n \tp = git_pathdup_submodule(path, \"config\");\n \tif (!p)\n \t\tdie(_(\"could not get submodule directory for '%s'\"), path);\n \tgit_config_set_in_file(p, \"core.worktree\",\n-\t\t\t relative_path(sb.buf, sm_gitdir, &rel_path));\n+\t\t\t relative_path(path, sm_gitdir, &rel_path));\n \tstrbuf_release(&sb);\n \tstrbuf_release(&rel_path);\n-\tfree(sm_gitdir);\n-\tfree(cwd);\n+\tfree(sm_gitdir_rel);\n+\tfree(path);\n \tfree(p);\n \treturn 0;\n }\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex fc11809..ea3fabb 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -818,7 +818,7 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano\n \t)\n '\n \n-test_expect_failure 'recursive relative submodules stay relative' '\n+test_expect_success 'recursive relative submodules stay relative' '\n \ttest_when_finished \"rm -rf super clone2 subsub sub3\" &&\n \tmkdir subsub &&\n \t(\n-- \n2.5.0.264.gc776916.dirty\n"},{"id":"282416","messageId":"1459469849-9643-3-git-send-email-sbeller@google.com","threadId":"41892","inReplyTo":"1459469849-9643-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/2] submodule--helper, module_clone: catch fprintf failure","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-01T00:17:29Z","receivedAt":"2016-04-01T00:17:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The return value of fprintf is unchecked, which may lead to\nunreported errors. Use fprintf_or_die to report the error to the user.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex b099817..be7bf5f 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -230,8 +230,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tif (!submodule_dot_git)\n \t\tdie_errno(_(\"cannot open file '%s'\"), sb.buf);\n \n-\tfprintf(submodule_dot_git, \"gitdir: %s\\n\",\n-\t\trelative_path(sm_gitdir, path, &rel_path));\n+\tfprintf_or_die(submodule_dot_git, \"gitdir: %s\\n\",\n+\t\t relative_path(sm_gitdir, path, &rel_path));\n \tif (fclose(submodule_dot_git))\n \t\tdie(_(\"could not close file %s\"), sb.buf);\n \tstrbuf_reset(&sb);\n-- \n2.5.0.264.gc776916.dirty\n"},{"id":"282463","messageId":"56FE8898.20906@ramsayjones.plus.com","threadId":"41892","inReplyTo":"1459469849-9643-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-04-01T14:41:28Z","receivedAt":"2016-04-01T14:41:28Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 01/04/16 01:17, Stefan Beller wrote:\n> Thanks Junio for review!\n> \n> v3:\n> * This is a resend of the last two patches of the series, i.e. it replaces\n> 44859d6626d4 and efdef1e2e in sb/submodule-helper-clone-regression-fix\n> \n> * use absolute_path for sm_gitdir\n\nHi Stefan,\n\nIn response to v1 of this series, I sent you a fixup patch to suppress a\nsparse warning (submodule: don't use an integer as a NULL pointer, 21-02-2016).\n\nIn v2, you introduced a second identical warning (rather, for the same\nreason: using 0 as a NULL pointer as the second argument to strbuf_detach()).\n\nI was just about to send another patch, when you sent this out. As a result,\nyou have suppressed the new warning, but the original remains.\n\nSo, ...\n\n> * removed todo\n> * we need to free the intermediate sm_gitdir, so rename that to sm_gitdir_rel\n> and free it afterwards.\n> * while we currently do not support an absolute `path`, we eventually might.\n> If `path` is absolute it would be a pointer to `argv`, which would lead to a\n> crash. Duplicate the path and the crash is prevented.\n> (* I thought we could use it as well for `path`, but we cannot; as \n> get_git_work_tree() != cwd)\n> * diff to sb/submodule-helper-clone-regression-fix:\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 89cbbda..be7bf5f 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -153,12 +153,13 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n> \n> static int module_clone(int argc, const char **argv, const char *prefix)\n> {\n> -\tconst char *path = NULL, *name = NULL, *url = NULL;\n> +\tconst char *name = NULL, *url = NULL;\n> \tconst char *reference = NULL, *depth = NULL;\n> \tint quiet = 0;\n> \tFILE *submodule_dot_git;\n> -\tchar *sm_gitdir, *p;\n> -\tstruct strbuf rel_path = STRBUF_INIT; /* for relative_path */\n> +\tchar *sm_gitdir_rel, *p, *path = NULL;\n> +\tconst char *sm_gitdir;\n> +\tstruct strbuf rel_path = STRBUF_INIT;\n> \tstruct strbuf sb = STRBUF_INIT;\n> \n> \tstruct option module_clone_options[] = {\n> @@ -198,26 +199,14 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \t\tdie(_(\"submodule--helper: unspecified or empty --path\"));\n> \n> \tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n> -\tsm_gitdir = strbuf_detach(&sb, NULL);\n> -\n> -\n> -\tif (!is_absolute_path(sm_gitdir)) {\n> -\t\tchar *cwd = xgetcwd();\n> -\t\tstrbuf_addf(&sb, \"%s/%s\", cwd, sm_gitdir);\n> -\t\tsm_gitdir = strbuf_detach(&sb, 0);\n> -\t\tfree(cwd);\n> -\t}\n> +\tsm_gitdir_rel = strbuf_detach(&sb, NULL);\n\n... this is good, but ...\n\n> +\tsm_gitdir = absolute_path(sm_gitdir_rel);\n> \n> \tif (!is_absolute_path(path)) {\n> -\t\t/*\n> -\t\t * TODO: add prefix here once we allow calling from non root\n> -\t\t * directory?\n> -\t\t */\n> -\t\tstrbuf_addf(&sb, \"%s/%s\",\n> -\t\t\t get_git_work_tree(),\n> -\t\t\t path);\n> +\t\tstrbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n> \t\tpath = strbuf_detach(&sb, 0);\n\n... can you please fix this up.\n\nThanks!\n\nATB,\nRamsay Jones\n\n\n> -\t}\n> +\t} else\n> +\t\tpath = xstrdup(path);\n> \n> \tif (!file_exists(sm_gitdir)) {\n> \t\tif (safe_create_leading_directories_const(sm_gitdir) < 0)\n> @@ -240,6 +229,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \tsubmodule_dot_git = fopen(sb.buf, \"w\");\n> \tif (!submodule_dot_git)\n> \t\tdie_errno(_(\"cannot open file '%s'\"), sb.buf);\n> +\n> \tfprintf_or_die(submodule_dot_git, \"gitdir: %s\\n\",\n> \t\t relative_path(sm_gitdir, path, &rel_path));\n> \tif (fclose(submodule_dot_git))\n> @@ -255,8 +245,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \t\t\t relative_path(path, sm_gitdir, &rel_path));\n> \tstrbuf_release(&sb);\n> \tstrbuf_release(&rel_path);\n> -\tfree(sm_gitdir);\n> -\n> +\tfree(sm_gitdir_rel);\n> +\tfree(path);\n> \tfree(p);\n> \treturn 0;\n> }\n> \n> v2:\n> * reworded commit message for test (Thanks Junio!)\n> * instead of \"simplifying\" the path handling in patch 2, we are prepared\n> for anything the user throws at us (i.e. instead of segfault we\n> die(_(\"submodule--helper: unspecified or empty --path\"));\n> (Thanks Eric, Jacob for pushing back here!)\n> * reword commit message for patch 3 (safe_create_leading_directories_const is\n> not a check, Thanks Junio!)\n> * patch 4 (the fix): That got reworked completely. No flow controlling\n> conditions in the write out phase!\n> * patch 5 is a bonus! (replace fprintf by fprintf_or die) When implementing\n> that I wondered if we also have close_or_die and open_or_die, but that doesn't\n> seem to be the case.\n> \n> Thanks,\n> Stefan\n> \n> v1:\n> \n> It took me longer than expected to fix this bug.\n> \n> It comes with a test and minor refactoring and applies on top of\n> origin/sb/submodule-helper, such that it can be merged into 2.7, 2.8 as well\n> as master.\n> \n> Patch 1 is a test which fails; it has a similar layout as the\n> real failing repository Norio Nomura pointed out, but simplified to the\n> bare essentials for reproduction of the bug.\n> \n> Patch 2&3 are not strictly necessary for fixing the isseu, but it removes\n> stupid code I wrote, so the resulting code looks a little better.\n> \n> Patch 4 fixes the issue by giving more information to relative_path,\n> such that a relative path can be found in all cases.\n> \n> Thanks,\n> Stefan\n> \n> Stefan Beller (2):\n> submodule--helper, module_clone: always operate on absolute paths\n> submodule--helper, module_clone: catch fprintf failure\n> \n> builtin/submodule--helper.c | 32 ++++++++++++++------------------\n> t/t7400-submodule-basic.sh | 2 +-\n> 2 files changed, 15 insertions(+), 19 deletions(-)\n> \n"},{"id":"282498","messageId":"xmqq7fghi1ds.fsf@gitster.mtv.corp.google.com","threadId":"41892","inReplyTo":"1459469849-9643-2-git-send-email-sbeller@google.com","subject":"Re: [PATCH 1/2] submodule--helper, module_clone: always operate on absolute paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-01T19:18:23Z","receivedAt":"2016-04-01T19:18:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> +\tchar *sm_gitdir_rel, *p, *path = NULL;\n> +\tconst char *sm_gitdir;\n> \tstruct strbuf rel_path = STRBUF_INIT;\n> \tstruct strbuf sb = STRBUF_INIT;\n> \n> @@ -198,7 +199,14 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \t\tdie(_(\"submodule--helper: unspecified or empty --path\"));\n> \n> \tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n> -\tsm_gitdir = strbuf_detach(&sb, NULL);\n> +\tsm_gitdir_rel = strbuf_detach(&sb, NULL);\n> +\tsm_gitdir = absolute_path(sm_gitdir_rel);\n\nWith this change, sm_gitdir will always be absolute, which means the\nparameter to clone_submodule() call (immedately after this hunk)\nwill now be always absolute. I do not think \"git clone\" called from\nthere will leave anything in the resulting repository that depends\non the --separate-git-dir= being relative or absolute, so this\nchange should not cause us new problems.\n\nBy the way, this line is the last use of sm_gitdir_rel before it\ngets freed. I wonder\n\n\tsm_gitdir = absolute_path(sb.buf);\n\nwould be sufficient. We can lose the variable, and free() on it at\nthe end of this function, and an extra allocation if we can do so.\n\n> +\tif (!is_absolute_path(path)) {\n> +\t\tstrbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n> +\t\tpath = strbuf_detach(&sb, 0);\n> +\t} else\n> +\t\tpath = xstrdup(path);\n> \n> \tif (!file_exists(sm_gitdir)) {\n> \t\tif (safe_create_leading_directories_const(sm_gitdir) < 0)\n\nOther than that, looks good to me.\n\nThanks.\n"},{"id":"282499","messageId":"xmqq37r5i0tv.fsf@gitster.mtv.corp.google.com","threadId":"41892","inReplyTo":"xmqq7fghi1ds.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] submodule--helper, module_clone: always operate on absolute paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-01T19:30:20Z","receivedAt":"2016-04-01T19:30:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> By the way, this line is the last use of sm_gitdir_rel before it\n> gets freed. I wonder\n>\n> \tsm_gitdir = absolute_path(sb.buf);\n>\n> would be sufficient. We can lose the variable, and free() on it at\n> the end of this function, and an extra allocation if we can do so.\n>\n>> +\tif (!is_absolute_path(path)) {\n>> +\t\tstrbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n>> +\t\tpath = strbuf_detach(&sb, 0);\n>> +\t} else\n>> +\t\tpath = xstrdup(path);\n>> \n>> \tif (!file_exists(sm_gitdir)) {\n>> \t\tif (safe_create_leading_directories_const(sm_gitdir) < 0)\n>\n> Other than that, looks good to me.\n\nAnother thing I noticed is that it will not stay safe forever to\nborrow the result from absolute_path() for extended period of time.\nSo how about this one (and Ramsay's \"NULL must be spelled NULL, not\n0\") on top?\n\n-- >8 --\nFrom: Junio C Hamano \nDate: Fri, 1 Apr 2016 12:23:16 -0700\nSubject: [PATCH] submodule--helper: do not borrow absolute_path() result for too long\n\nabsolute_path() is designed to allow its callers to take a brief\npeek of the result (typically, to be fed to functions like\nstrbuf_add() and relative_path() as a parameter) without having to\nworry about freeing it, but the other side of the coin of that\nmemory model is that the caller shouldn't rely too much on the\nresult living forever--there may be a helper function the caller\nsubsequently calls that makes its own call to absolute_path(),\ninvalidating the earlier result.\n\nUse xstrdup() to make our own copy, and free(3) it when we are done.\nWhile at it, remove an unnecessary sm_gitdir_rel variable that was\nonly used to as a parameter to call absolute_paht() and never used\nagain.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/submodule--helper.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex b660a22..e69b340 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -157,8 +157,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \tconst char *reference = NULL, *depth = NULL;\n \tint quiet = 0;\n \tFILE *submodule_dot_git;\n-\tchar *sm_gitdir_rel, *p, *path = NULL;\n-\tconst char *sm_gitdir;\n+\tchar *p, *path = NULL, *sm_gitdir;\n \tstruct strbuf rel_path = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \n@@ -199,8 +198,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"submodule--helper: unspecified or empty --path\"));\n \n \tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n-\tsm_gitdir_rel = strbuf_detach(&sb, NULL);\n-\tsm_gitdir = absolute_path(sm_gitdir_rel);\n+\tsm_gitdir = xstrdup(absolute_path(sb.buf));\n \n \tif (!is_absolute_path(path)) {\n \t\tstrbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n@@ -245,7 +243,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\t\t relative_path(path, sm_gitdir, &rel_path));\n \tstrbuf_release(&sb);\n \tstrbuf_release(&rel_path);\n-\tfree(sm_gitdir_rel);\n+\tfree(sm_gitdir);\n \tfree(path);\n \tfree(p);\n \treturn 0;\n-- \n2.8.0-225-g297c00e\n"},{"id":"282502","messageId":"CAPig+cSDNLvBYLUc1VvPoS=dQX9P3YCCiZVHcZ91MqCHZpf_dg@mail.gmail.com","threadId":"41892","inReplyTo":"xmqq37r5i0tv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] submodule--helper, module_clone: always operate on absolute paths","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-01T20:09:40Z","receivedAt":"2016-04-01T20:09:40Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 1, 2016 at 3:30 PM, Junio C Hamano wrote:\n> Subject: [PATCH] submodule--helper: do not borrow absolute_path() result for too long\n>\n> absolute_path() is designed to allow its callers to take a brief\n> peek of the result (typically, to be fed to functions like\n> strbuf_add() and relative_path() as a parameter) without having to\n> worry about freeing it, but the other side of the coin of that\n> memory model is that the caller shouldn't rely too much on the\n> result living forever--there may be a helper function the caller\n> subsequently calls that makes its own call to absolute_path(),\n> invalidating the earlier result.\n>\n> Use xstrdup() to make our own copy, and free(3) it when we are done.\n> While at it, remove an unnecessary sm_gitdir_rel variable that was\n> only used to as a parameter to call absolute_paht() and never used\n\ns/absolute_paht/absolute_path/\n\n> again.\n>\n> Signed-off-by: Junio C Hamano \n"},{"id":"282504","messageId":"xmqqy48xgj2p.fsf@gitster.mtv.corp.google.com","threadId":"41892","inReplyTo":"xmqq37r5i0tv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] submodule--helper, module_clone: always operate on absolute paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-01T20:39:10Z","receivedAt":"2016-04-01T20:39:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> From: Junio C Hamano \n> Date: Fri, 1 Apr 2016 12:23:16 -0700\n> Subject: [PATCH] submodule--helper: do not borrow absolute_path() result for too long\n>\n> absolute_path() is designed to allow its callers to take a brief\n> peek of the result (typically, to be fed to functions like\n> strbuf_add() and relative_path() as a parameter) without having to\n> ...\n> @@ -199,8 +198,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \t\tdie(_(\"submodule--helper: unspecified or empty --path\"));\n> \n> \tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n> -\tsm_gitdir_rel = strbuf_detach(&sb, NULL);\n> -\tsm_gitdir = absolute_path(sm_gitdir_rel);\n> +\tsm_gitdir = xstrdup(absolute_path(sb.buf));\n\nJust to prevent others from wasting time scratching their heads,\nwe need\n\n\tstrbuf_reset(&sb);\n\nhere, as the strbuf will be reused later and is expected to be empty\nat this point.\n\n> \n> \tif (!is_absolute_path(path)) {\n> \t\tstrbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n> @@ -245,7 +243,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \t\t\t relative_path(path, sm_gitdir, &rel_path));\n> \tstrbuf_release(&sb);\n> \tstrbuf_release(&rel_path);\n> -\tfree(sm_gitdir_rel);\n> +\tfree(sm_gitdir);\n> \tfree(path);\n> \tfree(p);\n> \treturn 0;\n"},{"id":"283194","messageId":"CAGZ79kYE3E7stNc3Xbzr-qvncLGt4qRPMbv897k5h0B50-Rogg@mail.gmail.com","threadId":"41892","inReplyTo":"56FE8898.20906@ramsayjones.plus.com","subject":"Re: [PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-12T15:58:23Z","receivedAt":"2016-04-12T15:58:23Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 1, 2016 at 7:41 AM, Ramsay Jones\n wrote:\n>\n>\n> On 01/04/16 01:17, Stefan Beller wrote:\n>> Thanks Junio for review!\n>>\n>> v3:\n>> * This is a resend of the last two patches of the series, i.e. it replaces\n>> 44859d6626d4 and efdef1e2e in sb/submodule-helper-clone-regression-fix\n>>\n>> * use absolute_path for sm_gitdir\n>\n> Hi Stefan,\n>\n> In response to v1 of this series, I sent you a fixup patch to suppress a\n> sparse warning (submodule: don't use an integer as a NULL pointer, 21-02-2016).\n>\n> In v2, you introduced a second identical warning (rather, for the same\n> reason: using 0 as a NULL pointer as the second argument to strbuf_detach()).\n>\n> I was just about to send another patch, when you sent this out. As a result,\n> you have suppressed the new warning, but the original remains.\n>\n> So, ...\n>\n>> * removed todo\n>> * we need to free the intermediate sm_gitdir, so rename that to sm_gitdir_rel\n>> and free it afterwards.\n>> * while we currently do not support an absolute `path`, we eventually might.\n>> If `path` is absolute it would be a pointer to `argv`, which would lead to a\n>> crash. Duplicate the path and the crash is prevented.\n>> (* I thought we could use it as well for `path`, but we cannot; as\n>> get_git_work_tree() != cwd)\n>> * diff to sb/submodule-helper-clone-regression-fix:\n>>\n>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n>> index 89cbbda..be7bf5f 100644\n>> --- a/builtin/submodule--helper.c\n>> +++ b/builtin/submodule--helper.c\n>> @@ -153,12 +153,13 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n>>\n>> static int module_clone(int argc, const char **argv, const char *prefix)\n>> {\n>> - const char *path = NULL, *name = NULL, *url = NULL;\n>> + const char *name = NULL, *url = NULL;\n>> const char *reference = NULL, *depth = NULL;\n>> int quiet = 0;\n>> FILE *submodule_dot_git;\n>> - char *sm_gitdir, *p;\n>> - struct strbuf rel_path = STRBUF_INIT; /* for relative_path */\n>> + char *sm_gitdir_rel, *p, *path = NULL;\n>> + const char *sm_gitdir;\n>> + struct strbuf rel_path = STRBUF_INIT;\n>> struct strbuf sb = STRBUF_INIT;\n>>\n>> struct option module_clone_options[] = {\n>> @@ -198,26 +199,14 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n>> die(_(\"submodule--helper: unspecified or empty --path\"));\n>>\n>> strbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n>> - sm_gitdir = strbuf_detach(&sb, NULL);\n>> -\n>> -\n>> - if (!is_absolute_path(sm_gitdir)) {\n>> - char *cwd = xgetcwd();\n>> - strbuf_addf(&sb, \"%s/%s\", cwd, sm_gitdir);\n>> - sm_gitdir = strbuf_detach(&sb, 0);\n>> - free(cwd);\n>> - }\n>> + sm_gitdir_rel = strbuf_detach(&sb, NULL);\n>\n> ... this is good, but ...\n>\n>> + sm_gitdir = absolute_path(sm_gitdir_rel);\n>>\n>> if (!is_absolute_path(path)) {\n>> - /*\n>> - * TODO: add prefix here once we allow calling from non root\n>> - * directory?\n>> - */\n>> - strbuf_addf(&sb, \"%s/%s\",\n>> - get_git_work_tree(),\n>> - path);\n>> + strbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n>> path = strbuf_detach(&sb, 0);\n>\n> ... can you please fix this up.\n>\n> Thanks!\n>\n> ATB,\n> Ramsay Jones\n\nLooking at the current code of origin/sb/submodule-helper-clone-regression-fix\nwe do not have this issue there, but I'll keep it in mind for a resend.\n\n>\n>\n>> - }\n>> + } else\n>> + path = xstrdup(path);\n>>\n>> if (!file_exists(sm_gitdir)) {\n>> if (safe_create_leading_directories_const(sm_gitdir) < 0)\n>> @@ -240,6 +229,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n>> submodule_dot_git = fopen(sb.buf, \"w\");\n>> if (!submodule_dot_git)\n>> die_errno(_(\"cannot open file '%s'\"), sb.buf);\n>> +\n>> fprintf_or_die(submodule_dot_git, \"gitdir: %s\\n\",\n>> relative_path(sm_gitdir, path, &rel_path));\n>> if (fclose(submodule_dot_git))\n>> @@ -255,8 +245,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n>> relative_path(path, sm_gitdir, &rel_path));\n>> strbuf_release(&sb);\n>> strbuf_release(&rel_path);\n>> - free(sm_gitdir);\n>> -\n>> + free(sm_gitdir_rel);\n>> + free(path);\n>> free(p);\n>> return 0;\n>> }\n>>\n>> v2:\n>> * reworded commit message for test (Thanks Junio!)\n>> * instead of \"simplifying\" the path handling in patch 2, we are prepared\n>> for anything the user throws at us (i.e. instead of segfault we\n>> die(_(\"submodule--helper: unspecified or empty --path\"));\n>> (Thanks Eric, Jacob for pushing back here!)\n>> * reword commit message for patch 3 (safe_create_leading_directories_const is\n>> not a check, Thanks Junio!)\n>> * patch 4 (the fix): That got reworked completely. No flow controlling\n>> conditions in the write out phase!\n>> * patch 5 is a bonus! (replace fprintf by fprintf_or die) When implementing\n>> that I wondered if we also have close_or_die and open_or_die, but that doesn't\n>> seem to be the case.\n>>\n>> Thanks,\n>> Stefan\n>>\n>> v1:\n>>\n>> It took me longer than expected to fix this bug.\n>>\n>> It comes with a test and minor refactoring and applies on top of\n>> origin/sb/submodule-helper, such that it can be merged into 2.7, 2.8 as well\n>> as master.\n>>\n>> Patch 1 is a test which fails; it has a similar layout as the\n>> real failing repository Norio Nomura pointed out, but simplified to the\n>> bare essentials for reproduction of the bug.\n>>\n>> Patch 2&3 are not strictly necessary for fixing the isseu, but it removes\n>> stupid code I wrote, so the resulting code looks a little better.\n>>\n>> Patch 4 fixes the issue by giving more information to relative_path,\n>> such that a relative path can be found in all cases.\n>>\n>> Thanks,\n>> Stefan\n>>\n>> Stefan Beller (2):\n>> submodule--helper, module_clone: always operate on absolute paths\n>> submodule--helper, module_clone: catch fprintf failure\n>>\n>> builtin/submodule--helper.c | 32 ++++++++++++++------------------\n>> t/t7400-submodule-basic.sh | 2 +-\n>> 2 files changed, 15 insertions(+), 19 deletions(-)\n>>\n"},{"id":"283403","messageId":"570E9C46.6080905@ramsayjones.plus.com","threadId":"41892","inReplyTo":"CAGZ79kYE3E7stNc3Xbzr-qvncLGt4qRPMbv897k5h0B50-Rogg@mail.gmail.com","subject":"Re: [PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-04-13T19:21:42Z","receivedAt":"2016-04-13T19:21:42Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 12/04/16 16:58, Stefan Beller wrote:\n> On Fri, Apr 1, 2016 at 7:41 AM, Ramsay Jones\n> wrote:\n>>\n[snip[\n\n>>> - }\n>>> + sm_gitdir_rel = strbuf_detach(&sb, NULL);\n>>\n>> ... this is good, but ...\n>>\n>>> + sm_gitdir = absolute_path(sm_gitdir_rel);\n>>>\n>>> if (!is_absolute_path(path)) {\n>>> - /*\n>>> - * TODO: add prefix here once we allow calling from non root\n>>> - * directory?\n>>> - */\n>>> - strbuf_addf(&sb, \"%s/%s\",\n>>> - get_git_work_tree(),\n>>> - path);\n>>> + strbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n>>> path = strbuf_detach(&sb, 0);\n>>\n>> ... can you please fix this up.\n>>\n>> Thanks!\n>>\n>> ATB,\n>> Ramsay Jones\n> \n> Looking at the current code of origin/sb/submodule-helper-clone-regression-fix\n> we do not have this issue there, but I'll keep it in mind for a resend.\n\nHmm, actually, the above change wasn't the original culprit (as I thought), but\na different instance of the same fault. :-D\n\nI've lost track of which version is now in 'pu' (currently @ 45a4edc \"Merge branch\n'sb/submodule-init' into pu\"), but sparse is still warning:\n\n SP submodule.c\n submodule.c:256:43: warning: Using plain integer as NULL pointer\n\nSo, the fix looks like:\n\n diff --git a/submodule.c b/submodule.c\n index 5d1238a..4cc1c27 100644\n --- a/submodule.c\n +++ b/submodule.c\n @@ -253,7 +253,7 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy\n return NULL;\n case SM_UPDATE_COMMAND:\n strbuf_addf(&sb, \"!%s\", s->command);\n - return strbuf_detach(&sb, 0);\n + return strbuf_detach(&sb, NULL);\n }\n return NULL;\n }\n\nAlso, I note that t7406-submodule-update.sh test #4 is failing.\n(looks like absolute vs relative paths)\n\nATB,\nRamsay Jones\n"},{"id":"283404","messageId":"CAGZ79kYj+4_6BsrcRRzsvdp_FcRG9VuiJ1tLjSo3D5_uWe2nfQ@mail.gmail.com","threadId":"41892","inReplyTo":"570E9C46.6080905@ramsayjones.plus.com","subject":"Re: [PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-13T20:34:55Z","receivedAt":"2016-04-13T20:34:55Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Apr 13, 2016 at 12:21 PM, Ramsay Jones\n wrote:\n>\n>\n> On 12/04/16 16:58, Stefan Beller wrote:\n>> On Fri, Apr 1, 2016 at 7:41 AM, Ramsay Jones\n>> wrote:\n>>>\n> [snip[\n>\n>>>> - }\n>>>> + sm_gitdir_rel = strbuf_detach(&sb, NULL);\n>>>\n>>> ... this is good, but ...\n>>>\n>>>> + sm_gitdir = absolute_path(sm_gitdir_rel);\n>>>>\n>>>> if (!is_absolute_path(path)) {\n>>>> - /*\n>>>> - * TODO: add prefix here once we allow calling from non root\n>>>> - * directory?\n>>>> - */\n>>>> - strbuf_addf(&sb, \"%s/%s\",\n>>>> - get_git_work_tree(),\n>>>> - path);\n>>>> + strbuf_addf(&sb, \"%s/%s\", get_git_work_tree(), path);\n>>>> path = strbuf_detach(&sb, 0);\n>>>\n>>> ... can you please fix this up.\n>>>\n>>> Thanks!\n>>>\n>>> ATB,\n>>> Ramsay Jones\n>>\n>> Looking at the current code of origin/sb/submodule-helper-clone-regression-fix\n>> we do not have this issue there, but I'll keep it in mind for a resend.\n>\n> Hmm, actually, the above change wasn't the original culprit (as I thought), but\n> a different instance of the same fault. :-D\n>\n> I've lost track of which version is now in 'pu' (currently @ 45a4edc \"Merge branch\n> 'sb/submodule-init' into pu\"), but sparse is still warning:\n>\n> SP submodule.c\n> submodule.c:256:43: warning: Using plain integer as NULL pointer\n>\n> So, the fix looks like:\n>\n> diff --git a/submodule.c b/submodule.c\n> index 5d1238a..4cc1c27 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -253,7 +253,7 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy\n> return NULL;\n> case SM_UPDATE_COMMAND:\n> strbuf_addf(&sb, \"!%s\", s->command);\n> - return strbuf_detach(&sb, 0);\n> + return strbuf_detach(&sb, NULL);\n> }\n> return NULL;\n> }\n>\n> Also, I note that t7406-submodule-update.sh test #4 is failing.\n> (looks like absolute vs relative paths)\n>\n> ATB,\n> Ramsay Jones\n>\n\nOk fixed this instance here, too.\nI'll hunt down the path issue now.\n\n>\n>\n"},{"id":"283408","messageId":"xmqqoa9djk60.fsf@gitster.mtv.corp.google.com","threadId":"41892","inReplyTo":"570E9C46.6080905@ramsayjones.plus.com","subject":"Re: [PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-13T21:03:35Z","receivedAt":"2016-04-13T21:03:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Also, I note that t7406-submodule-update.sh test #4 is failing.\n> (looks like absolute vs relative paths)\n\nI think that is $gmane/291363\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/291334/focus=291363\n"},{"id":"283422","messageId":"xmqqzisxi1w0.fsf@gitster.mtv.corp.google.com","threadId":"41892","inReplyTo":"CAGZ79kYj+4_6BsrcRRzsvdp_FcRG9VuiJ1tLjSo3D5_uWe2nfQ@mail.gmail.com","subject":"Re: [PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-13T22:23:43Z","receivedAt":"2016-04-13T22:23:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>\n> Ok fixed this instance here, too.\n\n... So... should I hold sb/submodule-helper-clone-regression-fix?\nIt has been marked to be merged to 'next' for some time now.\n"},{"id":"283426","messageId":"CAGZ79kbh4q-w6yt6eHrvcGvH6mhmCJG1WdRSdO6bgpX=iLwARw@mail.gmail.com","threadId":"41892","inReplyTo":"xmqqzisxi1w0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-13T22:30:22Z","receivedAt":"2016-04-13T22:30:22Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Apr 13, 2016 at 3:23 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>>\n>> Ok fixed this instance here, too.\n>\n> ... So... should I hold sb/submodule-helper-clone-regression-fix?\n> It has been marked to be merged to 'next' for some time now.\n\n(Both things Ramsay pointed at are in submodule.c, but\nsb/submodule-helper-clone-regression-fix never touches that file,\nso Ramsay talks about submodule-init here? I agree\nsubmodule-init is faulty and I am fixing/rewriting it now.)\n\nThis series (sb/submodule-helper-clone-regression-fix), has no issues\n(on its own as well as playing together with any other submodule\nseries in flight IIUC), so what is your concern for holding instead of merging?\n"},{"id":"283431","messageId":"xmqqmvoxi0vo.fsf@gitster.mtv.corp.google.com","threadId":"41892","inReplyTo":"CAGZ79kbh4q-w6yt6eHrvcGvH6mhmCJG1WdRSdO6bgpX=iLwARw@mail.gmail.com","subject":"Re: [PATCHv3 0/2] Fix relative path issues in recursive submodules.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-13T22:45:31Z","receivedAt":"2016-04-13T22:45:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> ... so what is your concern for holding instead of merging?\n\nNothing specific. Remember, I may be aware of all the discussion\nthreads but I am certainly not reading every word in every thread.\nWhen the participants are trustworthy enough, instead of going back\nto the list archive (and risk overlooking a message that asks\n\"please hold off merging this, I have another last-minute update\")\nto see if everything known has been resolved in a satisfactory way\nmyself, I'd just ask, which is more efficient _and_ reliable.\n"}]}