gc and repack ignore .git/*HEAD when checking reachability

3 messages, 2 authors, 2016-07-08 · open the first message on its own page

gc and repack ignore .git/*HEAD when checking reachability

From: Josh Triplett <josh@joshtriplett.org>
Date: 2016-07-08 03:00:01

The manpage for git gc says:
git gc tries very hard to be safe about the garbage it collects. In
particular, it will keep not only objects referenced by your current
set of branches and tags, but also objects referenced by the index,
remote-tracking branches, refs saved by git filter-branch in
refs/original/, or reflogs (which may reference commits in branches
that were later amended or rewound).
gc, repack, and anything else that uses the machinery in reachable.c
will also check HEAD, to include objects only reachable from a detached
HEAD (which the manpage should document).

However, unreachable.c does not check any other ref that sits directly
in the .git directory, such as MERGE_HEAD, FETCH_HEAD, or
CHERRY_PICK_HEAD.  To test this, try creating a new empty repository
with "git init repo ; cd repo", then use "git fetch URL" to fetch a
repository into FETCH_HEAD, then run "git repack -a -d -f", and then
"git show FETCH_HEAD".  This similarly affects "git gc", which will
unpack all the objects from the pack and leave them loose.

This could result in data loss, if a user expected that having an object
referenced from those places would protect it from pruning.

I think the right fix for this would involve having
mark_reachable_objects in reachable.c add all refs that match
.git/*HEAD, not just .git/HEAD itself.  (I'd suggest matching .git/*HEAD
rather than hardcoding the list of "special" refs, to provide
compatibility with any other tool or future version of git that
introduces another such ref.)  This seems fairly easily done with a new
variant of do_head_ref that includes all such refs, along with a
one-line change to mark_reachable_objects to use it.

Does this seem like a reasonable approach?

- Josh Triplett

Re: gc and repack ignore .git/*HEAD when checking reachability

From: Junio C Hamano <hidden>
Date: 2016-07-08 04:34:12

Josh Triplett [off-list ref] writes:
This could result in data loss, if a user expected that having an object
referenced from those places would protect it from pruning.
Yeah, luckily, nobody expects such.  I do not think any of our
document says nothing other than HEAD like CHERRY_PICK_HEAD is
reachability anchoring point; they are designed to be transient.

Because they are designed to be transient, I do not think there is
any downside (other than the initial start-up cost) to including
them in reachability computation.  Because they are meant to be
transient, the objects anchored by them would be reachable from
other anchoring points anyway.

Re: gc and repack ignore .git/*HEAD when checking reachability

From: Josh Triplett <josh@joshtriplett.org>
Date: 2016-07-08 06:45:01

On Thu, Jul 07, 2016 at 09:34:02PM -0700, Junio C Hamano wrote:
Josh Triplett [off-list ref] writes:
quoted
This could result in data loss, if a user expected that having an object
referenced from those places would protect it from pruning.
Yeah, luckily, nobody expects such.  I do not think any of our
document says nothing other than HEAD like CHERRY_PICK_HEAD is
reachability anchoring point; they are designed to be transient.
I can imagine at least one scenario that would result in data loss here:
git pull a URL (not referenced via any ref other than
FETCH_HEAD/MERGE_HEAD), get a merge conflict, get halfway through
resolving it, set that repository aside for a while, do something that
triggers a gc, then attempt to finish and commit.

Unlikely, but not impossible.  Same reason the reachability logic looks
at the index.

(I originally encountered this because I intended to add another
HEAD-like ref in .git, so I started investigating the logic around such
HEADs.)
Because they are designed to be transient, I do not think there is
any downside (other than the initial start-up cost) to including
them in reachability computation.  Because they are meant to be
transient, the objects anchored by them would be reachable from
other anchoring points anyway.
That sounds reasonable.  And if they *do* end up taking any time to
traverse, it's because they weren't reachable from other anchoring
points, so taking the extra time to traverse them seems fine.

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