On Tue, 26 Feb 2008, Brandon Casey wrote:
> 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.
> 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();
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
