Re: [PATCH] pack-objects: Print a message describing the number of threads for packing
- From
Nicolas Pitre <nico@cam.org>
- Date
- Feb 26, 2008, 17:05 UTC
- Message-ID
- <alpine.LFD.1.00.0802261149220.3167@xanadu.home>
- In-Reply-To
- <47C435DC.2070508@nrlssc.navy.mil>
On Tue, 26 Feb 2008, Brandon Casey wrote:
Show 29 quoted lines
> Jeff King wrote: > > On Fri, Feb 22, 2008 at 08:12:58PM -0600, Brandon Casey wrote: > > > >> + if (progress) > >> + fprintf(stderr, "Using %d pack threads.\n", > >> + delta_search_threads); > > > > I just noticed that this was in next. Do we really need to display this > > message? A considerable amount of discussion went into reducing git's > > chattiness and clutter during push and fetch, and I feel like this is a > > step backwards (yes, I know most people won't see it if they don't build > > with THREADED_DELTA_SEARCH). > > > > Can we show it only if threads != 1? Only if we auto-detected the number > > of threads and it wasn't 1? > > I like the message and thought it was useful especially for non-developers. > > Even if the number of threads was not auto-detected, it is a confirmation > that the number of threads used is the number of threads configured. > > For example, it seems easy to do this: > > git config pack.thread 4 > git repack > > The user would immediately know something was wrong when they saw the > message "Using 1 pack threads" instead of the "4" they thought they > configured.
Maybe a message for any unrecognized config option should be displayed instead.
Show 7 quoted lines
> Also, since it's only printed in the THREADED_DELTA_SEARCH > case, it's also a confirmation that this option was indeed used for a > particular build of git. > > Mainly, I thought it was a harmless message that other users would "enjoy" > seeing, but if others disagree, I won't argue. Notice I quoted "enjoy" to > emphasize it.
This is enjoyable maybe the first time, but that might get useless/annoying after a while. I think that displaying it in the autodetection case is a good compromize, and then simply specifying the number of threads explicitly will silence it.
Also, I think that such message should absolutely not be sent over in the context of a fetch/clone. This is a local matter only, and should be displayed only when those progress messages are meant for the local user.
Therefore I propose this patch instead:
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index b70b2e5..6dcb4e2 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -2236,11 +2236,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) die("--thin cannot be used to build an indexable pack."); #ifdef THREADED_DELTA_SEARCH - if (!delta_search_threads) /* --threads=0 means autodetect */ + if (!delta_search_threads) { /* --threads=0 means autodetect */ delta_search_threads = online_cpus(); - if (progress) - fprintf(stderr, "Using %d pack threads.\n", - delta_search_threads); + if (progress > pack_to_stdout) + fprintf(stderr, "Using %d pack threads.\n", + delta_search_threads); + } #endif prepare_packed_git();