Hi Duy,
On Thu, 30 Jun 2016, Duy Nguyen wrote:
> On Thu, Jun 30, 2016 at 3:06 PM, Johannes Schindelin
> <johannes.schinde...@gmx.de> wrote:
> > Even when NO_MMAP is empty, there might be no Unix sockets available (such
> > as is the case on Windows). In any case, you really only want to skip
> > these tests when index-helper is not available, so would you mind
> > squashing this patch in?
> >
> > -- snipsnap --
> > From: Johannes Schindelin <johannes.schinde...@gmx.de>
> > Subject: [PATCH] fixup! index-helper: new daemon for caching index and
> > related
> > stuff
> >
> > ---
> > t/t7900-index-helper.sh | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
> > index 6c9b4dd..12b5bf7 100755
> > --- a/t/t7900-index-helper.sh
> > +++ b/t/t7900-index-helper.sh
> > @@ -10,8 +10,10 @@ Testing git index-helper
> >
> > . ./test-lib.sh
> >
> > -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.
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.
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?
Thanks,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
