git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 00/19] Improve "refs" encapsulation and speed up deletes

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Jun 22, 2015, 14:02 UTC
Message-ID
<cover.1434980615.git.mhagger@alum.mit.edu>

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: Michael Haggerty
Message 1 of 24 in “Improve "refs" encapsulation and speed up deletes”
  1. 00/19 Improve "refs" encapsulation and speed up deletesMichael Haggerty, Jun 22, 2015
  2. 01/19 delete_ref(): move declaration to refs.hMichael Haggerty, Jun 22, 2015
  3. 02/19 remove_branches(): remove temporaryMichael Haggerty, Jun 22, 2015
  4. 03/19 delete_ref(): handle special case more explicitlyMichael Haggerty, Jun 22, 2015
  5. 04/19 delete_refs(): new function for the refs APIMichael Haggerty, Jun 22, 2015
  6. 05/19 delete_refs(): make error message more genericMichael Haggerty, Jun 22, 2015
  7. 06/19 delete_refs(): bail early if the packed-refs file cannot be rewrittenMichael Haggerty, Jun 22, 2015
  8. 07/19 prune_remote(): use delete_refs()Michael Haggerty, Jun 22, 2015
  9. 08/19 prune_refs(): use delete_refs()Michael Haggerty, Jun 22, 2015
  10. 09/19 repack_without_refs(): make function privateMichael Haggerty, Jun 22, 2015
  11. 10/19 initial_ref_transaction_commit(): function for initial ref creationMichael Haggerty, Jun 22, 2015
  12. 11/19 refs: remove some functions from the module's public interfaceMichael Haggerty, Jun 22, 2015
  13. 12/19 initial_ref_transaction_commit(): check for duplicate refsMichael Haggerty, Jun 22, 2015
  14. Junio C HamanoJun 22, 2015
  15. Michael HaggertyJun 23, 2015
  16. Junio C HamanoJun 23, 2015
  17. 13/19 initial_ref_transaction_commit(): check for ref D/F conflictsMichael Haggerty, Jun 22, 2015
  18. 14/19 refs: move the remaining ref module declarations to refs.hMichael Haggerty, Jun 22, 2015
  19. 15/19 refs.h: add some parameter names to function declarationsMichael Haggerty, Jun 22, 2015
  20. 16/19 check_branch_commit(): make first parameter constMichael Haggerty, Jun 22, 2015
  21. 17/19 update_ref(): don't read old reference value before deleteMichael Haggerty, Jun 22, 2015
  22. 18/19 cmd_update_ref(): make logic more straightforwardMichael Haggerty, Jun 22, 2015
  23. 19/19 delete_ref(): use the usual convention for old_sha1Michael Haggerty, Jun 22, 2015
  24. Junio C HamanoJun 22, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.