Linus Torvalds [off-list ref] writes:
I dunno. I like this patch better. It's a bit larger. I think it's a bit
more clearly separated (ie a "mark_object_reachable()" _literally_ just
puts the object on a list, and the whole traversal is a whole separate
phase), but I guess it's a matter of taste.
... which happens to match mine in this case ;-)
I'll consider this signed-off and do the usual forging (for people new on
the list, Cf. http://article.gmane.org/gmane.comp.version-control.git/19031).
On Wed, 10 Dec 2008, Junio C Hamano wrote:
I'll consider this signed-off and do the usual forging
Yea. I've even tested it a bit now:
[torvalds@nehalem git]$ ulimit -s 1024
[torvalds@nehalem git]$ git fsck --full
Segmentation fault
[torvalds@nehalem git]$ ./git-fsck --full
dangling commit 3d00b49495ceff119de52dc5443731e2d8d84b6b
dangling commit 4e0a3c7de9af3cbb53cc421329f0579679edbb51
...
so it does seem to fix the issue, and the patch looks safe enough.
It passes all the tests, and works fine on the kernel repo too (ugh, four
minutes! I used to run git-fsck religiously every day back in the early
days, now I realized that I must not have done so in _months_, and my
kernel tree has grown and so has fsck time).
But obviously the true test for fsck is some complex corruption, and I
didn't test that. I can't imagine that it introduces any new problems
though - but the bugs you can't imagine are always the worst ones ;)
Linus
On Thu, 11 Dec 2008, Linus Torvalds wrote:
But obviously the true test for fsck is some complex corruption, and I
didn't test that. I can't imagine that it introduces any new problems
though - but the bugs you can't imagine are always the worst ones ;)
Btw, even if it doesn't introduce any bugs, it _does_ change the order
that we traverse things in. It shouldn't matter, of course, but because it
always picks the last entry from the object array (it really treats the
array as a stack), it ends up traversing parents of commits (and the
entries in trees) by looking at the last parent (or entry) first.
The whole two-phase thing also means that rather traverse the references
as we find them, we'll end up traversing things later in one group. Again,
access ordering will change.
Absolutely nothing should care about this from a correctness angle, of
course, but I thought I'd point it out because I think it will change the
order that we print out errors in.
So if somebody has some test-case, and you get different output
before-and-after, it's not necessarily any indication of a problem, just
an effect of doing object traversal in slightly different order.
Linus