On Sat, Jan 30, 2010 at 7:29 PM, Nicolas Pitre <nico@xxxxxxxxxxx> wrote: > On Sat, 30 Jan 2010, Junio C Hamano wrote: > >> Dan McGee <dpmcgee@xxxxxxxxx> writes: >> >> > When the first piece of threaded code was introduced in commit 8ecce684, it >> > came with its own THREADED_DELTA_SEARCH Makefile option. Since this time, >> > more threaded code has come into the codebase and a NO_PTHREADS option has >> > also been added. Get rid of the original option as the newer, more generic >> > option covers everything we need. >> >> The patch is a good change but only in the "if it were like this from day >> one, things would have been much simpler" sense. It is a breakage to >> existing users with customized THREADED_DELTA_SEARCH in their config.mak >> files, isn't it? > > I think that the release of v1.7.0 is the perfect match for such a > "breakage". Unlike for the dashless move, I really doubt the majority > of Git users are using a customized THREADED_DELTA_SEARCH in a > config.mak if they do build Git themselves at all. So very few people > are likely to be inconvenienced, and yet the inconvenience can hardly be > qualified as a breakage since nothing will stop working in any case. > >> If we take only the part of your patch that applies to Makefile, but >> exclude the first hunk (description of THREADED_DELTA_SEARCH) and the last >> hunk (the necessary part to keep THREADED_DELTA_SEARCH working), and >> instead add something like: >> >> ifndef NO_PTHREADS >> THREADED_DELTA_SEARCH = YesPlease >> endif >> >> immediately before we include config.mak, would that be a workable >> solution to: >> >> (1) keep existing users happy; >> >> (2) remove the redundant logic to compute the default for two Make >> variables; and >> >> (3) keep control over use of threading in general _and_ use of >> threading in delta computation? > > IMHO I wouldn't bother that much. Simply mentioning in the 1.7.0 > release notes that THREADED_DELTA_SEARCH is no more should be fine. > Like I said, the existing users who might be affected are certainly few, > and the impact on them is rather trivial. This is everything I wanted to say as well. I just don't think this is that big of a deal to "break compatibility" as I can think of no reason why you would only want 2/3 of the pthreads code enabled (your point 3). What this does do is set the precedent for any future threads code to only use NO_PTHREADS and not introduce yet another preprocessor define. If this gets applied, it needs a small correction- I can resubmit, but the only difference is this: diff --git a/config.mak.in b/config.mak.in index 67b12f7..6008ac9 100644 --- a/config.mak.in +++ b/config.mak.in @@ -56,5 +56,4 @@ NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@ FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@ SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@ NO_PTHREADS=@NO_PTHREADS@ -THREADED_DELTA_SEARCH=@THREADED_DELTA_SEARCH@ PTHREAD_LIBS=@PTHREAD_LIBS@ -Dan -- 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