From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: Stefan Beller <sbeller@google.com>, Jeff King <peff@peff.net>,
git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH v3 00/19] Improve "refs" encapsulation and speed up deletes
Date: Mon, 22 Jun 2015 16:02:51 +0200 [thread overview]
Message-ID: <cover.1434980615.git.mhagger@alum.mit.edu> (raw)
This is v3 of the series. I think I have addressed all of the feedback
from v1 [1] and v2 [2]. Thanks to Stefan, Junio, and Peff for their
feedback about v2.
There are three significant changes since v2:
* Add a patch
delete_refs(): bail early if the packed-refs file cannot be rewritten
that changes delete_refs() to bail early if repack_without_refs()
fails. See the log message for the explanation.
* Add a patch
prune_refs(): use delete_refs()
that changes "git fetch --prune" to use delete_refs() as well. This
is analogous to the existing patch
prune_remote(): use delete_refs()
(Both of these changes remove quadratic behavior that can be
extremely expensive in repositories with many packed refs.)
* Append four more commits that change how delete_ref() interprets its
`old_sha1` parameter, to make it consistent with
ref_transaction_delete() and friends:
check_branch_commit(): make first parameter const
update_ref(): don't read old reference value before delete
cmd_update_ref(): make logic more straightforward
delete_ref(): use the usual convention for old_sha1
This change was suggested by Junio [3].
The last change is good for consistency, but less important than
expected in the real world. The reason is that the convention of
ref_transaction_delete() is that passing NULL_SHA1 as old_sha1 is a
bug (because why would somebody delete a reference that he knows not
to exist?) So we don't really gain a useful third case, though we do
gain a consistency check that might be useful someday. However, if you
don't like this change, the last four commits can be dropped
(actually, commits N-3 through N-1 are marginally useful cleanups
anyway so probably only the last commit should be dropped in this
case).
Other, minor changes since v2:
* Improve the commit message of "delete_refs(): make error message
more generic".
* Better explain the change to error output caused by "prune_remote():
use delete_refs()" in that commit's log message.
* Split the "add names for function parameters" changes into a
separate patch.
* Fix how die() is invoked in "initial_ref_transaction_commit():
function for initial ref creation" and remove some superfluous
braces.
This series is also available from my GitHub account [4] as branch
"init-delete-refs-api".
[1] http://thread.gmane.org/gmane.comp.version-control.git/271017
[2] http://thread.gmane.org/gmane.comp.version-control.git/271552
[3] http://article.gmane.org/gmane.comp.version-control.git/271697
[4] https://github.com/mhagger/git
Michael Haggerty (19):
delete_ref(): move declaration to refs.h
remove_branches(): remove temporary
delete_ref(): handle special case more explicitly
delete_refs(): new function for the refs API
delete_refs(): make error message more generic
delete_refs(): bail early if the packed-refs file cannot be rewritten
prune_remote(): use delete_refs()
prune_refs(): use delete_refs()
repack_without_refs(): make function private
initial_ref_transaction_commit(): function for initial ref creation
refs: remove some functions from the module's public interface
initial_ref_transaction_commit(): check for duplicate refs
initial_ref_transaction_commit(): check for ref D/F conflicts
refs: move the remaining ref module declarations to refs.h
refs.h: add some parameter names to function declarations
check_branch_commit(): make first parameter const
update_ref(): don't read old reference value before delete
cmd_update_ref(): make logic more straightforward
delete_ref(): use the usual convention for old_sha1
archive.c | 1 +
builtin/blame.c | 1 +
builtin/branch.c | 5 +-
builtin/clone.c | 18 ++++-
builtin/fast-export.c | 1 +
builtin/fetch.c | 25 ++++--
builtin/fmt-merge-msg.c | 1 +
builtin/init-db.c | 1 +
builtin/log.c | 1 +
builtin/remote.c | 33 +-------
builtin/update-ref.c | 21 ++++-
cache.h | 68 ----------------
fast-import.c | 6 +-
refs.c | 182 ++++++++++++++++++++++++++++++++++++++---
refs.h | 211 +++++++++++++++++++++++++++++++-----------------
remote-testsvn.c | 1 +
16 files changed, 371 insertions(+), 205 deletions(-)
--
2.1.4
next reply other threads:[~2015-06-22 14:03 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-22 14:02 Michael Haggerty [this message]
2015-06-22 14:02 ` [PATCH v3 01/19] delete_ref(): move declaration to refs.h Michael Haggerty
2015-06-22 14:02 ` [PATCH v3 02/19] remove_branches(): remove temporary Michael Haggerty
2015-06-22 14:02 ` [PATCH v3 03/19] delete_ref(): handle special case more explicitly Michael Haggerty
2015-06-22 14:02 ` [PATCH v3 04/19] delete_refs(): new function for the refs API Michael Haggerty
2015-06-22 14:02 ` [PATCH v3 05/19] delete_refs(): make error message more generic Michael Haggerty
2015-06-22 14:02 ` [PATCH v3 06/19] delete_refs(): bail early if the packed-refs file cannot be rewritten Michael Haggerty
2015-06-22 14:02 ` [PATCH v3 07/19] prune_remote(): use delete_refs() Michael Haggerty
2015-06-22 14:02 ` [PATCH v3 08/19] prune_refs(): " Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 09/19] repack_without_refs(): make function private Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 10/19] initial_ref_transaction_commit(): function for initial ref creation Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 11/19] refs: remove some functions from the module's public interface Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 12/19] initial_ref_transaction_commit(): check for duplicate refs Michael Haggerty
2015-06-22 21:06 ` Junio C Hamano
2015-06-23 7:11 ` Michael Haggerty
2015-06-23 17:44 ` Junio C Hamano
2015-06-22 14:03 ` [PATCH v3 13/19] initial_ref_transaction_commit(): check for ref D/F conflicts Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 14/19] refs: move the remaining ref module declarations to refs.h Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 15/19] refs.h: add some parameter names to function declarations Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 16/19] check_branch_commit(): make first parameter const Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 17/19] update_ref(): don't read old reference value before delete Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 18/19] cmd_update_ref(): make logic more straightforward Michael Haggerty
2015-06-22 14:03 ` [PATCH v3 19/19] delete_ref(): use the usual convention for old_sha1 Michael Haggerty
2015-06-22 21:10 ` Junio C Hamano
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=cover.1434980615.git.mhagger@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=sbeller@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).