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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).