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>

Jeff King committed Sep 22, 2016 at 01:24 UTC 72c5f88311dab7149fcca1f6029414ccb776fee8
3 files changed +34 -5
builtin/clone.c
+14 -2
@@ -670,7 +670,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
670 }
671 }
672
673 -static int checkout(void)
673 +static int checkout(int submodule_progress)
674 {
675 unsigned char sha1[20];
676 char *head;
@@ -734,6 +734,9 @@ static int checkout(void)
734 if (max_jobs != -1)
735 argv_array_pushf(&args, "--jobs=%d", max_jobs);
736
737 + if (submodule_progress)
738 + argv_array_push(&args, "--progress");
739 +
740 err = run_command_v_opt(args.argv, RUN_GIT_CMD);
741 argv_array_clear(&args);
742 }
@@ -841,6 +844,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
844 const char *src_ref_prefix = "refs/heads/";
845 struct remote *remote;
846 int err = 0, complete_refs_before_fetch = 1;
847 + int submodule_progress;
848
849 struct refspec *refspec;
850 const char *fetch_pattern;
@@ -1099,6 +1103,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1103
1104 update_head(our_head_points_at, remote_head, reflog_msg.buf);
1105
1106 + /*
1107 + * We want to show progress for recursive submodule clones iff
1108 + * we did so for the main clone. But only the transport knows
1109 + * the final decision for this flag, so we need to rescue the value
1110 + * before we free the transport.
1111 + */
1112 + submodule_progress = transport->progress;
1113 +
1114 transport_unlock_pack(transport);
1115 transport_disconnect(transport);
1116
@@ -1108,7 +1120,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1120 }
1121
1122 junk_mode = JUNK_LEAVE_REPO;
1111 - err = checkout();
1123 + err = checkout(submodule_progress);
1124
1125 strbuf_release(&reflog_msg);
1126 strbuf_release(&branch_top);
builtin/submodule--helper.c
+15 -3
@@ -442,7 +442,8 @@ static int module_name(int argc, const char **argv, const char *prefix)
442 }
443
444 static int clone_submodule(const char *path, const char *gitdir, const char *url,
445 - const char *depth, struct string_list *reference, int quiet)
445 + const char *depth, struct string_list *reference,
446 + int quiet, int progress)
447 {
448 struct child_process cp = CHILD_PROCESS_INIT;
449
@@ -450,6 +451,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
451 argv_array_push(&cp.args, "--no-checkout");
452 if (quiet)
453 argv_array_push(&cp.args, "--quiet");
454 + if (progress)
455 + argv_array_push(&cp.args, "--progress");
456 if (depth && *depth)
457 argv_array_pushl(&cp.args, "--depth", depth, NULL);
458 if (reference->nr) {
@@ -574,6 +577,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
577 {
578 const char *name = NULL, *url = NULL, *depth = NULL;
579 int quiet = 0;
580 + int progress = 0;
581 FILE *submodule_dot_git;
582 char *p, *path = NULL, *sm_gitdir;
583 struct strbuf rel_path = STRBUF_INIT;
@@ -600,6 +604,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
604 N_("string"),
605 N_("depth for shallow clones")),
606 OPT__QUIET(&quiet, "Suppress output for cloning a submodule"),
607 + OPT_BOOL(0, "progress", &progress,
608 + N_("force cloning progress")),
609 OPT_END()
610 };
611
@@ -633,7 +639,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
639
640 prepare_possible_alternates(name, &reference);
641
636 - if (clone_submodule(path, sm_gitdir, url, depth, &reference, quiet))
642 + if (clone_submodule(path, sm_gitdir, url, depth, &reference,
643 + quiet, progress))
644 die(_("clone of '%s' into submodule path '%s' failed"),
645 url, path);
646 } else {
@@ -683,6 +690,7 @@ struct submodule_update_clone {
690 struct submodule_update_strategy update;
691
692 /* configuration parameters which are passed on to the children */
693 + int progress;
694 int quiet;
695 int recommend_shallow;
696 struct string_list references;
@@ -701,7 +709,7 @@ struct submodule_update_clone {
709 int failed_clones_nr, failed_clones_alloc;
710 };
711 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
704 - SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \
712 + SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
713 NULL, NULL, NULL, \
714 STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
715
@@ -803,6 +811,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
811 child->err = -1;
812 argv_array_push(&child->args, "submodule--helper");
813 argv_array_push(&child->args, "clone");
814 + if (suc->progress)
815 + argv_array_push(&child->args, "--progress");
816 if (suc->quiet)
817 argv_array_push(&child->args, "--quiet");
818 if (suc->prefix)
@@ -949,6 +959,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
959 OPT_BOOL(0, "recommend-shallow", &suc.recommend_shallow,
960 N_("whether the initial clone should follow the shallow recommendation")),
961 OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
962 + OPT_BOOL(0, "progress", &suc.progress,
963 + N_("force cloning progress")),
964 OPT_END()
965 };
966
git-submodule.sh
+5
@@ -44,6 +44,7 @@ update=
44 prefix=
45 custom_name=
46 depth=
47 +progress=
48
49 die_if_unmatched ()
50 {
@@ -498,6 +499,9 @@ cmd_update()
499 -q|--quiet)
500 GIT_QUIET=1
501 ;;
502 + --progress)
503 + progress="--progress"
504 + ;;
505 -i|--init)
506 init=1
507 ;;
@@ -573,6 +577,7 @@ cmd_update()
577
578 {
579 git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
580 + ${progress:+"$progress"} \
581 ${wt_prefix:+--prefix "$wt_prefix"} \
582 ${prefix:+--recursive-prefix "$prefix"} \
583 ${update:+--update "$update"} \