clone: pass --progress decision to recursive submodules

When cloning with "--recursive", we'd generally expect
submodules to show progress reports if the main clone did,
too.

In older versions of git, this mostly worked out of the
box. Since we show progress by default when stderr is a tty,
and since the child clones inherit the parent stderr, then
both processes would come to the same decision by default.
If the parent clone was asked for "--quiet", we passed down
"--quiet" to the child. However, if stderr was not a tty and
the user specified "--progress", we did not propagate this
to the child.

That's a minor bug, but things got much worse when we
switched recently to submodule--helper's update_clone
command. With that change, the stderr of the child clones
are always connected to a pipe, and we never output
progress at all.

This patch teaches git-submodule and git-submodule--helper
how to pass down an explicit "--progress" flag when cloning.
The clone command then decides to propagate that flag based
on the cloning decision made earlier (which takes into
account isatty(2) of the parent process, existing --progress
or --quiet flags, etc). Since the child processes always run
without a tty on stderr, we don't have to worry about
passing an explicit "--no-progress"; it's the default for
them.

This fixes the recent loss of progress during recursive
clones. And as a bonus, it makes:

git clone --recursive --progress ... 2>&1 | cat

work by triggering progress explicitly in the children.

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/clone.c b/builtin/clone.c
index 404c5e80226c81a8f051e63fa8e40286fea95a18..28ce9383a14ced7ea3f135bdc0d42226aaf790af 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -670,7 +670,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
        }
 }
 
        }
 }
 

-static int checkout(void)
+static int checkout(int submodule_progress)

 {
        unsigned char sha1[20];
        char *head;
 {
        unsigned char sha1[20];
        char *head;


@@ -734,6 +734,9 @@ static int checkout(void)
                if (max_jobs != -1)
                        argv_array_pushf(&args, "--jobs=%d", max_jobs);
 
                if (max_jobs != -1)
                        argv_array_pushf(&args, "--jobs=%d", max_jobs);
 

+               if (submodule_progress)
+                       argv_array_push(&args, "--progress");
+

                err = run_command_v_opt(args.argv, RUN_GIT_CMD);
                argv_array_clear(&args);
        }
                err = run_command_v_opt(args.argv, RUN_GIT_CMD);
                argv_array_clear(&args);
        }


@@ -841,6 +844,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
        const char *src_ref_prefix = "refs/heads/";
        struct remote *remote;
        int err = 0, complete_refs_before_fetch = 1;
        const char *src_ref_prefix = "refs/heads/";
        struct remote *remote;
        int err = 0, complete_refs_before_fetch = 1;

+       int submodule_progress;

 
        struct refspec *refspec;
        const char *fetch_pattern;
 
        struct refspec *refspec;
        const char *fetch_pattern;


@@ -1099,6 +1103,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
        update_head(our_head_points_at, remote_head, reflog_msg.buf);
 
 
        update_head(our_head_points_at, remote_head, reflog_msg.buf);
 

+       /*
+        * We want to show progress for recursive submodule clones iff
+        * we did so for the main clone. But only the transport knows
+        * the final decision for this flag, so we need to rescue the value
+        * before we free the transport.
+        */
+       submodule_progress = transport->progress;
+

        transport_unlock_pack(transport);
        transport_disconnect(transport);
 
        transport_unlock_pack(transport);
        transport_disconnect(transport);
 


@@ -1108,7 +1120,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
        }
 
        junk_mode = JUNK_LEAVE_REPO;
        }
 
        junk_mode = JUNK_LEAVE_REPO;

