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

Re: [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log

From
Ronnie Sahlberg <sahlberg@google.com>
Date
May 19, 2014, 23:27 UTC
Message-ID
<CAL=YDWn=t_+0Tt7--k1wAfUFXwTpsaEUDgYwdKbDPjmca5xdVA@mail.gmail.com>
In-Reply-To
<xmqqy4y18lol.fsf@gitster.dls.corp.google.com>
On Fri, May 16, 2014 at 2:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Ronnie Sahlberg <sahlberg@google.com> writes:
>
>> Add a flag that allows us to truncate the reflog before we write the update.
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
>> ---
>
> Until we read the callers it is hard to see how such a feature is
> useful, though.

Shortly later in the series comes the update to builtin/reflog.c which uses this feature. Even later in the series I also use this in builtin/reflog.c to both do the ref update as well as the corresponding reflog updates all in one single transaction.

>
> (style) The two multi-line comments are formatted differently.
Fixed. Thanks.
Show 36 quoted lines
>
>>  refs.c | 12 ++++++++++--
>>  refs.h |  4 +++-
>>  2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index d8a1568..a8b583a 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_transaction *transaction,
>>               }
>>       }
>>
>> -     /* Update all reflog files */
>> +     /* Update all reflog files
>> +        We have already done all ref updates and deletes.
>> +        There is not much we can do here if there are any reflog
>> +        update errors, other than complain.
>> +     */
>>       for (i = 0; i < n; i++) {
>>               struct ref_update *update = updates[i];
>>
>> @@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_transaction *transaction,
>>                       continue;
>>               if (update->reflog_fd == -1)
>>                       continue;
>> -
>> +             if (update->flags & REFLOG_TRUNCATE)
>> +                     if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||
>> +                         ftruncate(update->reflog_fd, 0)) {
>> +                             error("Could not truncate reflog");
>> +                     }
>
> The {} looks somewhat curious here.  If you seeked to the beginning,
> and failed to truncate, do you still want to go on without "return"
> in front of the error()?  Would that overwrite existing entries?
I have changed it to do this :
+               if (update->flags & REFLOG_TRUNCATE)
+                       if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||
+                               ftruncate(update->reflog_fd, 0)) {
+                               error("Could not truncate reflog: %s",
+                                     update->refname);
+                               rollback_lock_file(&update->reflog_lock);
+                               update->reflog_fd = -1;
+                       }

At this point we have already committed all the changes to refs and also performed all unlinks for refs we are deleting. So if things start to go wrong here during the reflog updates it is too late to do much more than complain.

Show 20 quoted lines
>
>>               if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
>>                                    update->new_sha1,
>>                                    update->committer, update->msg)) {
>> diff --git a/refs.h b/refs.h
>> index 2e22a26..b1b97e7 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction,
>>                           const unsigned char *old_sha1,
>>                           int flags, int have_old, const char *msg);
>>
>> +#define REFLOG_TRUNCATE 0x01
>>  /*
>> - * Append a reflog entry for refname.
>> + * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
>> + * this update will first truncate the reflog before writing the entry.
>>   */
>>  int transaction_update_reflog(struct ref_transaction *transaction,
>>                             const char *refname,
Previous: Junio C HamanoNext: Ronnie Sahlberg
Message 13 of 43 in “Finish implementing ref and reflog transactions”
  1. 00/31 Finish implementing ref and reflog transactionsRonnie Sahlberg, May 14, 2014
  2. 01/31 refs.c make ref_transaction_create a wrapper to ref_transaction_updateRonnie Sahlberg, May 14, 2014
  3. 02/31 refs.c: make ref_transaction_delete a wrapper for ref_transaction_updateRonnie Sahlberg, May 14, 2014
  4. 03/31 refs.c: rename the transaction functionsRonnie Sahlberg, May 14, 2014
  5. Junio C HamanoMay 16, 2014
  6. Ronnie SahlbergMay 19, 2014
  7. Junio C HamanoMay 19, 2014
  8. 04/31 refs.c: add a new update_type field to ref_updateRonnie Sahlberg, May 14, 2014
  9. 05/31 refs.c: add a function to append a reflog entry to a fdRonnie Sahlberg, May 14, 2014
  10. 06/31 refs.c: add a transaction function to append a reflog entryRonnie Sahlberg, May 14, 2014
  11. 07/31 refs.c: add a flag to allow reflog updates to truncate the logRonnie Sahlberg, May 14, 2014
  12. Junio C HamanoMay 16, 2014
  13. Ronnie SahlbergMay 19, 2014
  14. 08/31 refs.c: only write reflog update if msg is non-NULLRonnie Sahlberg, May 14, 2014
  15. Junio C HamanoMay 16, 2014
  16. Ronnie SahlbergMay 19, 2014
  17. 09/31 refs.c: allow multiple reflog updates during a single transactionRonnie Sahlberg, May 14, 2014
  18. Junio C HamanoMay 16, 2014
  19. Eric SunshineMay 16, 2014
  20. Ronnie SahlbergMay 19, 2014
  21. Ronnie SahlbergMay 19, 2014
  22. 10/31 reflog.c: use a reflog transaction when writing during expireRonnie Sahlberg, May 14, 2014
  23. 11/31 refs.c: null terminate the string in copy_msgRonnie Sahlberg, May 14, 2014
  24. 12/31 refs.c: track the refnames we are deleting in the transaction structureRonnie Sahlberg, May 14, 2014
  25. 13/31 refs.c: update the list of deleted refs during _update instead of _commitRonnie Sahlberg, May 14, 2014
  26. 14/31 refs.c: return error instead of dying when locking fails during transactionRonnie Sahlberg, May 14, 2014
  27. 15/31 refs.c: lock the ref during _update instead of during _commitRonnie Sahlberg, May 14, 2014
  28. 16/31 refs.c: add an error argument to create/delete/update just like commitRonnie Sahlberg, May 14, 2014
  29. 17/31 refs.c: make _update_reflog take an error argumentRonnie Sahlberg, May 14, 2014
  30. 18/31 refs.c: return immediately from _commit if the transaction has an errorRonnie Sahlberg, May 14, 2014
  31. 19/31 tests: move tests for -z update/delete/verify to after the ref is createdRonnie Sahlberg, May 14, 2014
  32. 20/31 refs.c: check for lock conflicts already in _updateRonnie Sahlberg, May 14, 2014
  33. 21/31 refs.c allow multiple updates of the same ref in a transactionRonnie Sahlberg, May 14, 2014
  34. 22/31 refs.c: release all remaining locks during transaction_freeRonnie Sahlberg, May 14, 2014
  35. 23/31 reflog.c: use the existing transaction to also lock and update the refRonnie Sahlberg, May 14, 2014
  36. 24/31 refs.c: make unlock_ref staticRonnie Sahlberg, May 14, 2014
  37. 25/31 refs.c: make close_ref staticRonnie Sahlberg, May 14, 2014
  38. 26/31 refs.c: make commit_ref staticRonnie Sahlberg, May 14, 2014
  39. 27/31 refs.c: remove the function lock_any_ref_for_updateRonnie Sahlberg, May 14, 2014
  40. 28/31 refs.c: make struct ref_lock private to refs.cRonnie Sahlberg, May 14, 2014
  41. 29/31 refs.c: allow passing raw git_committer_info as email to _update_reflogRonnie Sahlberg, May 14, 2014
  42. 30/31 refs.c: move ref_update and other definitions to earlier in the fileRonnie Sahlberg, May 14, 2014
  43. 31/31 refs.c: use the transaction to manage the reflog in rename_refsRonnie Sahlberg, May 14, 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.