git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Bug report: 'filtering content' delayed progress message does not respect --quiet
@ 2021-03-21 20:53 Sean Allred
  2021-03-26  8:31 ` Jeff King
  0 siblings, 1 reply; 7+ messages in thread
From: Sean Allred @ 2021-03-21 20:53 UTC (permalink / raw)
  To: git

Hi folks,

Submitting the filled-out template from git-bugreport. Let me know if
I can be of any assistance.

Thanks!
-Sean

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

  Called `git clone --quiet git://path/to/private/repo`

What did you expect to happen? (Expected behavior)

  Expected git to be quiet :-)  Did not expect writes to stderr/stdout.

What happened instead? (Actual behavior)

  Received output that looked like

      Filtering content:  --% (--/--), --.-- MiB | --.-- MiB/s

What's different between what you expected and what actually happened?

  `--quiet` should suppress all output, but in actuality, that line
  was still output.

Anything else you want to add:

  Submitting from https://github.com/git-lfs/git-lfs/issues/1270 (re
  https://github.com/git-lfs/git-lfs/issues/1270#issuecomment-461176647).
  Other details may be available there.

  I've verified I can still reproduce on the current version of git
  (listed below) with a private repo (note to self:
  gid://gitlab/Project/2458).

  It looks like we need similar handling from
  index_pack.c:resolve_deltas at entry.c:finish_delayed_checkout.  It
  looks like the necessary state may not be available where it's
  needed, but this is based on about two minutes of reading :-)

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.31.0.windows.1
cpu: x86_64
built from commit: 959b2f0c6fb12f8ba13f9015c4482fa8133b7f9c
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Windows 10.0 17763
compiler info: gnuc: 10.2
libc info: no libc information available
$SHELL (typically, interactive shell): <unset>


[Enabled Hooks]
not run from a git repository - no hooks to show


-- 
-Sean

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Bug report: 'filtering content' delayed progress message does not respect --quiet
  2021-03-21 20:53 Bug report: 'filtering content' delayed progress message does not respect --quiet Sean Allred
@ 2021-03-26  8:31 ` Jeff King
  2021-08-25 18:15   ` [PATCH] checkout: make delayed checkout respect --quiet and --no-progress Matheus Tavares
  0 siblings, 1 reply; 7+ messages in thread
From: Jeff King @ 2021-03-26  8:31 UTC (permalink / raw)
  To: Sean Allred; +Cc: Lars Schneider, git

On Sun, Mar 21, 2021 at 03:53:07PM -0500, Sean Allred wrote:

> What did you do before the bug happened? (Steps to reproduce your issue)
> 
>   Called `git clone --quiet git://path/to/private/repo`
> 
> What did you expect to happen? (Expected behavior)
> 
>   Expected git to be quiet :-)  Did not expect writes to stderr/stdout.
> 
> What happened instead? (Actual behavior)
> 
>   Received output that looked like
> 
>       Filtering content:  --% (--/--), --.-- MiB | --.-- MiB/s

+cc Lars, who added this in 52f1d62eb4 (convert: display progress for
filtered objects that have been delayed, 2017-08-20).

The message is in finish_delayed_checkout(), which gets only a "struct
checkout" to carry the state. That has a "quiet" field, but I'm not sure
it is set appropriately. E.g., builtin/checkout.c's checkout_worktree()
does not set it at all, and it is unconditionally set in unpack-trees.c's
check_updates().

We should obviously be respecting --quiet, but also checking isatty(2)
before auto-enabling. Probably we need a separate show_progress field.
For unpack-trees, I think it would get set from o->verbose_update, which
is what controls the existing "Updating files" meter. For checkout.c, it
probably comes from checkout_opts.show_progress.

-Peff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] checkout: make delayed checkout respect --quiet and --no-progress
  2021-03-26  8:31 ` Jeff King
@ 2021-08-25 18:15   ` Matheus Tavares
  2021-08-25 23:35     ` Ævar Arnfjörð Bjarmason
  2021-08-26 19:10     ` [PATCH v2] " Matheus Tavares
  0 siblings, 2 replies; 7+ messages in thread
From: Matheus Tavares @ 2021-08-25 18:15 UTC (permalink / raw)
  To: peff; +Cc: allred.sean, git, larsxschneider

