On Fri, 2007-11-09 at 10:24 -0800, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> > On Fri, 9 Nov 2007, Kristian Høgsberg wrote:
> >
> >> This fixes the race in the last test int t3700.
> >
> > Well, it is not a race. My fault. I thought it was.
> >
> > What you basically did was to make sure that the index is up-to-date after
> > adding the files. You might even want to say that in the commit message,
> > and only then say that it fixes t3700, too.
>
> Ah, it all makes sense. I should have been more relaxed and
> careful when I first read your t3700 patch.
>
> If this were a breakage in racy-git-avoidance code, then
> refresh_cache() Kristian added (or "add --refresh" immediately
> after "git commit" in the test script) would have been fooled by
> the same raciness and wouldn't have changed a thing.
>
> This discussion exposes a problem with add_files_to_cache()
> function.
>
> Try this in a clean repository that tracks Makefile:
>
> $ git diff-files --name-only ;# no output
> $ touch Makefile
> $ git diff-files --name-only
> Makefile
> $ git add -u
> $ git diff-files --name-only
> Makefile
> $ git add Makefile
> $ git diff-files --name-only ;# no output
>
> I think this is a broken behaviour. As long as we are adding
> the contents from a path on the filesystem to the corresponding
> location in the index, it should sync the stat bits for the
> entry in the index with the filesystem to make the entry
> stat-clean. IOW, we should not see stat-dirtiness reported
> after "add -u".
Yup, that's what I expected, and why I couldn't quite understand why the
refresh was necessary.
> The funny thing is that add_files_to_cache() run by "git add -u"
> calls add_file_to_cache() to add the updated contents for
> touched paths, but the latter function is used in the more
> explicit "git add Makefile" codepath, without any magic
> postprocessing after the function returns to sync the stat
> info. IOW, both "add -u" and "add Makefile" ends up calling
> add_file_to_cache("Makefile") and I do not see why we are
> getting different results.
There is some timing involved in this, which may explain the different
results you see. On my laptop I can trigger the add_files_to_cache()
problem roughly 4 out of 5 times, but on my faster desktop, it can't
trigger it.
> And add_file_to_cache(), which calls add_file_to_index() on
> the_index, does call the fill_stat_cache_info() to sync the stat
> information by itself, as it should be. I am still puzzled with
> this.
>
> So I think Kristian's two refresh_cache() do fix the issue, but
> at the same time I _think_ it is a workaround for broken
> add_files_to_cache() behaviour, which is what we should fix.
OK, I'll resend the patch with a better description, and add a refresh
call for the user index too, for the git commit <file> case.
cheers,
Kristian
-
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
