[PATCH] clone: use --quiet when stderr is not a terminal

From: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date: Mar 13, 2020, 21:09 UTC
Message-ID: <pull.581.git.1584133742475.gitgitgadget@gmail.com>

From: Derrick Stolee <dstolee@microsoft.com>

"git clone" is used by many build systems to download Git code before running a build. The output of these systems is usually color-coded to separate stdout and stderr output, which highlights anything over stderr as an error or warning. Most build systems use "--quiet" when cloning to avoid adding progress noise to these outputs, but occasionally users create their own scripts that call "git clone" and forget the --quiet option.

Just such a user voiced a complaint that "git clone" was showing "error messages" in bright red. The messages were progress indicators for "Updating files".

To save users from this confusion, let's default to --quiet when stderr is not a terminal window.

To test that this works, use the GIT_PROGRESS_DELAY environment variable to enforce that all progress indicators appear immediately, and check that a redirected stderr has no output. We also need to update some tests that inspect stderr after a "git clone" or "git submodule update" command. It is easy to update the clone tests with the --verbose option, while we can remove the clone output from the expected output of the submodule test.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
    clone: use --quiet when stderr is not a terminal
    
    I think this is generally how we are intending Git builtins to work.
    There was a complaint recently about my proposed addition of progress to
    'git read-tree', but that was because scripts would suddenly get noisy
    if they were not expecting it. This is the opposite: we will make 'git
    clone' quieter.
    
    Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-581%2Fderrickstolee%2Fclone-quiet-default-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-581/derrickstolee/clone-quiet-default-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/581

 builtin/clone.c             | 3 +++
 t/t5550-http-fetch-dumb.sh  | 2 +-
 t/t5601-clone.sh            | 7 ++++++-
 t/t7406-submodule-update.sh | 8 --------
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 1ad26f4d8c8..a2e6905f0ef 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -957,6 +957,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
 
+	if (!isatty(2))
+		option_verbosity = -1;
+
 	packet_trace_identity("clone");
 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
 			     builtin_clone_usage, 0);
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index b811d89cfd6..c0bdcafa304 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -332,7 +332,7 @@ test_expect_success 'redirects can be forbidden/allowed' '
 	test_must_fail git -c http.followRedirects=false \
 		clone $HTTPD_URL/dumb-redir/repo.git dumb-redir &&
 	git -c http.followRedirects=true \
-		clone $HTTPD_URL/dumb-redir/repo.git dumb-redir 2>stderr
+		clone --verbose $HTTPD_URL/dumb-redir/repo.git dumb-redir 2>stderr
 '
 
 test_expect_success 'redirects are reported to stderr' '
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 84ea2a3eb70..2902a201977 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -39,7 +39,7 @@ test_expect_success 'clone with excess parameters (2)' '
 
 test_expect_success C_LOCALE_OUTPUT 'output from clone' '
 	rm -fr dst &&
-	git clone -n "file://$(pwd)/src" dst >output 2>&1 &&
+	git clone --verbose -n "file://$(pwd)/src" dst >output 2>&1 &&
 	test $(grep Clon output | wc -l) = 1
 '
 
@@ -297,6 +297,11 @@ test_expect_success 'clone from original with relative alternate' '
 	grep /src/\\.git/objects target-10/objects/info/alternates
 '
 
+test_expect_success 'clone quietly without terminal' '
+	GIT_PROGRESS_DELAY=0 git clone src progress 2>err &&
+	test_must_be_empty err
+'
+
 test_expect_success 'clone checking out a tag' '
 	git clone --branch=some-tag src dst.tag &&
 	GIT_DIR=src/.git git rev-parse some-tag >expected &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 4fb447a143e..ebf08e3a77a 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -115,18 +115,10 @@ Submodule path '../super/submodule': checked out '$submodulesha1'
 EOF
 
 cat <<EOF >expect2
-Cloning into '$pwd/recursivesuper/super/merging'...
-Cloning into '$pwd/recursivesuper/super/none'...
-Cloning into '$pwd/recursivesuper/super/rebasing'...
-Cloning into '$pwd/recursivesuper/super/submodule'...
 Submodule 'merging' ($pwd/merging) registered for path '../super/merging'
 Submodule 'none' ($pwd/none) registered for path '../super/none'
 Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'
 Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule'
-done.
-done.
-done.
-done.
 EOF
 
 test_expect_success 'submodule update --init --recursive from subdirectory' '

base-commit: b4374e96c84ed9394fed363973eb540da308ed4f

-- 
gitgitgadget