Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Duy Nguyen]

On Sun, Feb 11, 2018 at 9:44 PM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Sat, Feb 10 2018, Duy Nguyen jotted:
>
>> On Sat, Feb 10, 2018 at 1:37 AM, Ævar Arnfjörð Bjarmason
>>  wrote:
>>>
>>> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>>>
 * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
  - dir.c: stop ignoring opendir() error in open_cached_dir()
  - update-index doc: note a fixed bug in the untracked cache
  - dir.c: fix missing dir invalidation in untracked code
  - dir.c: avoid stat() in valid_cached_dir()
  - status: add a failing test showing a core.untrackedCache bug

  Some bugs around "untracked cache" feature have been fixed.

  Will merge to 'next'.
>>>
>>> I think you / Nguyễn may not have seen my
>>> <87d11omi2o@evledraar.gmail.com>
>>> (https://public-inbox.org/git/87d11omi2o@evledraar.gmail.com/)
>>
>> I have. But since you wrote "I haven't found... yet", I assumed you
>> were still on it. You didn't give me much info to follow anyway.
>
> Haven't had time to dig further, sorry, and won't be able to share the
> repository. Is there some UC inspection command that can be run on the
> relevant path / other thing that'll be indicative of what went wrong?

There's test-dump-untracked-cache that will give you all the data.
>From then on, you may need to dig in the code a bit to see how that
data should be processed.

There's no obfuscation support in that command, unfortunately, so you
can't just send me the dump. But if you could limit it to a few
"blocks" related to the error messages, then manual obfuscation should
not take that much time (either that or just add obfuscation in
test-dump-untracked-cache.c, it's probably easier task; or I can do
this for you)

>>> As noted there I think it's best to proceed without the "dir.c: stop
>>> ignoring opendir[...]" patch.
>>>
>>> It's going to be a bad regression in 2.17 if it ends up spewing pagefuls
>>> of warnings in previously working repos if the UC is on.
>>
>> "previously working" is a strong word when opendir() starts to
>> complain. Sure we can suppress/ignore the error messages but I don't
>> think it's a healthy attitude. Unreported bugs can't be fixed.
>
> I mean that for the user it returned the right "git status" info and
> didn't print errors, but yeah, the index may have been in some
> internally funny state.

One question (I wasn't clear from your previous mail). Does "git
status" always report the same errors when run multiple times, or does
it just report once, then next "git status" is silent? I suppose it's
the former case..
-- 
Duy

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Ævar Arnfjörð Bjarmason]

On Sat, Feb 10 2018, Duy Nguyen jotted:

> On Sat, Feb 10, 2018 at 1:37 AM, Ævar Arnfjörð Bjarmason
>  wrote:
>>
>> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>>
>>> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>>>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>>>  - update-index doc: note a fixed bug in the untracked cache
>>>  - dir.c: fix missing dir invalidation in untracked code
>>>  - dir.c: avoid stat() in valid_cached_dir()
>>>  - status: add a failing test showing a core.untrackedCache bug
>>>
>>>  Some bugs around "untracked cache" feature have been fixed.
>>>
>>>  Will merge to 'next'.
>>
>> I think you / Nguyễn may not have seen my
>> <87d11omi2o@evledraar.gmail.com>
>> (https://public-inbox.org/git/87d11omi2o@evledraar.gmail.com/)
>
> I have. But since you wrote "I haven't found... yet", I assumed you
> were still on it. You didn't give me much info to follow anyway.

Haven't had time to dig further, sorry, and won't be able to share the
repository. Is there some UC inspection command that can be run on the
relevant path / other thing that'll be indicative of what went wrong?

>> As noted there I think it's best to proceed without the "dir.c: stop
>> ignoring opendir[...]" patch.
>>
>> It's going to be a bad regression in 2.17 if it ends up spewing pagefuls
>> of warnings in previously working repos if the UC is on.
>
> "previously working" is a strong word when opendir() starts to
> complain. Sure we can suppress/ignore the error messages but I don't
> think it's a healthy attitude. Unreported bugs can't be fixed.

I mean that for the user it returned the right "git status" info and
didn't print errors, but yeah, the index may have been in some
internally funny state.

> We could perhaps stop reporting after we have printed like ... 5 lines
> or so? That keeps the noise level down a bit and probably still give
> enough indicator to at least repair the cache.

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Duy Nguyen]

On Sat, Feb 10, 2018 at 1:37 AM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>
>> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>>  - update-index doc: note a fixed bug in the untracked cache
>>  - dir.c: fix missing dir invalidation in untracked code
>>  - dir.c: avoid stat() in valid_cached_dir()
>>  - status: add a failing test showing a core.untrackedCache bug
>>
>>  Some bugs around "untracked cache" feature have been fixed.
>>
>>  Will merge to 'next'.
>
> I think you / Nguyễn may not have seen my
> <87d11omi2o@evledraar.gmail.com>
> (https://public-inbox.org/git/87d11omi2o@evledraar.gmail.com/)

I have. But since you wrote "I haven't found... yet", I assumed you
were still on it. You didn't give me much info to follow anyway.

> As noted there I think it's best to proceed without the "dir.c: stop
> ignoring opendir[...]" patch.
>
> It's going to be a bad regression in 2.17 if it ends up spewing pagefuls
> of warnings in previously working repos if the UC is on.

"previously working" is a strong word when opendir() starts to
complain. Sure we can suppress/ignore the error messages but I don't
think it's a healthy attitude. Unreported bugs can't be fixed.

We could perhaps stop reporting after we have printed like ... 5 lines
or so? That keeps the noise level down a bit and probably still give
enough indicator to at least repair the cache.
-- 
Duy

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>
>> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>>  - update-index doc: note a fixed bug in the untracked cache
>>  - dir.c: fix missing dir invalidation in untracked code
>>  - dir.c: avoid stat() in valid_cached_dir()
>>  - status: add a failing test showing a core.untrackedCache bug
>>
>>  Some bugs around "untracked cache" feature have been fixed.
>>
>>  Will merge to 'next'.
>
> I think you / Nguyễn may not have seen my
> <87d11omi2o@evledraar.gmail.com>
> (https://public-inbox.org/git/87d11omi2o@evledraar.gmail.com/)
>
> As noted there I think it's best to proceed without the "dir.c: stop
> ignoring opendir[...]" patch.
>
> It's going to be a bad regression in 2.17 if it ends up spewing pagefuls
> of warnings in previously working repos if the UC is on.

Well, I am not sure if it is a regression to diagnose problematic
untracked cache information left by earlier version of the software
with bugs.  After all, this is still an experimental feature, and we
do want to see the warning to serve its purpose to diagnose possible
remaining bugs, and new similar ones when a newer iteration of the
feature introduces, no?

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 01 2018, Junio C. Hamano jotted:

> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>  - update-index doc: note a fixed bug in the untracked cache
>  - dir.c: fix missing dir invalidation in untracked code
>  - dir.c: avoid stat() in valid_cached_dir()
>  - status: add a failing test showing a core.untrackedCache bug
>
>  Some bugs around "untracked cache" feature have been fixed.
>
>  Will merge to 'next'.

I think you / Nguyễn may not have seen my
<87d11omi2o@evledraar.gmail.com>
(https://public-inbox.org/git/87d11omi2o@evledraar.gmail.com/)

As noted there I think it's best to proceed without the "dir.c: stop
ignoring opendir[...]" patch.

It's going to be a bad regression in 2.17 if it ends up spewing pagefuls
of warnings in previously working repos if the UC is on.

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Ævar Arnfjörð Bjarmason]

On Fri, Feb 09 2018, Johannes Schindelin jotted:

> Hi,
>
> On Thu, 1 Feb 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>>
>> > * ab/wildmatch-tests (2018-01-30) 10 commits
>> >  - wildmatch test: mark test as EXPENSIVE_ON_WINDOWS
>> >  - test-lib: add an EXPENSIVE_ON_WINDOWS prerequisite
>> >  - wildmatch test: create & test files on disk in addition to in-memory
>> >  - wildmatch test: perform all tests under all wildmatch() modes
>> >  - wildmatch test: use test_must_fail, not ! for test-wildmatch
>> >  - wildmatch test: remove dead fnmatch() test code
>> >  - wildmatch test: use a paranoia pattern from nul_match()
>> >  - wildmatch test: don't try to vertically align our output
>> >  - wildmatch test: use more standard shell style
>> >  - wildmatch test: indent with tabs, not spaces
>> >
>> >  More tests for wildmatch functions.
>> >
>> >  Expecting an update.
>> >  cf. <87vaga9mgf@evledraar.gmail.com>
>>
>> The 2018-01-30 series is the update mentioned in
>> 87vaga9mgf@evledraar.gmail.com. You probably noticed this / just
>> didn't adjust the note since you queued in in pu already, but just in
>> case: the known issues in it have been resolved, but hopefully Johannes
>> Schindelin can test it on Windows & report.
>
> Sorry, I did not have time to look at this. All I can say is that the `pu`
> builds are green for a couple of days already. Which I celebrate!

Thanks, if you get time it would be great to know if:

time GIT_TEST_LONG=1 ./t3070-wildmatch.sh

Runs cleanly for you, and how long it takes now. Even though it's going
to take a long time still, I optimized the test a lot so I expect it'll
be quicker.

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Johannes Schindelin]

Hi,

On Thu, 1 Feb 2018, Ævar Arnfjörð Bjarmason wrote:

> On Thu, Feb 01 2018, Junio C. Hamano jotted:
> 
> > * ab/wildmatch-tests (2018-01-30) 10 commits
> >  - wildmatch test: mark test as EXPENSIVE_ON_WINDOWS
> >  - test-lib: add an EXPENSIVE_ON_WINDOWS prerequisite
> >  - wildmatch test: create & test files on disk in addition to in-memory
> >  - wildmatch test: perform all tests under all wildmatch() modes
> >  - wildmatch test: use test_must_fail, not ! for test-wildmatch
> >  - wildmatch test: remove dead fnmatch() test code
> >  - wildmatch test: use a paranoia pattern from nul_match()
> >  - wildmatch test: don't try to vertically align our output
> >  - wildmatch test: use more standard shell style
> >  - wildmatch test: indent with tabs, not spaces
> >
> >  More tests for wildmatch functions.
> >
> >  Expecting an update.
> >  cf. <87vaga9mgf@evledraar.gmail.com>
> 
> The 2018-01-30 series is the update mentioned in
> 87vaga9mgf@evledraar.gmail.com. You probably noticed this / just
> didn't adjust the note since you queued in in pu already, but just in
> case: the known issues in it have been resolved, but hopefully Johannes
> Schindelin can test it on Windows & report.

Sorry, I did not have time to look at this. All I can say is that the `pu`
builds are green for a couple of days already. Which I celebrate!

Ciao,
Dscho

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Ævar Arnfjörð Bjarmason]

On Fri, Feb 02 2018, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason  writes:
>
>> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>>
>>> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>>>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>>>  - update-index doc: note a fixed bug in the untracked cache
>>>  - dir.c: fix missing dir invalidation in untracked code
>>>  - dir.c: avoid stat() in valid_cached_dir()
>>>  - status: add a failing test showing a core.untrackedCache bug
>>>
>>>  Some bugs around "untracked cache" feature have been fixed.
>>>
>>>  Will merge to 'next'.
>>
>> The "update-index doc: note a fixed bug in the untracked cache" needs to
>> be amended so it doesn't say "Before 2.16, ":
>
> True; we could just say "earlier", but I am inclined to suggest that
> we get drop it altogether.  Description of historical bugs is of no
> interest with the version that already fixes them, so the _only_
> value the doc update adds is to tell readers that the untracked
> cache feature is still not well proven, and core.untrackedCache may
> serve as an escape hatch from its bugs by disabling the mechanism
> added for the feature.  I am *not* opposed to a replacement of the
> patch that just says something like "This feature has been cause of
> bugs even in recent versions of Git, and you may want to disable it
> as a workaround when you are hit by an otherwise undiscovered bug in
> this area", though.

 - It's my experience that most users today who aren't *nix graybeards
   don't use the documentation shipped on their system as their primary
   source for docs.

   They go to Google and might find the manpage there. Thus this
   documentation will be read by users on pre-2.17 (or whenever this bug
   fix gets included).

 - This is very useful information if you're deploying
   core.untrackedCache across a site with differing git versions. Just
   because you have 2.17 doesn't mean everywhere you're about to deploy
   core.untrackedCache does.

 - In general I agree that we shouldn't be documenting old bugs, but I
   think in this case it makes sense since the bug's really bad. Without
   thinking to disable core.untrackedCache there's seemingly no way to
   fix it without wiping away the index, which might lose you work.

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>
>> * ab/wildmatch-tests (2018-01-30) 10 commits
> The 2018-01-30 series is the update mentioned in
> 87vaga9mgf@evledraar.gmail.com. You probably noticed this / just
> didn't adjust the note since you queued in in pu already, but just in
> case: the known issues in it have been resolved, but hopefully Johannes
> Schindelin can test it on Windows & report.

Thanks for a correction.  Very much appreciated.  Let's start moving
this forward then.
>> * ab/sha1dc-build (2017-12-12) 4 commits
>>  . Makefile: use the sha1collisiondetection submodule by default
>>  - sha1dc_git.h: re-arrange an ifdef chain for a subsequent change
>>  - Makefile: under "make dist", include the sha1collisiondetection submodule
>>  - Makefile: don't error out under DC_SHA1_EXTERNAL if DC_SHA1_SUBMODULE=auto
> Do you want to peel of 4/4 and just keep 1-3 should I submit another
> version without 4/4?

Nah, let's just discard the tip one without prejudice and move the
remainder forward.

Thanks.

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>
>> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>>  - update-index doc: note a fixed bug in the untracked cache
>>  - dir.c: fix missing dir invalidation in untracked code
>>  - dir.c: avoid stat() in valid_cached_dir()
>>  - status: add a failing test showing a core.untrackedCache bug
>>
>>  Some bugs around "untracked cache" feature have been fixed.
>>
>>  Will merge to 'next'.
>
> The "update-index doc: note a fixed bug in the untracked cache" needs to
> be amended so it doesn't say "Before 2.16, ":

True; we could just say "earlier", but I am inclined to suggest that
we get drop it altogether.  Description of historical bugs is of no
interest with the version that already fixes them, so the _only_
value the doc update adds is to tell readers that the untracked
cache feature is still not well proven, and core.untrackedCache may
serve as an escape hatch from its bugs by disabling the mechanism
added for the feature.  I am *not* opposed to a replacement of the
patch that just says something like "This feature has been cause of
bugs even in recent versions of Git, and you may want to disable it
as a workaround when you are hit by an otherwise undiscovered bug in
this area", though.

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 01 2018, Junio C. Hamano jotted:

> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>  - update-index doc: note a fixed bug in the untracked cache
>  - dir.c: fix missing dir invalidation in untracked code
>  - dir.c: avoid stat() in valid_cached_dir()
>  - status: add a failing test showing a core.untrackedCache bug
>
>  Some bugs around "untracked cache" feature have been fixed.
>
>  Will merge to 'next'.

The "update-index doc: note a fixed bug in the untracked cache" needs to
be amended so it doesn't say "Before 2.16, ":


https://github.com/gitster/git/commit/b9fc38c9f90b8bf2c9147ad536813b66aa45220d#diff-01fe1588047a177245bfaf82336cdeaeR467

I'm not sure whether you're planning this for 2.16.2, or whether it'll
be in 2.17.0, but the patch should be amended to mention either one of
those.

I can submit a replacement patch, but figured this was trivial enough
(and you know more about the release plan) that you'd like to amend this
locally.

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Ævar Arnfjörð Bjarmason]

