Re: [PATCH v13 04/20] index-helper: new daemon for caching index and related stuff

From: Duy Nguyen <pclouds@gmail.com>
Date: Jul 2, 2016, 12:43 UTC
Message-ID: <CACsJy8Bke0EQ9rOEBmz6BcvcfPe-X_rktaOku9F-wUOrnVMJDw@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.20.1607021312210.12947@virtualbox>

On Sat, Jul 2, 2016 at 1:20 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 23 quoted lines

>> > -test -n "$NO_MMAP" && {
>> > -       skip_all='skipping index-helper tests: no mmap'
>> > +git index-helper -h 2>/dev/null
>> > +test $? = 129 ||
>>
>> So when NO_MMAP is set, "git index-helper -h" will set $? to 1.
>
> Not quite.
>
> When NO_MMAP is set, index-helper will not be compiled. Or at least it
> should not be:
>
>> +ifndef NO_MMAP
>> +ifndef NO_UNIX_SOCKETS
>> +       PROGRAM_OBJS += index-helper.o
>> +endif
>> +endif
>
> If it is *unset*, *and* if NO_UNIX_SOCKETS is *also* unset, index-helper
> gets compiled, and -h triggers code in parse-options.c or usage.c that
> exits with status 129.
>
> So I do not think that this is subtle.

The question is "whether index-helper is supported?" but you need to go through parse-options.c and that strange (to me) exit status 129 to conclude that index-helper is in fact built. It's not just as straight forwarding from reading the test.

> But it just occurred to me that the #ifndef NO_MMAP in index-helper.c is
> unnecessary and that its #else clause (containing a loop() that fails)
> contains dead code: we never compile this code with NO_MMAP, and neither
> should we.

Yeah that's probably old code from POSIX shm time. Back then there were calls to shm_open and stuff, not just mmap.

> Dave, would you mind taking that #ifndef NO_MMAP out of "index-helper: new
> daemon for caching index and related stuff" when you re-roll?

-- 
Duy