Thread (16 messages) flat view 16 messages, 3 authors, 2016-06-15
  • (off-list ancestor, not in this archive)
  • Re: git bugs · Junio C Hamano <hidden> · 2016-06-15
  • Re: git bugs · Ben Lynn <hidden> · 2016-06-15
  • Re: git bugs · Linus Torvalds <torvalds@linux-foundation.org> · 2016-06-15
  • Re: git bugs · Ben Lynn <hidden> · 2016-06-15
  • Re: git bugs · Linus Torvalds <torvalds@linux-foundation.org> · 2016-06-15
  • Re: git bugs · Ben Lynn <hidden> · 2016-06-15
  • Re: git bugs · Linus Torvalds <torvalds@linux-foundation.org> · 2016-06-15
  • Re: git bugs · Ben Lynn <hidden> · 2016-06-15
  • Re: git bugs · Ben Lynn <hidden> · 2016-06-15
  • Re: git bugs · Linus Torvalds <torvalds@linux-foundation.org> · 2016-06-15
  • Re: git bugs · Ben Lynn <hidden> · 2016-06-15
  • Re: git bugs · Linus Torvalds <torvalds@linux-foundation.org> · 2016-06-15
  • Re: git bugs · Ben Lynn <hidden> · 2016-06-15
  • Re: git bugs · Linus Torvalds <torvalds@linux-foundation.org> · 2016-06-15
  • Re: git bugs · Ben Lynn <hidden> · 2016-06-15
  • Re: git bugs · Linus Torvalds <torvalds@linux-foundation.org> · 2016-06-15

Re: git bugs

From: Ben Lynn <hidden>
Date: 2016-06-15 22:44:43

Am I going crazy? All of a sudden I think I can get away without a
size zero hack. How about this smudging routine:

if (!ce_match_stat_basic(ce, &st)) {
  recompute_sha1_and_update_index();  // no other checks required
}

That should be sufficient. I think what happened was the following.
Once upon a time, the race fix was "if (stats_match) cached_size = 0",
which is nice because you don't have to examine file contents. Later,
because of the

 $ echo xyzzy >frotz ; git-update-index --add frotz ; : >frotz
 $ sleep 3
 $ echo filfre >nitfol ; git-update-index --add nitfol

issue, the ce_modified_check_fs was added.

But then if we're going to be examining file contents anyway, we may
as well drop the whole size zero trick and simply update the hash. The
bug I brought up also goes away.

-Ben

P.S: I could go through the history to see, but I bet there was a
stage after the race condition was discovered but before it was
realized that
  $ git update-index 'foo'
  : modify 'foo' in-place without changing its size
  : wait for enough time
  $ git update-index 'bar'
was a problem.

On Tue, Jun 10, 2008 at 7:39 PM, Linus Torvalds
[off-list ref] wrote:

On Tue, 10 Jun 2008, Ben Lynn wrote:
quoted
Ah, I hadn't seen that. Yes, it is better to use the first write as
the timestamp. Would this catch everything? If the filesystem clock is
monotonically increasing and consistent then with this setup, you can
touch files even as they are being indexed? (Disregarding nonsense
like changing sizes by 2^32.)
Yes, I think that at that point it would protect against arbitrary
modifications even concurrently to index file creation.

That said, I don't think you even need a new index file format. We could
just do a stat() on starting the index file creation, and then do a
futimes() system call at the end to re-set the mtime to the beginning
before we rename it back over the old index file.

               Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help