The 'Filtering contents...' progress report from delayed checkout is
displayed even when checkout and clone are invoked with --quiet or
--no-progress. Furthermore, it is displayed unconditionally, without
first checking whether stdout is a tty. Let's fix these issues and also
add some regression tests for the two code paths that currently use
delayed checkout: unpack_trees.c:check_updates() and
builtin/checkout.c:checkout_worktree().

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
---

The test section of the patch is a bit long because it checks all the
verbosity options related to the progress report, and it also tests both
the check_updates() and checkout_worktree() code paths. If that is
overkill, I can remove some tests.

 builtin/checkout.c    |  2 +-
 entry.c               |  7 +++--
 entry.h               |  3 ++-
 t/t0021-conversion.sh | 63 +++++++++++++++++++++++++++++++++++++++++++
 unpack-trees.c        |  2 +-
 5 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index b5d477919a..b23bc149d1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -404,7 +404,7 @@ static int checkout_worktree(const struct checkout_opts *opts,
 	mem_pool_discard(&ce_mem_pool, should_validate_cache_entries());
 	remove_marked_cache_entries(&the_index, 1);
 	remove_scheduled_dirs();
-	errs |= finish_delayed_checkout(&state, &nr_checkouts);
+	errs |= finish_delayed_checkout(&state, &nr_checkouts, opts->show_progress);
 
 	if (opts->count_checkout_paths) {
 		if (nr_unmerged)
diff --git a/entry.c b/entry.c
index 125fabdbd5..044e8ec92c 100644
--- a/entry.c
+++ b/entry.c
@@ -159,7 +159,8 @@ static int remove_available_paths(struct string_list_item *item, void *cb_data)
 	return !available;
 }
 
-int finish_delayed_checkout(struct checkout *state, int *nr_checkouts)
+int finish_delayed_checkout(struct checkout *state, int *nr_checkouts,
+			    int show_progress)
 {
 	int errs = 0;
 	unsigned delayed_object_count;
@@ -173,7 +174,9 @@ int finish_delayed_checkout(struct checkout *state, int *nr_checkouts)
 
 	dco->state = CE_RETRY;
 	delayed_object_count = dco->paths.nr;
-	progress = start_delayed_progress(_("Filtering content"), delayed_object_count);
+	progress = show_progress
+		? start_delayed_progress(_("Filtering content"), delayed_object_count)
+		: NULL;
 	while (dco->filters.nr > 0) {
 		for_each_string_list_item(filter, &dco->filters) {
 			struct string_list available_paths = STRING_LIST_INIT_NODUP;
diff --git a/entry.h b/entry.h
index b8c0e170dc..7c889e58fd 100644
--- a/entry.h
+++ b/entry.h
@@ -43,7 +43,8 @@ static inline int checkout_entry(struct cache_entry *ce,
 }
 
 void enable_delayed_checkout(struct checkout *state);
-int finish_delayed_checkout(struct checkout *state, int *nr_checkouts);
+int finish_delayed_checkout(struct checkout *state, int *nr_checkouts,
+			    int show_progress);
 
 /*
  * Unlink the last component and schedule the leading directories for
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index b5749f327d..be12b92f21 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
 
 TEST_ROOT="$PWD"
 PATH=$TEST_ROOT:$PATH
@@ -1061,4 +1062,66 @@ test_expect_success PERL,SYMLINKS,CASE_INSENSITIVE_FS \
 	)
 '
 
+test_expect_success PERL 'setup for progress tests' '
+	git init progress &&
+	(
+		cd progress &&
+		git config filter.delay.process "rot13-filter.pl delay-progress.log clean smudge delay" &&
+		git config filter.delay.required true &&
+
+		echo "*.a filter=delay" >.gitattributes &&
+		touch test-delay10.a &&
+		git add . &&
+		git commit -m files
+	)
+'
+
+for mode in pathspec branch
+do
+	case "$mode" in
+	pathspec) opt='.' ;;
+	branch) opt='-f HEAD' ;;
+	esac
+
+	test_expect_success PERL,TTY "delayed checkout shows progress by default only on tty ($mode checkout)" '
+		(
+			cd progress &&
+			rm -f *.a delay-progress.log &&
+			test_terminal env GIT_PROGRESS_DELAY=0 git checkout $opt 2>err &&
+			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
+			grep "Filtering content" err &&
+
+			rm -f *.a delay-progress.log &&
+			GIT_PROGRESS_DELAY=0 git checkout $opt 2>err &&
+			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
+			! grep "Filtering content" err
+		)
+	'
+
+	test_expect_success PERL,TTY "delayed checkout ommits progress with --quiet ($mode checkout)" '
+		(
+			cd progress &&
+			rm -f *.a delay-progress.log &&
+			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --quiet $opt 2>err &&
+			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
+			! grep "Filtering content" err
+		)
+	'
+
+	test_expect_success PERL,TTY "delayed checkout honors --[no]-progress ($mode checkout)" '
+		(
+			cd progress &&
+			rm -f *.a delay-progress.log &&
+			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --no-progress $opt 2>err &&
+			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
+			! grep "Filtering content" err &&
+
+			rm -f *.a delay-progress.log &&
+			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --quiet --progress $opt 2>err &&
+			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
+			grep "Filtering content" err
+		)
+	'
+done
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 5786645f31..f07304f1b7 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -479,7 +479,7 @@ static int check_updates(struct unpack_trees_options *o,
 		errs |= run_parallel_checkout(&state, pc_workers, pc_threshold,
 					      progress, &cnt);
 	stop_progress(&progress);
-	errs |= finish_delayed_checkout(&state, NULL);
+	errs |= finish_delayed_checkout(&state, NULL, o->verbose_update);
 	git_attr_set_direction(GIT_ATTR_CHECKIN);
 
 	if (o->clone)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] checkout: make delayed checkout respect --quiet and --no-progress
  2021-08-25 18:15   ` [PATCH] checkout: make delayed checkout respect --quiet and --no-progress Matheus Tavares
@ 2021-08-25 23:35     ` Ævar Arnfjörð Bjarmason
  2021-08-26 14:26       ` Matheus Tavares Bernardino
  2021-08-26 19:10     ` [PATCH v2] " Matheus Tavares
  1 sibling, 1 reply; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-08-25 23:35 UTC (permalink / raw)
  To: Matheus Tavares; +Cc: peff, allred.sean, git, larsxschneider


On Wed, Aug 25 2021, Matheus Tavares wrote:

> The test section of the patch is a bit long because it checks all the
> verbosity options related to the progress report, and it also tests both
> the check_updates() and checkout_worktree() code paths. If that is
> overkill, I can remove some tests.

More exhaustive tests are generally nice..

> +test_expect_success PERL 'setup for progress tests' '
> +	git init progress &&
> +	(
> +		cd progress &&
> +		git config filter.delay.process "rot13-filter.pl delay-progress.log clean smudge delay" &&
> +		git config filter.delay.required true &&
> +
> +		echo "*.a filter=delay" >.gitattributes &&
> +		touch test-delay10.a &&
> +		git add . &&
> +		git commit -m files
> +	)
> +'

This doesn't seem to depend on PERL, should this really be a skip_all at
the top if we don't have the TTY prereq, i.e. we shouldn't bother?

> +
> +for mode in pathspec branch
> +do
> +	case "$mode" in
> +	pathspec) opt='.' ;;
> +	branch) opt='-f HEAD' ;;
> +	esac
> +
> +	test_expect_success PERL,TTY "delayed checkout shows progress by default only on tty ($mode checkout)" '

All of the PERL,TTY can just be TTY, since TTY itself checks PERL.

> +		(
> +			cd progress &&
> +			rm -f *.a delay-progress.log &&
> +			test_terminal env GIT_PROGRESS_DELAY=0 git checkout $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			grep "Filtering content" err &&

This seems to need TTY...

> +			rm -f *.a delay-progress.log &&
> +			GIT_PROGRESS_DELAY=0 git checkout $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			! grep "Filtering content" err

But this one doesn't, perhaps it could be a non-TTY test?

> +		)
> +	'
> +
> +	test_expect_success PERL,TTY "delayed checkout ommits progress with --quiet ($mode checkout)" '
> +		(
> +			cd progress &&
> +			rm -f *.a delay-progress.log &&
> +			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --quiet $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			! grep "Filtering content" err
> +		)
> +	'
> +
> +	test_expect_success PERL,TTY "delayed checkout honors --[no]-progress ($mode checkout)" '
> +		(
> +			cd progress &&
> +			rm -f *.a delay-progress.log &&
> +			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --no-progress $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			! grep "Filtering content" err &&
> +
> +			rm -f *.a delay-progress.log &&
> +			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --quiet --progress $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			grep "Filtering content" err
> +		)
> +	'

It looks like these tests could be split into one helper function which
just passed params for e.g. whether the "Filtering content" grep was
negated, and what command should be run.

Also if possible the two sections of the test could be split up, and
then the "rm -rf" could just be a "test_when_finished" at the top...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] checkout: make delayed checkout respect --quiet and --no-progress
  2021-08-25 23:35     ` Ævar Arnfjörð Bjarmason
@ 2021-08-26 14:26       ` Matheus Tavares Bernardino
  2021-08-27  2:26         ` Jeff King
  0 siblings, 1 reply; 7+ messages in thread
From: Matheus Tavares Bernardino @ 2021-08-26 14:26 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Jeff King, allred.sean, git, Lars Schneider

Hi, Ævar

Thanks for the comments!

On Wed, Aug 25, 2021 at 8:39 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Wed, Aug 25 2021, Matheus Tavares wrote:
>
> > +test_expect_success PERL 'setup for progress tests' '
> > +     git init progress &&
> > +     (
> > +             cd progress &&
> > +             git config filter.delay.process "rot13-filter.pl delay-progress.log clean smudge delay" &&
> > +             git config filter.delay.required true &&
> > +
> > +             echo "*.a filter=delay" >.gitattributes &&
> > +             touch test-delay10.a &&
> > +             git add . &&
> > +             git commit -m files
> > +     )
> > +'
>
> This doesn't seem to depend on PERL,

It actually depends on PERL because `git add .` will run the clean
filter for `test-delay10.a`.

> should this really be a skip_all at
> the top if we don't have the TTY prereq, i.e. we shouldn't bother?

Yeah, I think it could be a skip_all. But as you pointed out below,
one of the tests doesn't really depend on TTY, so I guess we could
leave the independent prereqs for each test.

> > +
> > +for mode in pathspec branch
> > +do
> > +     case "$mode" in
> > +     pathspec) opt='.' ;;
> > +     branch) opt='-f HEAD' ;;
> > +     esac
> > +
> > +     test_expect_success PERL,TTY "delayed checkout shows progress by default only on tty ($mode checkout)" '
>
> All of the PERL,TTY can just be TTY, since TTY itself checks PERL.

I don't mind changing that, but isn't it a bit clearer for readers to
have both dependencies explicitly?

> > +             (
> > +                     cd progress &&
> > +                     rm -f *.a delay-progress.log &&
> > +                     test_terminal env GIT_PROGRESS_DELAY=0 git checkout $opt 2>err &&
> > +                     grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> > +                     grep "Filtering content" err &&
>
> This seems to need TTY...
>
> > +                     rm -f *.a delay-progress.log &&
> > +                     GIT_PROGRESS_DELAY=0 git checkout $opt 2>err &&
> > +                     grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> > +                     ! grep "Filtering content" err
>
> But this one doesn't, perhaps it could be a non-TTY test?

Good catch, I'll split this test in two.

> > +             )
> > +     '
> > +
> > +     test_expect_success PERL,TTY "delayed checkout ommits progress with --quiet ($mode checkout)" '
> > +             (
> > +                     cd progress &&
> > +                     rm -f *.a delay-progress.log &&
> > +                     test_terminal env GIT_PROGRESS_DELAY=0 git checkout --quiet $opt 2>err &&
> > +                     grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> > +                     ! grep "Filtering content" err
> > +             )
> > +     '
> > +
> > +     test_expect_success PERL,TTY "delayed checkout honors --[no]-progress ($mode checkout)" '
> > +             (
> > +                     cd progress &&
> > +                     rm -f *.a delay-progress.log &&
> > +                     test_terminal env GIT_PROGRESS_DELAY=0 git checkout --no-progress $opt 2>err &&
> > +                     grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> > +                     ! grep "Filtering content" err &&
> > +
> > +                     rm -f *.a delay-progress.log &&
> > +                     test_terminal env GIT_PROGRESS_DELAY=0 git checkout --quiet --progress $opt 2>err &&
> > +                     grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> > +                     grep "Filtering content" err
> > +             )
> > +     '
>
> It looks like these tests could be split into one helper function which
> just passed params for e.g. whether the "Filtering content" grep was
> negated, and what command should be run.

Makes sense, I'll do that.

> Also if possible the two sections of the test could be split up, and
> then the "rm -rf" could just be a "test_when_finished" at the top...

Hmm, as we are removing the `test-delay10.a` file in order to check it
out again with custom options, I think it's a bit clearer to remove it
right before the actual git checkout invocation.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] checkout: make delayed checkout respect --quiet and --no-progress
  2021-08-25 18:15   ` [PATCH] checkout: make delayed checkout respect --quiet and --no-progress Matheus Tavares
  2021-08-25 23:35     ` Ævar Arnfjörð Bjarmason
@ 2021-08-26 19:10     ` Matheus Tavares
  1 sibling, 0 replies; 7+ messages in thread
From: Matheus Tavares @ 2021-08-26 19:10 UTC (permalink / raw)
  To: matheus.bernardino; +Cc: allred.sean, git, larsxschneider, peff, avarab

The 'Filtering contents...' progress report from delayed checkout is
displayed even when checkout and clone are invoked with --quiet or
--no-progress. Furthermore, it is displayed unconditionally, without
first checking whether stdout is a tty. Let's fix these issues and also
add some regression tests for the two code paths that currently use
delayed checkout: unpack_trees.c:check_updates() and
builtin/checkout.c:checkout_worktree().

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
---

Changes since v1:

- Exctract duplicated code from the different test cases into an
  auxiliary function.
- Split test that do not depend on TTY. 

 builtin/checkout.c    |  2 +-
 entry.c               |  7 +++--
 entry.h               |  3 +-
 t/t0021-conversion.sh | 71 +++++++++++++++++++++++++++++++++++++++++++
 unpack-trees.c        |  2 +-
 5 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index b5d477919a..b23bc149d1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -404,7 +404,7 @@ static int checkout_worktree(const struct checkout_opts *opts,
 	mem_pool_discard(&ce_mem_pool, should_validate_cache_entries());
 	remove_marked_cache_entries(&the_index, 1);
 	remove_scheduled_dirs();
-	errs |= finish_delayed_checkout(&state, &nr_checkouts);
+	errs |= finish_delayed_checkout(&state, &nr_checkouts, opts->show_progress);
 
 	if (opts->count_checkout_paths) {
 		if (nr_unmerged)
diff --git a/entry.c b/entry.c
index 125fabdbd5..044e8ec92c 100644
--- a/entry.c
+++ b/entry.c
@@ -159,7 +159,8 @@ static int remove_available_paths(struct string_list_item *item, void *cb_data)
 	return !available;
 }
 
-int finish_delayed_checkout(struct checkout *state, int *nr_checkouts)
+int finish_delayed_checkout(struct checkout *state, int *nr_checkouts,
+			    int show_progress)
 {
 	int errs = 0;
 	unsigned delayed_object_count;
@@ -173,7 +174,9 @@ int finish_delayed_checkout(struct checkout *state, int *nr_checkouts)
 
 	dco->state = CE_RETRY;
 	delayed_object_count = dco->paths.nr;
-	progress = start_delayed_progress(_("Filtering content"), delayed_object_count);
+	progress = show_progress
+		? start_delayed_progress(_("Filtering content"), delayed_object_count)
+		: NULL;
 	while (dco->filters.nr > 0) {
 		for_each_string_list_item(filter, &dco->filters) {
 			struct string_list available_paths = STRING_LIST_INIT_NODUP;
diff --git a/entry.h b/entry.h
index b8c0e170dc..7c889e58fd 100644
--- a/entry.h
+++ b/entry.h
@@ -43,7 +43,8 @@ static inline int checkout_entry(struct cache_entry *ce,
 }
 
 void enable_delayed_checkout(struct checkout *state);
-int finish_delayed_checkout(struct checkout *state, int *nr_checkouts);
+int finish_delayed_checkout(struct checkout *state, int *nr_checkouts,
+			    int show_progress);
 
 /*
  * Unlink the last component and schedule the leading directories for
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index b5749f327d..33dfc9cd56 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
 
 TEST_ROOT="$PWD"
 PATH=$TEST_ROOT:$PATH
@@ -1061,4 +1062,74 @@ test_expect_success PERL,SYMLINKS,CASE_INSENSITIVE_FS \
 	)
 '
 
+test_expect_success PERL 'setup for progress tests' '
+	git init progress &&
+	(
+		cd progress &&
+		git config filter.delay.process "rot13-filter.pl delay-progress.log clean smudge delay" &&
+		git config filter.delay.required true &&
+
+		echo "*.a filter=delay" >.gitattributes &&
+		touch test-delay10.a &&
+		git add . &&
+		git commit -m files
+	)
+'
+
+test_delayed_checkout_progress () {
+	if test "$1" = "!"
+	then
+		local expect_progress=N &&
+		shift
+	else
+		local expect_progress=
+	fi &&
+
+	if test $# -lt 1
+	then
+		BUG "no command given to test_delayed_checkout_progress"
+	fi &&
+
+	(
+		cd progress &&
+		GIT_PROGRESS_DELAY=0 &&
+		export GIT_PROGRESS_DELAY &&
+		rm -f *.a delay-progress.log &&
+
+		"$@" 2>err &&
+		grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
+		if test "$expect_progress" = N
+		then
+			! grep "Filtering content" err
+		else
+			grep "Filtering content" err
+		fi
+	)
+}
+
+for mode in pathspec branch
+do
+	case "$mode" in
+	pathspec) opt='.' ;;
+	branch) opt='-f HEAD' ;;
+	esac
+
+	test_expect_success PERL,TTY "delayed checkout shows progress by default on tty ($mode checkout)" '
+		test_delayed_checkout_progress test_terminal git checkout $opt
+	'
+
+	test_expect_success PERL "delayed checkout ommits progress on non-tty ($mode checkout)" '
+		test_delayed_checkout_progress ! git checkout $opt
+	'
+
+	test_expect_success PERL,TTY "delayed checkout ommits progress with --quiet ($mode checkout)" '
+		test_delayed_checkout_progress ! test_terminal git checkout --quiet $opt
+	'
+
+	test_expect_success PERL,TTY "delayed checkout honors --[no]-progress ($mode checkout)" '
+		test_delayed_checkout_progress ! test_terminal git checkout --no-progress $opt &&
+		test_delayed_checkout_progress test_terminal git checkout --quiet --progress $opt
+	'
+done
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 5786645f31..f07304f1b7 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -479,7 +479,7 @@ static int check_updates(struct unpack_trees_options *o,
 		errs |= run_parallel_checkout(&state, pc_workers, pc_threshold,
 					      progress, &cnt);
 	stop_progress(&progress);
-	errs |= finish_delayed_checkout(&state, NULL);
+	errs |= finish_delayed_checkout(&state, NULL, o->verbose_update);
 	git_attr_set_direction(GIT_ATTR_CHECKIN);
 
 	if (o->clone)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] checkout: make delayed checkout respect --quiet and --no-progress
  2021-08-26 14:26       ` Matheus Tavares Bernardino
@ 2021-08-27  2:26         ` Jeff King
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff King @ 2021-08-27  2:26 UTC (permalink / raw)
  To: Matheus Tavares Bernardino
  Cc: Ævar Arnfjörð Bjarmason, allred.sean, git,
	Lars Schneider

On Thu, Aug 26, 2021 at 11:26:46AM -0300, Matheus Tavares Bernardino wrote:

> > > +for mode in pathspec branch
> > > +do
> > > +     case "$mode" in
> > > +     pathspec) opt='.' ;;
> > > +     branch) opt='-f HEAD' ;;
> > > +     esac
> > > +
> > > +     test_expect_success PERL,TTY "delayed checkout shows progress by default only on tty ($mode checkout)" '
> >
> > All of the PERL,TTY can just be TTY, since TTY itself checks PERL.
> 
> I don't mind changing that, but isn't it a bit clearer for readers to
> have both dependencies explicitly?

No just clearer, but the perl dependency of TTY is an implementation
detail. It's conceivable that we could end up converting it to another
language (e.g., I recall there are at least some races with the stdin
mechanism, according to [0]).

-Peff

[0] https://lore.kernel.org/git/20190520125016.GA13474@sigill.intra.peff.net/

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-27  2:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-21 20:53 Bug report: 'filtering content' delayed progress message does not respect --quiet Sean Allred
2021-03-26  8:31 ` Jeff King
2021-08-25 18:15   ` [PATCH] checkout: make delayed checkout respect --quiet and --no-progress Matheus Tavares
2021-08-25 23:35     ` Ævar Arnfjörð Bjarmason
2021-08-26 14:26       ` Matheus Tavares Bernardino
2021-08-27  2:26         ` Jeff King
2021-08-26 19:10     ` [PATCH v2] " Matheus Tavares

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).