Subject: Re: [PATCH 28/29] grep: given --threads with NO_PTHREADS=YesPlease, warn

From: Brandon Williams <[email-masked]>
Date: Thu, 11 May 2017 13:21:31 -0700

On 05/11, Ævar Arnfjörð Bjarmason wrote:
> Add a warning about missing thread support when grep.threads or
> --threads is set to a non 0 (default) or 1 (no parallelism) value
> under NO_PTHREADS=YesPlease.
> 
> This is for consistency with the index-pack & pack-objects commands,
> which also take a --threads option & are configurable via
> pack.threads, and have long warned about the same under
> NO_PTHREADS=YesPlease.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> ---
>  builtin/grep.c  | 11 +++++++++++
>  t/t7810-grep.sh | 18 ++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 1c0adb30f3..f4e08dd2b6 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -289,6 +289,15 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
>  		if (num_threads < 0)
>  			die(_("invalid number of threads specified (%d) for %s"),
>  			    num_threads, var);
> +#ifdef NO_PTHREADS
> +		else if (num_threads && num_threads != 1) {
> +			/* TRANSLATORS: %s is the configuration
> +			   variable for tweaking threads, currently
> +			   grep.threads */

nit: this comment isn't formatted properly:
  /*
   * ... comment ...
   */

> +			warning(_("no threads support, ignoring %s"), var);
> +			num_threads = 0;
> +		}
> +#endif
>  	}
>  
>  	return st;
> @@ -1233,6 +1242,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  	else if (num_threads < 0)
>  		die(_("invalid number of threads specified (%d)"), num_threads);
>  #else
> +	if (num_threads)
> +		warning(_("no threads support, ignoring --threads"));
>  	num_threads = 0;
>  #endif
>  
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 561709ef6a..f106387820 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -791,6 +791,24 @@ do
>  	"
>  done
>  
> +test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'grep --threads=N or pack.threads=N warns when no pthreads' '
> +	git grep --threads=2 Hello hello_world 2>err &&
> +	grep ^warning: err >warnings &&
> +	test_line_count = 1 warnings &&
> +	grep -F "no threads support, ignoring --threads" err &&
> +	git -c grep.threads=2 grep Hello hello_world 2>err &&
> +	grep ^warning: err >warnings &&
> +	test_line_count = 1 warnings &&
> +	grep -F "no threads support, ignoring grep.threads" err &&
> +	git -c grep.threads=2 grep --threads=4 Hello hello_world 2>err &&
> +	grep ^warning: err >warnings &&
> +	test_line_count = 2 warnings &&
> +	grep -F "no threads support, ignoring --threads" err &&
> +	grep -F "no threads support, ignoring grep.threads" err &&
> +	git -c grep.threads=0 grep --threads=0 Hello hello_world 2>err &&
> +	test_line_count = 0 err
> +'
> +

Same bit about doing the correct checks on the error strings to account
for translation.

>  test_expect_success 'grep from a subdirectory to search wider area (1)' '
>  	mkdir -p s &&
>  	(
> -- 
> 2.11.0
> 

-- 
Brandon Williams