All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Dmitry Neverov <dmitry.neverov@gmail.com>, git@vger.kernel.org
Subject: Re: Git gc removes all packs
Date: Tue, 17 Feb 2015 11:55:15 -0500	[thread overview]
Message-ID: <20150217165514.GA12176@peff.net> (raw)
In-Reply-To: <54E36EBF.2070600@alum.mit.edu>

On Tue, Feb 17, 2015 at 05:39:27PM +0100, Michael Haggerty wrote:

> > You can't symlink refs like this. The loose refs in the filesystem may
> > be migrated into the "packed-refs" file, at which point your symlink
> > will be broken. That is a likely reason why git would not find any refs.
> > 
> > So your setup will not ever work reliably.  But IMHO, it is a bug that
> > git does not notice the broken symlink and abort an operation which is
> > computing reachability in order to drop objects. As you noticed, it
> > means a misconfiguration or filesystem error results in data loss.
> 
> There's a bunch of code in refs.c that is there explicitly for reading
> loose references that are symlinks. If the link contents literally start
> with "refs/", then they are read and treated as a symbolic ref.
> Otherwise, the symlink is just followed.

Right, but we should be able to notice that:

  1. We found a symlink.

  2. We couldn't read it its ref value (because it's a broken link).

I think we _do_ notice that at the lowest level, and set REF_ISBROKEN.
But the problem is that the reachability code in prune and in
pack-objects (triggered by "repack -ad") uses for_each_ref, and not
for_each_rawref. So they ignore "broken" refs rather than complaining,
even though failing to read a ref may mean we could drop objects which
were only mentioned by that ref.

> It is still possible to write symbolic refs that are represented as
> symlinks (see core.preferSymlinkRefs), but that backwards-compatibility
> code was added in 2006(!) Maybe it's time to deprecate it. And maybe we
> should start working towards a future where any symlinks under "refs"
> cause git to complain.

I wouldn't mind seeing all of the symlink code go away, but I think it
is orthogonal to the problem I mentioned.

-Peff

  reply	other threads:[~2015-02-17 16:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-05 15:13 Git gc removes all packs Dmitry Neverov
2015-02-05 20:03 ` Jeff King
2015-02-17 16:39   ` Michael Haggerty
2015-02-17 16:55     ` Jeff King [this message]
2015-02-17 20:37       ` Michael Haggerty
2015-02-17 21:57         ` Junio C Hamano
2015-02-17 22:19           ` Michael Haggerty
2015-02-18  7:13             ` Junio C Hamano
2015-02-27 10:16   ` Dmitry Neverov
2015-02-27 13:14     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150217165514.GA12176@peff.net \
    --to=peff@peff.net \
    --cc=dmitry.neverov@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.