Re: [PATCH] submodule--helper.c: remove duplicate code
On Wed, Mar 8, 2017 at 9:44 AM, <me@vtolstov.org> wrote:
Show 16 quoted lines
> From: Valery Tolstov <me@vtolstov.org>
>
> Remove code fragment from module_clone that duplicates functionality
> of connect_work_tree_and_git_dir in dir.c
>
> Signed-off-by: Valery Tolstov <me@vtolstov.org>
> ---
>>> I think we can reuse code from module_clone that writes .git link.
>>> Possibly this code fragment needs to be factored out from module_clone
>>
>> That fragment already exists, see dir.h:
>> connect_work_tree_and_git_dir(work_tree, git_dir);
>> Maybe another good microproject is to use that in module_clone.
>
> By suggestion of Stefan Beller I would like to make this micro
> improvement as my microproject for GSoc.
Thanks for looking into this code deduplication!
Well these two parts of the code are subtly different, though. When applying this patch to origin/master (3bc53220cb2 is the last time I fetched from Junio), then
$ make
$ cd t
$ prove --timer --jobs 25 ./t[0-8][0-9]*.sh
... lots of output...
Test Summary Report
-------------------
./t3600-rm.sh (Wstat: 256 Tests: 79 Failed: 24)
Failed tests: 39-40, 43-59, 61-65
Non-zero exit status: 1
./t4059-diff-submodule-not-initialized.sh (Wstat: 256 Tests: 8 Failed: 5)
Failed tests: 4-8
Non-zero exit status: 1
./t7001-mv.sh (Wstat: 256 Tests: 47 Failed: 7)
Failed tests: 39-45
Non-zero exit status: 1
./t7412-submodule-absorbgitdirs.sh (Wstat: 256 Tests: 11 Failed: 6)
Failed tests: 3-7, 9
Non-zero exit status: 1
./t7400-submodule-basic.sh (Wstat: 256 Tests: 90 Failed: 13)
Failed tests: 46-47, 49, 75, 79-87
Non-zero exit status: 1
./t7406-submodule-update.sh (Wstat: 256 Tests: 52 Failed: 14)
Failed tests: 5, 28-31, 33-34, 36-39, 43, 45-46
Non-zero exit status: 1When then running one of them with debug output (See t/README for the debug flags, I remember divx, the video format as a sufficient set of flags to get enough debug output)
$ ./t7406-submodule-update.sh -d -i -v -x
...
++ cd foo
++ git submodule deinit -f sub
Cleared directory 'sub'
Submodule 'foo/sub' (/usr/local/google/home/sbeller/OSS/git/t/trash
directory.t7406-submodule-update/withsubs/../rebasing) unregistered
for path 'sub'
++ git submodule update --init sub
+ test_eval_ret_=1
+ want_trace
+ test t = t
+ test t = t
+ set +x
error: last command exited with $?=1
not ok 5 - submodule update --init from and of subdirectory
#
# git init withsubs &&
# (cd withsubs &&
# mkdir foo &&
# git submodule add "$(pwd)/../rebasing" foo/sub &&
# (cd foo &&
# git submodule deinit -f sub &&
# git submodule update --init sub 2>../../actual2
# )
# ) &&
# test_i18ncmp expect2 actual2
#
$ cd trash\ directory.t7406-submodule-update/withsubs/foo/ $ git submodule deinit -f sub Cleared directory 'sub' Submodule 'foo/sub' (/usr/local/google/home/sbeller/OSS/git/t/trash directory.t7406-submodule-update/withsubs/../rebasing) unregistered for path 'sub'
$ git submodule update --init sub Submodule 'foo/sub' (/usr/local/google/home/sbeller/OSS/git/t/trash directory.t7406-submodule-update/withsubs/../rebasing) registered for path 'sub' fatal: could not get submodule directory for '/usr/local/google/home/sbeller/OSS/git/t/trash directory.t7406-submodule-update/withsubs/foo/sub' Failed to clone 'foo/sub'. Retry scheduled fatal: could not get submodule directory for '/usr/local/google/home/sbeller/OSS/git/t/trash directory.t7406-submodule-update/withsubs/foo/sub' Failed to clone 'foo/sub' a second time, aborting
So I think we're missing to create the directory? (Just guessing)
Maybe we need to have 2293f77a081 (connect_work_tree_and_git_dir: safely create leading directories, part of origin/sb/checkout-recurse-submodules, also found at https://public-inbox.org/git/20170306205919.9713-8-sbeller@google.com/ ) first before we can apply this patch.
Thanks, Stefan
Show 50 quoted lines
>
> builtin/submodule--helper.c | 22 +++-------------------
> 1 file changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 899dc334e..cda8a3bc1 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -579,7 +579,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
> const char *name = NULL, *url = NULL, *depth = NULL;
> int quiet = 0;
> int progress = 0;
> - FILE *submodule_dot_git;
> char *p, *path = NULL, *sm_gitdir;
> struct strbuf rel_path = STRBUF_INIT;
> struct strbuf sb = STRBUF_INIT;
> @@ -653,27 +652,12 @@ static int module_clone(int argc, const char **argv, const char *prefix)
> strbuf_reset(&sb);
> }
>
> - /* Write a .git file in the submodule to redirect to the superproject. */
> - strbuf_addf(&sb, "%s/.git", path);
> - if (safe_create_leading_directories_const(sb.buf) < 0)
> - die(_("could not create leading directories of '%s'"), sb.buf);
> - submodule_dot_git = fopen(sb.buf, "w");
> - if (!submodule_dot_git)
> - die_errno(_("cannot open file '%s'"), sb.buf);
> -
> - fprintf_or_die(submodule_dot_git, "gitdir: %s\n",
> - relative_path(sm_gitdir, path, &rel_path));
> - if (fclose(submodule_dot_git))
> - die(_("could not close file %s"), sb.buf);
> - strbuf_reset(&sb);
> - strbuf_reset(&rel_path);
> -
> - /* Redirect the worktree of the submodule in the superproject's config */
> 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(path, sm_gitdir, &rel_path));
> +
> + /* Connect module worktree and git dir */
> + connect_work_tree_and_git_dir(path, sm_gitdir);
>
> /* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
> git_config_get_string("submodule.alternateLocation", &sm_alternate);
> --
> 2.12.0.190.g250ed7eaf
>
me@vtolstov.org
