Re: [PATCHv2 4/5] submodule--helper, module_clone: always operate on absolute paths
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 31, 2016, 22:26 UTC
- Message-ID
- <xmqqfuv6l1xf.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1459458280-17619-5-git-send-email-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 5 quoted lines
> + 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);It would be surprising that this would be the first codepath that needs to get an absolute pathname in the codebase that is more than 10 years old, wouldn't it? Don't we have a reasonable API helper function to do this kind of thing already?
... goes and looks ...
Doesn't absolute_path() work here?
Show 8 quoted lines
> @@ -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))Looks like an unrelated change to me.
Show 24 quoted lines
> @@ -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);
> +This addition of blank, too.
Show 16 quoted lines
> 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 && > (