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

[PATCH v5 00/35] Lockfile correctness and refactoring

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Sep 16, 2014, 19:33 UTC
Message-ID
<1410896036-12750-1-git-send-email-mhagger@alum.mit.edu>

Next iteration of my lockfile fixes and refactoring. Thanks to Torsten Bögershausen, Junio, Peff, Ronnie Sahlberg, and Johannes Sixt for their comments about v4.

I believe that this series addresses all of the comments from v1 [1], v2 [2], v3 [3], and v4 [4].

Changes since v4:
* Rebase to current master.
* Explain lock_file ownership and the point of its pid field.
* Correct the log message for
    "delete_ref_loose(): don't muck around in the lock_file's filename"
* Replace an assert() with a die("BUG:").
* Add a sanity check that lk->filename is empty before reusing a
  lock_file object.
* Initialize the length of lk->filename more intelligently.
* Rename trim_last_path_elm() to trim_last_path_component().
* Make some die() messages more informative.
* Add some sanity checks to commit_lock_file().
* Rename REF_NODEREF to REF_NO_DEREF.
* Rename some static functions:
  * remove_lock_file() -> remove_lock_files()
  * remove_lock_file_on_signal() -> remove_lock_files_on_signal()
* Add a function get_locked_file_path(), to isolate callers a bit more
  from the innards of lock_file.

There are some conflicts with branch rs/ref-transaction; I pushed my proposed merge of these two branches to

    https://github.com/mhagger/git.git lock-correctness-v5-rs-ref-transaction

[1] http://thread.gmane.org/gmane.comp.version-control.git/245609 [2] http://thread.gmane.org/gmane.comp.version-control.git/245801 [3] http://thread.gmane.org/gmane.comp.version-control.git/246222 [4] http://thread.gmane.org/gmane.comp.version-control.git/256564

Michael Haggerty (35):
  unable_to_lock_die(): rename function from unable_to_lock_index_die()
  api-lockfile: expand the documentation
  rollback_lock_file(): do not clear filename redundantly
  rollback_lock_file(): exit early if lock is not active
  rollback_lock_file(): set fd to -1
  lockfile: unlock file if lockfile permissions cannot be adjusted
  hold_lock_file_for_append(): release lock on errors
  lock_file(): always add lock_file object to lock_file_list
  lockfile.c: document the various states of lock_file objects
  cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN
  delete_ref_loose(): don't muck around in the lock_file's filename
  prepare_index(): declare return value to be (const char *)
  write_packed_entry_fn(): convert cb_data into a (const int *)
  lock_file(): exit early if lockfile cannot be opened
  remove_lock_file(): call rollback_lock_file()
  commit_lock_file(): inline temporary variable
  commit_lock_file(): die() if called for unlocked lockfile object
  commit_lock_file(): if close fails, roll back
  commit_lock_file(): rollback lock file on failure to rename
  api-lockfile: document edge cases
  dump_marks(): remove a redundant call to rollback_lock_file()
  git_config_set_multivar_in_file(): avoid call to rollback_lock_file()
  lockfile: avoid transitory invalid states
  struct lock_file: declare some fields volatile
  try_merge_strategy(): remove redundant lock_file allocation
  try_merge_strategy(): use a statically-allocated lock_file object
  commit_lock_file(): use a strbuf to manage temporary space
  Change lock_file::filename into a strbuf
  resolve_symlink(): use a strbuf for internal scratch space
  resolve_symlink(): take a strbuf parameter
  trim_last_path_component(): replace last_path_elm()
  Extract a function commit_lock_file_to()
  Rename LOCK_NODEREF to LOCK_NO_DEREF
  lockfile.c: rename static functions
  get_locked_file_path(): new function
 Documentation/technical/api-lockfile.txt |  72 +++++--
 builtin/commit.c                         |  16 +-
 builtin/merge.c                          |  15 +-
 builtin/reflog.c                         |   2 +-
 builtin/update-index.c                   |   2 +-
 cache.h                                  |  19 +-
 config.c                                 |  28 +--
 fast-import.c                            |   4 +-
 lockfile.c                               | 334 +++++++++++++++++++------------
 read-cache.c                             |  12 +-
 refs.c                                   |  29 +--
 shallow.c                                |   6 +-
 12 files changed, 334 insertions(+), 205 deletions(-)
