Re: [PATCH] builtin-commit: Refresh cache after adding files.

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 9, 2007, 18:24 UTC
Message-ID: <7vhcjvtgz5.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <Pine.LNX.4.64.0711091702190.4362@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 9 quoted lines

> 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".

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.

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.