From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Ronnie Sahlberg <sahlberg@google.com>,
Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 0/5] fix pack-refs pruning of refs/foo
Date: Sat, 23 Aug 2014 01:23:34 -0400 [thread overview]
Message-ID: <20140823052334.GA17813@peff.net> (raw)
I noticed that "git pack-refs --all" will pack a top-level ref like
"refs/foo", but will not actually prune "$GIT_DIR/refs/foo". I do not
see the point in having a policy not to pack "refs/foo" if "--all" is
given. But even if we did have such a policy, this seems broken; we
should either pack and prune, or leave them untouched. I don't see any
indication that the existing behavior was intentional.
The problem is that pack-refs's prune_ref calls lock_ref_sha1, which
enforces this "no toplevel" behavior. I am not sure there is any real
point to this, given that most callers use lock_any_ref_for_update,
which is exactly equivalent but without the toplevel check.
The first two patches deal with this by switching pack-refs to use
lock_any_ref_for_update. This will conflict slightly with Ronnie's
ref-transaction work, as he gets rid of lock_ref_sha1 entirely, and
moves the code directly into prune_ref. This can be trivially resolved
in favor of my patch, I think.
The third patch is a cleanup I noticed while looking around, and I think
should not conflict with anyone (and is a good thing to do).
The last two are trickier. I wondered if we could get rid of
lock_ref_sha1 entirely. After pack-refs, there are two callers:
fast-import.c and walker.c. After converting the first, it occurred to
me that Ronnie might be touching the same areas, and I see that yes,
indeed, there's quite a bit of conflict (and he reaches the same end
goal of dropping it entirely).
So in that sense I do not mind dropping the final two patches. Ronnie's
endpoint is much better, moving to a ref_transaction. However, there is
actually a buffer overflow in the existing code. Ronnie's series fixes
it in a similar way (moving to a strbuf), and I'm fine with that
endpoint. But given that the ref transaction code is not yet merged (and
would certainly never be maint-track), I think it is worth applying the
buffer overflow fix separately.
I think the final patch can probably be dropped, then. It is a clean-up,
but one that we can just get when Ronnie's series is merged.
[1/5]: git-prompt: do not look for refs/stash in $GIT_DIR
[2/5]: pack-refs: prune top-level refs like "refs/foo"
[3/5]: fast-import: clean up pack_data pointer in end_packfile
[4/5]: fast-import: fix buffer overflow in dump_tags
[5/5]: fast-import: stop using lock_ref_sha1
-Peff
next reply other threads:[~2014-08-23 5:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-23 5:23 Jeff King [this message]
2014-08-23 5:26 ` [PATCH 1/5] git-prompt: do not look for refs/stash in $GIT_DIR Jeff King
2014-08-23 5:27 ` [PATCH 2/5] pack-refs: prune top-level refs like "refs/foo" Jeff King
2014-08-23 5:27 ` [PATCH 3/5] fast-import: clean up pack_data pointer in end_packfile Jeff King
2014-08-25 17:16 ` Ronnie Sahlberg
2014-08-25 18:55 ` Jeff King
2014-08-23 5:32 ` [PATCH 4/5] fast-import: fix buffer overflow in dump_tags Jeff King
2014-08-25 17:11 ` Ronnie Sahlberg
2014-08-23 5:33 ` [PATCH 5/5] fast-import: stop using lock_ref_sha1 Jeff King
2014-08-25 17:22 ` Ronnie Sahlberg
2014-08-23 7:29 ` [PATCH 0/5] fix pack-refs pruning of refs/foo Michael Haggerty
2014-08-23 7:46 ` Jeff King
2014-08-25 17:38 ` Ronnie Sahlberg
2014-08-25 18:56 ` 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=20140823052334.GA17813@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=mhagger@alum.mit.edu \
--cc=sahlberg@google.com \
/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.