From 68b939b2f097b6675c4aaa178655559aa81b25cb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Sep 2013 16:05:13 -0400 Subject: clone: send diagnostic messages to stderr Putting messages like "Cloning into.." and "done" on stdout is un-Unix and uselessly clutters the stdout channel. Send them to stderr. We have to tweak two tests to accommodate this: 1. t5601 checks for doubled output due to forking, and doesn't actually care where the output goes; adjust it to check stderr. 2. t5702 is trying to test whether progress output was sent to stderr, but naively does so by checking whether stderr produced any output. Instead, have it look for "%", a token found in progress output but not elsewhere (and which lets us avoid hard-coding the progress text in the test). This should not regress any scripts that try to parse the current output, as the output is already internationalized and therefore unstable. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/clone.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index 430307b29..42c40f2bf 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -380,7 +380,7 @@ static void clone_local(const char *src_repo, const char *dest_repo) } if (0 <= option_verbosity) - printf(_("done.\n")); + fprintf(stderr, _("done.\n")); } static const char *junk_work_tree; @@ -552,12 +552,12 @@ static void update_remote_refs(const struct ref *refs, if (check_connectivity) { if (0 <= option_verbosity) - printf(_("Checking connectivity... ")); + fprintf(stderr, _("Checking connectivity... ")); if (check_everything_connected_with_transport(iterate_ref_map, 0, &rm, transport)) die(_("remote did not send all necessary objects")); if (0 <= option_verbosity) - printf(_("done\n")); + fprintf(stderr, _("done\n")); } if (refs) { @@ -850,9 +850,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (0 <= option_verbosity) { if (option_bare) - printf(_("Cloning into bare repository '%s'...\n"), dir); + fprintf(stderr, _("Cloning into bare repository '%s'...\n"), dir); else - printf(_("Cloning into '%s'...\n"), dir); + fprintf(stderr, _("Cloning into '%s'...\n"), dir); } init_db(option_template, INIT_DB_QUIET); write_config(&option_config); -- cgit v1.2.1 From 2856cbf0ae604c233e7dc62ba7931a469165ce4f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Sep 2013 16:06:50 -0400 Subject: clone: treat "checking connectivity" like other progress When stderr does not point to a tty, we typically suppress "we are now in this phase" progress reporting (e.g., we ask the server not to send us "counting objects" and the like). The new "checking connectivity" message is in the same vein, and should be suppressed. Since clone relies on the transport code to make the decision, we can simply sneak a peek at the "progress" field of the transport struct. That properly takes into account both the verbosity and progress options we were given, as well as the result of isatty(). Note that we do not set up that progress flag for a local clone, as we do not fetch using the transport at all. That's acceptable here, though, because we also do not perform a connectivity check in that case. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/clone.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index 42c40f2bf..72cc40a1c 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs, const struct ref *rm = mapped_refs; if (check_connectivity) { - if (0 <= option_verbosity) + if (transport->progress) fprintf(stderr, _("Checking connectivity... ")); if (check_everything_connected_with_transport(iterate_ref_map, 0, &rm, transport)) die(_("remote did not send all necessary objects")); - if (0 <= option_verbosity) + if (transport->progress) fprintf(stderr, _("done\n")); } -- cgit v1.2.1 From 643f918d13906cbccdc5ad188767fc7895e30fc1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Sep 2013 16:35:13 -0400 Subject: clone: always set transport options A clone will always create a transport struct, whether we are cloning locally or using an actual protocol. In the local case, we only use the transport to get the list of refs, and then transfer the objects out-of-band. However, there are many options that we do not bother setting up in the local case. For the most part, these are noops, because they only affect the object-fetching stage (e.g., the --depth option). However, some options do have a visible impact. For example, giving the path to upload-pack via "-u" does not currently work for a local clone, even though we need upload-pack to get the ref list. We can just drop the conditional entirely and set these options for both local and non-local clones. Rather than keep track of which options impact the object versus the ref fetching stage, we can simply let the noops be noops (and the cost of setting the options in the first place is not high). The one exception is that we also check that the transport provides both a "get_refs_list" and a "fetch" method. We will now be checking the former for both cases (which is good, since a transport that cannot fetch refs would not work for a local clone), and we tweak the conditional to check for a "fetch" only when we are non-local. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/clone.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index 72cc40a1c..63f298be9 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -885,27 +885,25 @@ int cmd_clone(int argc, const char **argv, const char *prefix) remote = remote_get(option_origin); transport = transport_get(remote, remote->url[0]); - if (!is_local) { - if (!transport->get_refs_list || !transport->fetch) - die(_("Don't know how to clone %s"), transport->url); + if (!transport->get_refs_list || (!is_local && !transport->fetch)) + die(_("Don't know how to clone %s"), transport->url); - transport_set_option(transport, TRANS_OPT_KEEP, "yes"); + transport_set_option(transport, TRANS_OPT_KEEP, "yes"); - if (option_depth) - transport_set_option(transport, TRANS_OPT_DEPTH, - option_depth); - if (option_single_branch) - transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1"); + if (option_depth) + transport_set_option(transport, TRANS_OPT_DEPTH, + option_depth); + if (option_single_branch) + transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1"); - transport_set_verbosity(transport, option_verbosity, option_progress); + transport_set_verbosity(transport, option_verbosity, option_progress); - if (option_upload_pack) - transport_set_option(transport, TRANS_OPT_UPLOADPACK, - option_upload_pack); + if (option_upload_pack) + transport_set_option(transport, TRANS_OPT_UPLOADPACK, + option_upload_pack); - if (transport->smart_options && !option_depth) - transport->smart_options->check_self_contained_and_connected = 1; - } + if (transport->smart_options && !option_depth) + transport->smart_options->check_self_contained_and_connected = 1; refs = transport_get_remote_refs(transport); -- cgit v1.2.1