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

Re: [PATCH 8/8] reflog_expire(): lock symbolic refs themselves, not their referent

From
Stefan Beller <sbeller@google.com>
Date
Feb 13, 2015, 17:16 UTC
Message-ID
<CAGZ79kbn=0VwZ6t3zE=BFGVeZqe5=H43gHLEzD2-OCar107abw@mail.gmail.com>
In-Reply-To
<54DE259C.4030001@alum.mit.edu>
On Fri, Feb 13, 2015 at 8:26 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
Show 14 quoted lines
>
> What is the best way forward?
>
> Switching to holding only "HEAD.lock" while updating "logs/HEAD" is the
> right solution, but it would introduce an incompatibility with old
> versions of Git and libgit2 (and maybe JGit?) Probably nobody would
> notice, but who knows?
>
> Therefore, I propose that we hold *both* "HEAD.lock" *and*
> "refs/heads/master.lock" when modifying "logs/HEAD" (even when running
> "reflog expire" or "reflog delete"). I think this will be compatible
> with old versions of Git and libgit2, and it will also fix some design
> problems mentioned above. Plus, it will prevent updates to the two
> reflogs from appearing out of order relative to each other.

Yeah, I agree on that. It's ok to lock both for now (it doesn't really harm the user). But we really need to document it in the source code why we lock both and that the 2nd lock can be removed after time has passed, something like:

/*
 * We need to lock both the symbolic ref as well as
 * the dereferenced ref for now because of inconsistencies with
 * older and other implementations of git. Originally only the
 * dereferenced ref lock was held, but discussion($gmane/263555)
 * turned out, we actually want to hold the lock on the symbolic ref.
 * For now hold both locks until all implementation hold the lock on
 * the symbolic ref. (Feb/2015)
 */
Show 13 quoted lines
>
> Someday, when old clients have been retired, we can optionally switch to
> locking *only* "HEAD.lock" when modifying "logs/HEAD".
>
> What do people think?
> Michael
>
> [1] https://github.com/libgit2/libgit2/issues/2902
>
> --
> Michael Haggerty
> mhagger@alum.mit.edu
>
Previous: Michael HaggertyNext: Junio C Hamano
Message 35 of 46 in “Fix some problems with reflog expiration”
  1. 0/8 Fix some problems with reflog expirationMichael Haggerty, Feb 9, 2015
  2. 1/8 write_ref_sha1(): remove check for lock == NULLMichael Haggerty, Feb 9, 2015
  3. Stefan BellerFeb 10, 2015
  4. Jeff KingFeb 11, 2015
  5. 2/8 write_ref_sha1(): Move write elision test to callersMichael Haggerty, Feb 9, 2015
  6. Junio C HamanoFeb 12, 2015
  7. 3/8 lock_ref_sha1_basic(): do not set force_write for missing referencesMichael Haggerty, Feb 9, 2015
  8. Stefan BellerFeb 10, 2015
  9. Jeff KingFeb 11, 2015
  10. Stefan BellerFeb 11, 2015
  11. Michael HaggertyFeb 12, 2015
  12. 4/8 reflog: fix documentationMichael Haggerty, Feb 9, 2015
  13. Stefan BellerFeb 10, 2015
  14. 5/8 reflog: rearrange the manpageMichael Haggerty, Feb 9, 2015
  15. Stefan BellerFeb 10, 2015
  16. Michael HaggertyFeb 12, 2015
  17. Junio C HamanoFeb 12, 2015
  18. 6/8 reflog_expire(): ignore --updateref for symbolic referencesMichael Haggerty, Feb 9, 2015
  19. Stefan BellerFeb 11, 2015
  20. Michael HaggertyFeb 12, 2015
  21. Stefan BellerFeb 12, 2015
  22. Junio C HamanoFeb 12, 2015
  23. Jeff KingFeb 12, 2015
  24. Michael HaggertyFeb 13, 2015
  25. 7/8 reflog_expire(): never update a reference to null_sha1Michael Haggerty, Feb 9, 2015
  26. Eric SunshineFeb 9, 2015
  27. Michael HaggertyFeb 12, 2015
  28. 8/8 reflog_expire(): lock symbolic refs themselves, not their referentMichael Haggerty, Feb 9, 2015
  29. Stefan BellerFeb 11, 2015
  30. Junio C HamanoFeb 11, 2015
  31. Stefan BellerFeb 11, 2015
  32. Michael HaggertyFeb 12, 2015
  33. Stefan BellerFeb 12, 2015
  34. Michael HaggertyFeb 13, 2015
  35. Stefan BellerFeb 13, 2015
  36. Junio C HamanoFeb 13, 2015
  37. Stefan BellerFeb 13, 2015
  38. Junio C HamanoFeb 13, 2015
  39. Stefan BellerFeb 13, 2015
  40. Junio C HamanoFeb 13, 2015
  41. Michael HaggertyFeb 13, 2015
  42. Junio C HamanoFeb 13, 2015
  43. Michael HaggertyFeb 14, 2015
  44. Stefan BellerFeb 9, 2015
  45. refs.c: get rid of force_write flagStefan Beller, Feb 10, 2015
  46. Michael HaggertyFeb 12, 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.