-- 
2.1.0
Next: Michael Haggerty
Message 1 of 73 in “Lockfile correctness and refactoring”
  1. 00/35 Lockfile correctness and refactoringMichael Haggerty, Sep 16, 2014
  2. 01/35 unable_to_lock_die(): rename function from unable_to_lock_index_die()Michael Haggerty, Sep 16, 2014
  3. Jonathan NiederSep 16, 2014
  4. 02/35 api-lockfile: expand the documentationMichael Haggerty, Sep 16, 2014
  5. Jonathan NiederSep 16, 2014
  6. Michael HaggertySep 22, 2014
  7. 03/35 rollback_lock_file(): do not clear filename redundantlyMichael Haggerty, Sep 16, 2014
  8. Jonathan NiederSep 16, 2014
  9. 04/35 rollback_lock_file(): exit early if lock is not activeMichael Haggerty, Sep 16, 2014
  10. Jonathan NiederSep 16, 2014
  11. 05/35 rollback_lock_file(): set fd to -1Michael Haggerty, Sep 16, 2014
  12. Jonathan NiederSep 16, 2014
  13. Jonathan NiederSep 16, 2014
  14. Michael HaggertySep 17, 2014
  15. 06/35 lockfile: unlock file if lockfile permissions cannot be adjustedMichael Haggerty, Sep 16, 2014
  16. Jonathan NiederSep 16, 2014
  17. 07/35 hold_lock_file_for_append(): release lock on errorsMichael Haggerty, Sep 16, 2014
  18. Jonathan NiederSep 16, 2014
  19. Michael HaggertySep 17, 2014
  20. 08/35 lock_file(): always add lock_file object to lock_file_listMichael Haggerty, Sep 16, 2014
  21. Jonathan NiederSep 16, 2014
  22. Michael HaggertySep 17, 2014
  23. Torsten BögershausenSep 18, 2014
  24. Michael HaggertySep 18, 2014
  25. 09/35 lockfile.c: document the various states of lock_file objectsMichael Haggerty, Sep 16, 2014
  26. Jonathan NiederSep 16, 2014
  27. Michael HaggertySep 22, 2014
  28. Jonathan NiederSep 22, 2014
  29. 10/35 cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LENMichael Haggerty, Sep 16, 2014
  30. Jonathan NiederSep 16, 2014
  31. Michael HaggertySep 22, 2014
  32. 11/35 delete_ref_loose(): don't muck around in the lock_file's filenameMichael Haggerty, Sep 16, 2014
  33. Jonathan NiederSep 16, 2014
  34. 12/35 prepare_index(): declare return value to be (const char *)Michael Haggerty, Sep 16, 2014
  35. Jonathan NiederSep 16, 2014
  36. 13/35 write_packed_entry_fn(): convert cb_data into a (const int *)Michael Haggerty, Sep 16, 2014
  37. 14/35 lock_file(): exit early if lockfile cannot be openedMichael Haggerty, Sep 16, 2014
  38. Jonathan NiederSep 16, 2014
  39. 15/35 remove_lock_file(): call rollback_lock_file()Michael Haggerty, Sep 16, 2014
  40. Jonathan NiederSep 16, 2014
  41. 16/35 commit_lock_file(): inline temporary variableMichael Haggerty, Sep 16, 2014
  42. Jonathan NiederSep 16, 2014
  43. 17/35 commit_lock_file(): die() if called for unlocked lockfile objectMichael Haggerty, Sep 16, 2014
  44. Jonathan NiederSep 16, 2014
  45. 18/35 commit_lock_file(): if close fails, roll backMichael Haggerty, Sep 16, 2014
  46. Jonathan NiederSep 16, 2014
  47. Michael HaggertySep 23, 2014
  48. 19/35 commit_lock_file(): rollback lock file on failure to renameMichael Haggerty, Sep 16, 2014
  49. Jonathan NiederSep 16, 2014
  50. Michael HaggertySep 23, 2014
  51. 20/35 api-lockfile: document edge casesMichael Haggerty, Sep 16, 2014
  52. Jonathan NiederSep 16, 2014
  53. Michael HaggertySep 23, 2014
  54. 21/35 dump_marks(): remove a redundant call to rollback_lock_file()Michael Haggerty, Sep 16, 2014
  55. Jonathan NiederSep 16, 2014
  56. 22/35 git_config_set_multivar_in_file(): avoid call to rollback_lock_file()Michael Haggerty, Sep 16, 2014
  57. Jonathan NiederSep 16, 2014
  58. Michael HaggertySep 23, 2014
  59. 23/35 lockfile: avoid transitory invalid statesMichael Haggerty, Sep 16, 2014
  60. Jonathan NiederSep 16, 2014
  61. Michael HaggertySep 23, 2014
  62. 24/35 struct lock_file: declare some fields volatileMichael Haggerty, Sep 16, 2014
  63. 25/35 try_merge_strategy(): remove redundant lock_file allocationMichael Haggerty, Sep 16, 2014
  64. 26/35 try_merge_strategy(): use a statically-allocated lock_file objectMichael Haggerty, Sep 16, 2014
  65. 27/35 commit_lock_file(): use a strbuf to manage temporary spaceMichael Haggerty, Sep 16, 2014
  66. 28/35 Change lock_file::filename into a strbufMichael Haggerty, Sep 16, 2014
  67. 29/35 resolve_symlink(): use a strbuf for internal scratch spaceMichael Haggerty, Sep 16, 2014
  68. 30/35 resolve_symlink(): take a strbuf parameterMichael Haggerty, Sep 16, 2014
  69. 31/35 trim_last_path_component(): replace last_path_elm()Michael Haggerty, Sep 16, 2014
  70. 32/35 Extract a function commit_lock_file_to()Michael Haggerty, Sep 16, 2014
  71. 33/35 Rename LOCK_NODEREF to LOCK_NO_DEREFMichael Haggerty, Sep 16, 2014
  72. 34/35 lockfile.c: rename static functionsMichael Haggerty, Sep 16, 2014
  73. 35/35 get_locked_file_path(): new functionMichael Haggerty, Sep 16, 2014

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.