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

Re: [PATCHv3 00/13] the refs-transactions-reflog series

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Dec 4, 2014, 18:14 UTC
Message-ID
<20141204181404.GD9992@google.com>
In-Reply-To
<54809577.4080302@alum.mit.edu>
Michael Haggerty wrote:
Show 5 quoted lines
> I am still unhappy with the approach of this series, for the reasons
> that I explained earlier [1]. In short, I think that the abstraction
> level is wrong. In my opinion, consumers of the refs API should barely
> even have to *know* about reflogs, let alone implement reflog expiration
> themselves.

Okay, now returning to the substance of this comment. This is revisiting themes from [3], so my opinions are probably not a surprise.

I think that the API changes that you and Stefan are proposing are consistent and could both go in.

You suggested refactoring expire_reflogs() to use a callback that decides what to expire. Then it doesn't have to care that the expiration happens by creating a new reflog and copying over the reflog entries that are not being expired. The result is a clean reflog expiration API.

The ref-transaction-reflog series allows those low-level steps to be part of a ref transaction. Any ref backend (the current files-based backend or a future other one) would get a chance to reimplement those low-level steps, which are part of what happens during ref updates and reflog deletion. The goal is for all reflog updates to use the transaction API, so that new ref/reflog backends only need to implement the transaction functions.

So *both* are making good changes, with different goals.

The implementation of the reflog expiration API can use the ref transaction API.

> Of course, reflog expiration *should* be done atomically. But that is
> the business of the refs module; callers shouldn't have to do all the
> complicated work of building the transaction themselves.

I don't understand this comment. After the ref-transaction-reflog series, a transaction_update_ref() call still takes care of the corresponding reflog update without the caller having to worry about it.

Thanks for looking it over, Jonathan

> [1] http://thread.gmane.org/gmane.comp.version-control.git/259712/focus=259770
[3] http://thread.gmane.org/gmane.comp.version-control.git/259939/focus=259967
Previous: Jonathan NiederNext: Stefan Beller
Message 19 of 25 in “[PATCHv3 00/13] the refs-transactions-reflog series”
  1. Stefan BellerDec 4, 2014
  2. 01/13 refs.c: make ref_transaction_create a wrapper for ref_transaction_updateStefan Beller, Dec 4, 2014
  3. 02/13 refs.c: make ref_transaction_delete a wrapper for ref_transaction_updateStefan Beller, Dec 4, 2014
  4. 03/13 refs.c: add a function to append a reflog entry to a fdStefan Beller, Dec 4, 2014
  5. 04/13 refs.c: rename the transaction functionsStefan Beller, Dec 4, 2014
  6. 05/13 refs.c: rename transaction.updates to transaction.ref_updatesStefan Beller, Dec 4, 2014
  7. 06/13 refs.c: add a transaction function to truncate or append a reflog entryStefan Beller, Dec 4, 2014
  8. 07/13 reflog.c: use a reflog transaction when writing during expireStefan Beller, Dec 4, 2014
  9. 08/13 refs.c: rename log_ref_setup to create_reflogStefan Beller, Dec 4, 2014
  10. 09/13 refs.c: remove unlock_ref/close_ref/commit_ref from the refs apiStefan Beller, Dec 4, 2014
  11. 10/13 refs.c: remove lock_any_ref_for_updateStefan Beller, Dec 4, 2014
  12. 11/13 refs.c: don't expose the internal struct ref_lock in the header fileStefan Beller, Dec 4, 2014
  13. 12/13 refs.c: use a bit for ref_update have_oldStefan Beller, Dec 4, 2014
  14. Torsten BögershausenDec 4, 2014
  15. Andreas SchwabDec 4, 2014
  16. 13/13 refs.c: allow deleting refs with a broken sha1Stefan Beller, Dec 4, 2014
  17. Michael HaggertyDec 4, 2014
  18. Jonathan NiederDec 4, 2014
  19. Jonathan NiederDec 4, 2014
  20. Stefan BellerDec 4, 2014
  21. Michael HaggertyDec 4, 2014
  22. Junio C HamanoDec 4, 2014
  23. Junio C HamanoDec 4, 2014
  24. Stefan BellerDec 4, 2014
  25. Jonathan NiederDec 4, 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.