Re: "git am --abort" screwing up index?

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

 



On Sun, Aug 16, 2015 at 12:46 PM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Maybe it has always done this, and I just haven't noticed (I usually
> _just_ do the "git reset --hard" thing, don't ask me why I wanted to
> be doubly sure this time). But maybe it's an effect of the new
> built-in "am".

I bisected this. It's definitely used to work, and the regression is
from the new built-in am. But I cannot bisect into that branch
'pt/am-builtin', because "git am" doesn't actually work in the middle
of that branch.

So I've verified that commit c1e5ca90dba8 ("Merge branch
'es/worktree-add'") is good, and that commit 7aa2da616208 ("Merge
branch 'pt/am-builtin'") is bad, but I cannot pinpoint the exact
commit where "git am --abort" starts breaking the index.

But I assume it's simply that initial implementation of "--abort" in
commit 33388a71d23e ("builtin-am: implement --abort") that already
ends up rewriting the index from scratch without applying the old stat
data.

The test-case is pretty simple: just force a "git am" failure, then do
"git am --abort", and then you can check whether the index stat()
information is valid in various ways. For the kernel, doing a "git
reset --hard" makes it obvious because the reset will force all files
to be written out (since the index stat information doesn't match the
current tree). But you can do it by just counting system calls for a
"git diff" too. On the git tree, for example, when the index has
matching stat information, you get something like

  [torvalds@i7 git]$ strace -cf git diff
  ..
    0.04    0.000025           1        26         4 open
  ..

ie you only actually ended up with 26 open() system calls. When the
index is not in sync with the stat information, "git diff" will have
to open each file to see what the actual contents are, and you get

  [torvalds@i7 git]$ strace -cf git diff
  ...
    0.30    0.000070           0      5987       302 open
  ...

so now it opened about 6k files instead (and for the kernel, that
number will be much larger, of course).

I _think_ it's because git-am (in "clean_index()") uses read_tree(),
while it probably should use "unpack_trees" with opts.update and
opts.reset set (like reset_index() does in builtin/reset.h).

I have to go off do my weekly -rc now, and probably won't get to
debugging this much further. Adding Stefan to the cc, since he helped
with that "--abort" implementation.

          Linus
--
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]