git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Kristian Høgsberg <krh@redhat.com>
Date
Nov 9, 2007, 18:39 UTC
Message-ID
<1194633596.30909.18.camel@hinata.boston.redhat.com>
In-Reply-To
<7vhcjvtgz5.fsf@gitster.siamese.dyndns.org>
On Fri, 2007-11-09 at 10:24 -0800, Junio C Hamano wrote:
Show 41 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> 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.

Show 8 quoted lines
> 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.

Show 8 quoted lines
> 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

Previous: Junio C HamanoNext: Junio C Hamano
Message 7 of 14 in “builtin-commit: Refresh cache after adding files.”
  1. builtin-commit: Refresh cache after adding files.Kristian Høgsberg, Nov 9, 2007
  2. Johannes SchindelinNov 9, 2007
  3. Kristian HøgsbergNov 9, 2007
  4. Johannes SchindelinNov 9, 2007
  5. Kristian HøgsbergNov 9, 2007
  6. Junio C HamanoNov 9, 2007
  7. Kristian HøgsbergNov 9, 2007
  8. Junio C HamanoNov 10, 2007
  9. Johannes SchindelinNov 10, 2007
  10. Junio C HamanoNov 10, 2007
  11. git-add: make the entry stat-clean after re-adding the same contentsJunio C Hamano, Nov 10, 2007
  12. Linus TorvaldsNov 10, 2007
  13. 1/2 ce_match_stat, run_diff_files: use symbolic constants for readabilityJunio C Hamano, Nov 10, 2007
  14. 2/2 git-add: make the entry stat-clean after re-adding the same contentsJunio C Hamano, Nov 10, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.