-       err = checkout();
+       err = checkout(submodule_progress);

 
        strbuf_release(&reflog_msg);
        strbuf_release(&branch_top);
 
        strbuf_release(&reflog_msg);
        strbuf_release(&branch_top);

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 9d79f1994a43efed3883ccb80898292a687c4d76..a0c3427496123d08ff550df8c4656732421ee80a 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -442,7 +442,8 @@ static int module_name(int argc, const char **argv, const char *prefix)
 }
 
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
 }
 
 static int clone_submodule(const char *path, const char *gitdir, const char *url,

-                          const char *depth, struct string_list *reference, int quiet)
+                          const char *depth, struct string_list *reference,
+                          int quiet, int progress)

 {
        struct child_process cp = CHILD_PROCESS_INIT;
 
 {
        struct child_process cp = CHILD_PROCESS_INIT;
 


@@ -450,6 +451,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
        argv_array_push(&cp.args, "--no-checkout");
        if (quiet)
                argv_array_push(&cp.args, "--quiet");
        argv_array_push(&cp.args, "--no-checkout");
        if (quiet)
                argv_array_push(&cp.args, "--quiet");

+       if (progress)
+               argv_array_push(&cp.args, "--progress");

        if (depth && *depth)
                argv_array_pushl(&cp.args, "--depth", depth, NULL);
        if (reference->nr) {
        if (depth && *depth)
                argv_array_pushl(&cp.args, "--depth", depth, NULL);
        if (reference->nr) {


@@ -574,6 +577,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 {
        const char *name = NULL, *url = NULL, *depth = NULL;
        int quiet = 0;
 {
        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;
        FILE *submodule_dot_git;
        char *p, *path = NULL, *sm_gitdir;
        struct strbuf rel_path = STRBUF_INIT;


@@ -600,6 +604,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
                           N_("string"),
                           N_("depth for shallow clones")),
                OPT__QUIET(&quiet, "Suppress output for cloning a submodule"),
                           N_("string"),
                           N_("depth for shallow clones")),
                OPT__QUIET(&quiet, "Suppress output for cloning a submodule"),

+               OPT_BOOL(0, "progress", &progress,
+                          N_("force cloning progress")),

                OPT_END()
        };
 
                OPT_END()
        };
 


@@ -633,7 +639,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 
                prepare_possible_alternates(name, &reference);
 
 
                prepare_possible_alternates(name, &reference);
 

-               if (clone_submodule(path, sm_gitdir, url, depth, &reference, quiet))
+               if (clone_submodule(path, sm_gitdir, url, depth, &reference,
+                                   quiet, progress))

                        die(_("clone of '%s' into submodule path '%s' failed"),
                            url, path);
        } else {
                        die(_("clone of '%s' into submodule path '%s' failed"),
                            url, path);
        } else {


@@ -683,6 +690,7 @@ struct submodule_update_clone {
        struct submodule_update_strategy update;
 
        /* configuration parameters which are passed on to the children */
        struct submodule_update_strategy update;
 
        /* configuration parameters which are passed on to the children */

+       int progress;

        int quiet;
        int recommend_shallow;
        struct string_list references;
        int quiet;
        int recommend_shallow;
        struct string_list references;


@@ -701,7 +709,7 @@ struct submodule_update_clone {
        int failed_clones_nr, failed_clones_alloc;
 };
 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
        int failed_clones_nr, failed_clones_alloc;
 };
 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \

-       SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \
+       SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \

        NULL, NULL, NULL, \
        STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
 
        NULL, NULL, NULL, \
        STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
 


@@ -803,6 +811,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
        child->err = -1;
        argv_array_push(&child->args, "submodule--helper");
        argv_array_push(&child->args, "clone");
        child->err = -1;
        argv_array_push(&child->args, "submodule--helper");
        argv_array_push(&child->args, "clone");

+       if (suc->progress)
+               argv_array_push(&child->args, "--progress");

        if (suc->quiet)
                argv_array_push(&child->args, "--quiet");
        if (suc->prefix)
        if (suc->quiet)
                argv_array_push(&child->args, "--quiet");
        if (suc->prefix)


@@ -949,6 +959,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
                OPT_BOOL(0, "recommend-shallow", &suc.recommend_shallow,
                            N_("whether the initial clone should follow the shallow recommendation")),
                OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
                OPT_BOOL(0, "recommend-shallow", &suc.recommend_shallow,
                            N_("whether the initial clone should follow the shallow recommendation")),
                OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),

+               OPT_BOOL(0, "progress", &suc.progress,
+                           N_("force cloning progress")),

                OPT_END()
        };
 
                OPT_END()
        };
 

diff --git a/git-submodule.sh b/git-submodule.sh
index a1cc71b521b76c346509207f4ef9a24d48ead229..a024a135d6663c8a8d5ceb926e3f42df1f577411 100755 (executable)
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -44,6 +44,7 @@ update=
 prefix=
 custom_name=
 depth=
 prefix=
 custom_name=
 depth=

+progress=

 
 die_if_unmatched ()
 {
 
 die_if_unmatched ()
 {


@@ -498,6 +499,9 @@ cmd_update()
                -q|--quiet)
                        GIT_QUIET=1
                        ;;
                -q|--quiet)
                        GIT_QUIET=1
                        ;;

+               --progress)
+                       progress="--progress"
+                       ;;

                -i|--init)
                        init=1
                        ;;
                -i|--init)
                        init=1
                        ;;


@@ -573,6 +577,7 @@ cmd_update()
 
        {
        git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
 
        {
        git submodule--helper update-clone ${GIT_QUIET:+--quiet} \

+               ${progress:+"$progress"} \

                ${wt_prefix:+--prefix "$wt_prefix"} \
                ${prefix:+--recursive-prefix "$prefix"} \
                ${update:+--update "$update"} \
                ${wt_prefix:+--prefix "$wt_prefix"} \
                ${prefix:+--recursive-prefix "$prefix"} \
                ${update:+--update "$update"} \