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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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.

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux