Re: [PATCH v6 00/39] Lockfile correctness and refactoring

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> Next iteration of my lockfile fixes and refactoring. Thanks to
> Jonathan Nieder and Torsten Bögershausen for their comments about v5.
>
> I believe that this series addresses all of the comments from v1 [1],
> v2 [2], v3 [3], v4 [4], and v5 [5].

Looked quite good.  Will replace---let's aim to merge this topic to
'next' soonish.

Thanks.

>
> Changes since v4:
>
> * Revise API documentation.
>
> * Split out a separate header file for the lockfile API: lockfile.h.
>
> * Change close_lock_file() to rollback on errors and make it into a
>   NOOP if the file is already closed.
>
> * Don't set lk->on_list until the lock_file object is completely on
>   the lock_file_list.
>
> * Delete some information from the docstring in lockfile.c (now
>   lockfile.h) that is redundant with the API docs in api-lockfile.txt.
>
> * Remove the accidental extra call of git_config_clear() in
>   git_config_set_multivar_in_file() when commit_lock_file() fails.
>
> * Adjust some comments, error messages, and commit messages.
>
> * Rebase to current master.
>
> [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
> [5] http://thread.gmane.org/gmane.comp.version-control.git/257159
>
> Michael Haggerty (39):
>   unable_to_lock_die(): rename function from unable_to_lock_index_die()
>   api-lockfile: revise and expand the documentation
>   close_lock_file(): exit (successfully) if file is already closed
>   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 initialize and register lock_file object
>   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
>   close_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
>   hold_lock_file_for_append(): restore errno before returning
>   Move read_index() definition to read-cache.c
>   lockfile.h: extract new header file for the functions in lockfile.c
>
>  Documentation/technical/api-lockfile.txt | 220 ++++++++++++++++++------
>  builtin/add.c                            |   1 +
>  builtin/apply.c                          |   1 +
>  builtin/checkout-index.c                 |   2 +-
>  builtin/checkout.c                       |   2 +-
>  builtin/clone.c                          |   1 +
>  builtin/commit.c                         |  17 +-
>  builtin/describe.c                       |   1 +
>  builtin/diff.c                           |   1 +
>  builtin/gc.c                             |   2 +-
>  builtin/merge.c                          |  16 +-
>  builtin/mv.c                             |   2 +-
>  builtin/read-tree.c                      |   1 +
>  builtin/receive-pack.c                   |   1 +
>  builtin/reflog.c                         |   4 +-
>  builtin/reset.c                          |   1 +
>  builtin/rm.c                             |   2 +-
>  builtin/update-index.c                   |   3 +-
>  bundle.c                                 |   1 +
>  cache-tree.c                             |   1 +
>  cache.h                                  |  20 +--
>  config.c                                 |  16 +-
>  credential-store.c                       |   1 +
>  fast-import.c                            |   5 +-
>  fetch-pack.c                             |   1 +
>  lockfile.c                               | 284 +++++++++++++++++--------------
>  lockfile.h                               |  84 +++++++++
>  merge-recursive.c                        |   1 +
>  merge.c                                  |   1 +
>  read-cache.c                             |  21 ++-
>  refs.c                                   |  30 ++--
>  rerere.c                                 |   1 +
>  sequencer.c                              |   1 +
>  sha1_file.c                              |   1 +
>  shallow.c                                |   7 +-
>  test-scrap-cache-tree.c                  |   1 +
>  36 files changed, 492 insertions(+), 263 deletions(-)
>  create mode 100644 lockfile.h