On Thu, Feb 01 2018, Junio C. Hamano jotted:

> * ab/wildmatch-tests (2018-01-30) 10 commits
>  - wildmatch test: mark test as EXPENSIVE_ON_WINDOWS
>  - test-lib: add an EXPENSIVE_ON_WINDOWS prerequisite
>  - wildmatch test: create & test files on disk in addition to in-memory
>  - wildmatch test: perform all tests under all wildmatch() modes
>  - wildmatch test: use test_must_fail, not ! for test-wildmatch
>  - wildmatch test: remove dead fnmatch() test code
>  - wildmatch test: use a paranoia pattern from nul_match()
>  - wildmatch test: don't try to vertically align our output
>  - wildmatch test: use more standard shell style
>  - wildmatch test: indent with tabs, not spaces
>
>  More tests for wildmatch functions.
>
>  Expecting an update.
>  cf. <87vaga9mgf@evledraar.gmail.com>

The 2018-01-30 series is the update mentioned in
87vaga9mgf@evledraar.gmail.com. You probably noticed this / just
didn't adjust the note since you queued in in pu already, but just in
case: the known issues in it have been resolved, but hopefully Johannes
Schindelin can test it on Windows & report.

> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>  - update-index doc: note a fixed bug in the untracked cache
>  - dir.c: fix missing dir invalidation in untracked code
>  - dir.c: avoid stat() in valid_cached_dir()
>  - status: add a failing test showing a core.untrackedCache bug
>
>  Some bugs around "untracked cache" feature have been fixed.
>
>  Will merge to 'next'.

It must be Murphy's law or something, but even though the bug has been
there for years I just had some internal users again run into the bug
this fixes, so I'm building an internal v2.16.1 + custom patches (this
included).

> * ab/sha1dc-build (2017-12-12) 4 commits
>  . Makefile: use the sha1collisiondetection submodule by default
>  - sha1dc_git.h: re-arrange an ifdef chain for a subsequent change
>  - Makefile: under "make dist", include the sha1collisiondetection submodule
>  - Makefile: don't error out under DC_SHA1_EXTERNAL if DC_SHA1_SUBMODULE=auto
>
>  Push the submodule version of collision-detecting SHA-1 hash
>  implementation a bit harder on builders.
>
>  The earlier two may make sense, but leaning toward rejecting the last step.
>  cf. 

This has been lingering for a long time. I think it makes sense just to
merge the first 3 down and then we can discuss 4/4 in another
submission. [12]/4 solve real bugs we have now, 3/4 is harmless to merge
down (and makes 4/4 smaller when we get around to discussing it again),
it's just 4/4 that's been stalling this.

Do you want to peel of 4/4 and just keep 1-3 should I submit another
version without 4/4?