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

Re: Transaction patch series overview

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 26, 2014, 21:01 UTC
Message-ID
<xmqqlhqbge3a.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20140826000354.GW20185@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 34 quoted lines
>>  https://code-review.googlesource.com/#/q/topic:ref-transaction-1
>
> Outcome of the experiment: patches gained some minor changes and then
>
>  1-12
> 	Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
>  13
> 	Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
> 	Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
>  14
> 	Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
>  15-16
> 	Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
> 	Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
>  17
> 	Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
>
>  18
> 	Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
> 	Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
>  19
> 	Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
>  20
> 	Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
> 	Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> I found the web UI helpful in seeing how each patch evolved since I
> last looked at it.  Interdiff relative to the version in pu is below.

Thanks for the interdiff, it really helps to be able to take a glance without having to click around.

It seems that I can hold the topic in 'pu' a bit longer and expect a reroll from this effort before merging it to 'next'?

> The next series from Ronnie's collection is available at
> https://code-review.googlesource.com/#/q/topic:ref-transaction in case
> someone wants a fresh series to look at.
Thanks.
Show 15 quoted lines
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 668fa6a..9bf1003 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1765,8 +1765,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	transaction = ref_transaction_begin(&err);
>  	if (!transaction ||
>  	    ref_transaction_update(transaction, "HEAD", sha1,
> -				   current_head ?
> -				   current_head->object.sha1 : NULL,
> +				   current_head
> +				   ? current_head->object.sha1 : NULL,
>  				   0, !!current_head, &err) ||
>  	    ref_transaction_commit(transaction, sb.buf, &err)) {
>  		rollback_index_files();
Perhaps this is nicer, but probably most people would not care.
Show 25 quoted lines
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 91099ad..224fadc 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -580,18 +580,18 @@ static char *update(struct command *cmd, struct shallow_info *si)
>  
>  		if (shallow_update && si->shallow_ref[cmd->index] &&
>  		    update_shallow_ref(cmd, si))
> -			return xstrdup("shallow error");
> +			return "shallow error";
>  
>  		transaction = ref_transaction_begin(&err);
>  		if (!transaction ||
>  		    ref_transaction_update(transaction, namespaced_name,
>  					   new_sha1, old_sha1, 0, 1, &err) ||
>  		    ref_transaction_commit(transaction, "push", &err)) {
> -			char *str = strbuf_detach(&err, NULL);
>  			ref_transaction_free(transaction);
>  
> -			rp_error("%s", str);
> -			return str;
> +			rp_error("%s", err.buf);
> +			strbuf_release(&err);
> +			return "failed to update ref";
>  		}

I am guessing that the purpose of this change is to make sure that cmd->error_string is taken from a fixed set of strings, not an arbitrary collection of errors from the transaction subsystem, but what is the significance of doing so? Do the other side i18n while running print_ref_status() or something?

Show 14 quoted lines
> diff --git a/refs.h b/refs.h
> index aad846c..69ef28c 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -180,8 +180,7 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
>   */
>  #define REF_NODEREF	0x01
>  /*
> - * Locks any ref (for 'HEAD' type refs) and sets errno to something
> - * meaningful on failure.
> + * This function sets errno to something meaningful on failure.
>   */
>  extern struct ref_lock *lock_any_ref_for_update(const char *refname,
>  						const unsigned char *old_sha1,

To contrast this function with lock_ref_sha1() that only allowed refs under refs/ hierarchy, the comment may have made sense, but now that other function is gone, losing the comment makes sense.

I removed from the above the interdiff hunks I did not have any comments or questions on.

Thanks again.
Previous: Jonathan NiederNext: Jonathan Nieder
Message 8 of 129 in “Transaction patch series overview”
  1. Ronnie SahlbergJul 30, 2014
  2. Ronnie SahlbergJul 31, 2014
  3. Ronnie SahlbergAug 8, 2014
  4. Ronnie SahlbergAug 19, 2014
  5. Junio C HamanoAug 19, 2014
  6. Jonathan NiederAug 20, 2014
  7. Jonathan NiederAug 26, 2014
  8. Junio C HamanoAug 26, 2014
  9. Jonathan NiederAug 26, 2014
  10. 0/20 rs/ref-transaction-1 (Re: Transaction patch series overview)Jonathan Nieder, Aug 27, 2014
  11. 01/20 refs.c: change ref_transaction_create to do error checking and return statusJonathan Nieder, Aug 27, 2014
  12. 02/20 refs.c: update ref_transaction_delete to check for error and return statusJonathan Nieder, Aug 27, 2014
  13. 03/20 refs.c: make ref_transaction_begin take an err argumentJonathan Nieder, Aug 27, 2014
  14. 04/20 refs.c: add transaction.status and track OPEN/CLOSEDJonathan Nieder, Aug 27, 2014
  15. 05/20 tag.c: use ref transactions when doing updatesJonathan Nieder, Aug 27, 2014
  16. 06/20 replace.c: use the ref transaction functions for updatesJonathan Nieder, Aug 27, 2014
  17. 07/20 commit.c: use ref transactions for updatesJonathan Nieder, Aug 27, 2014
  18. 08/20 sequencer.c: use ref transactions for all ref updatesJonathan Nieder, Aug 27, 2014
  19. 09/20 fast-import.c: change update_branch to use ref transactionsJonathan Nieder, Aug 27, 2014
  20. 10/20 branch.c: use ref transaction for all ref updatesJonathan Nieder, Aug 27, 2014
  21. 11/20 refs.c: change update_ref to use a transactionJonathan Nieder, Aug 27, 2014
  22. 12/20 receive-pack.c: use a reference transaction for updating the refsJonathan Nieder, Aug 27, 2014
  23. 13/20 fast-import.c: use a ref transaction when dumping tagsJonathan Nieder, Aug 27, 2014
  24. 14/20 walker.c: use ref transaction for ref updatesJonathan Nieder, Aug 27, 2014
  25. 15/20 refs.c: make lock_ref_sha1 staticJonathan Nieder, Aug 27, 2014
  26. 16/20 refs.c: remove the update_ref_lock functionJonathan Nieder, Aug 27, 2014
  27. 17/20 refs.c: remove the update_ref_write functionJonathan Nieder, Aug 27, 2014
  28. 18/20 refs.c: remove lock_ref_sha1Jonathan Nieder, Aug 27, 2014
  29. 19/20 refs.c: make prune_ref use a transaction to delete the refJonathan Nieder, Aug 27, 2014
  30. 20/20 refs.c: make delete_ref use a transactionJonathan Nieder, Aug 27, 2014
  31. Junio C HamanoAug 27, 2014
  32. Junio C HamanoAug 27, 2014
  33. 0/22 rs/ref-transaction-1 (Re: Transaction patch series overview)Jonathan Nieder, Sep 2, 2014
  34. 01/22 refs.c: change ref_transaction_create to do error checking and return statusJonathan Nieder, Sep 2, 2014
  35. 02/22 refs.c: update ref_transaction_delete to check for error and return statusJonathan Nieder, Sep 2, 2014
  36. 03/22 refs.c: make ref_transaction_begin take an err argumentJonathan Nieder, Sep 2, 2014
  37. 04/22 refs.c: add transaction.status and track OPEN/CLOSEDJonathan Nieder, Sep 2, 2014
  38. 05/22 tag.c: use ref transactions when doing updatesJonathan Nieder, Sep 2, 2014
  39. 06/22 replace.c: use the ref transaction functions for updatesJonathan Nieder, Sep 2, 2014
  40. 07/22 commit.c: use ref transactions for updatesJonathan Nieder, Sep 2, 2014
  41. 08/22 sequencer.c: use ref transactions for all ref updatesJonathan Nieder, Sep 2, 2014
  42. 09/22 fast-import.c: change update_branch to use ref transactionsJonathan Nieder, Sep 2, 2014
  43. 10/22 branch.c: use ref transaction for all ref updatesJonathan Nieder, Sep 2, 2014
  44. 11/22 refs.c: change update_ref to use a transactionJonathan Nieder, Sep 2, 2014
  45. 12/22 receive-pack.c: use a reference transaction for updating the refsJonathan Nieder, Sep 2, 2014
  46. 13/22 fast-import.c: use a ref transaction when dumping tagsJonathan Nieder, Sep 2, 2014
  47. 14/22 walker.c: use ref transaction for ref updatesJonathan Nieder, Sep 2, 2014
  48. 15/22 refs.c: make lock_ref_sha1 staticJonathan Nieder, Sep 2, 2014
  49. 16/22 refs.c: remove the update_ref_lock functionJonathan Nieder, Sep 2, 2014
  50. 17/22 refs.c: remove the update_ref_write functionJonathan Nieder, Sep 2, 2014
  51. 18/22 refs.c: remove lock_ref_sha1Jonathan Nieder, Sep 2, 2014
  52. 19/22 refs.c: make prune_ref use a transaction to delete the refJonathan Nieder, Sep 2, 2014
  53. 20/22 refs.c: make delete_ref use a transactionJonathan Nieder, Sep 2, 2014
  54. 21/22 update-ref --stdin: narrow scope of err strbufJonathan Nieder, Sep 2, 2014
  55. 22/22 update-ref --stdin: pass transaction around explicitlyJonathan Nieder, Sep 2, 2014
  56. Using Gerrit to review Git patches (was: Re: Transaction patch series overview)Michael Haggerty, Aug 27, 2014
  57. Shawn PearceAug 28, 2014
  58. 0/19 rs/ref-transaction (Re: Transaction patch series overview)Jonathan Nieder, Sep 11, 2014
  59. 01/19 mv test: recreate mod/ directory instead of relying on stale copyJonathan Nieder, Sep 11, 2014
  60. 02/19 wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as successJonathan Nieder, Sep 11, 2014
  61. 03/19 wrapper.c: add a new function unlink_or_msgJonathan Nieder, Sep 11, 2014
  62. 04/19 refs.c: add an err argument to delete_ref_looseJonathan Nieder, Sep 11, 2014
  63. 05/19 refs.c: pass the ref log message to _create/delete/update instead of _commitJonathan Nieder, Sep 11, 2014
  64. 06/19 rename_ref: don't ask read_ref_full where the ref came fromJonathan Nieder, Sep 11, 2014
  65. 07/19 refs.c: move the check for valid refname to lock_ref_sha1_basicJonathan Nieder, Sep 11, 2014
  66. 08/19 refs.c: call lock_ref_sha1_basic directly from commitJonathan Nieder, Sep 11, 2014
  67. 09/19 refs.c: pass a skip list to name_conflict_fnJonathan Nieder, Sep 11, 2014
  68. 10/19 refs.c: ref_transaction_commit: distinguish name conflicts from other errorsJonathan Nieder, Sep 11, 2014
  69. 11/19 fetch.c: change s_update_ref to use a ref transactionJonathan Nieder, Sep 11, 2014
  70. 12/19 refs.c: make write_ref_sha1 staticJonathan Nieder, Sep 11, 2014
  71. 13/19 refs.c: change resolve_ref_unsafe reading argument to be a flags fieldJonathan Nieder, Sep 11, 2014
  72. 14/19 branch -d: avoid repeated symref resolutionJonathan Nieder, Sep 11, 2014
  73. 15/19 refs.c: fix handling of badly named refsJonathan Nieder, Sep 11, 2014
  74. 16/19 for-each-ref.c: improve message before aborting on broken refJonathan Nieder, Sep 11, 2014
  75. 17/19 refs.c: do not permit err == NULLJonathan Nieder, Sep 11, 2014
  76. 18/19 lockfile: remove unable_to_lock_errorJonathan Nieder, Sep 11, 2014
  77. 19/19 ref_transaction_commit: bail out on failure to remove a refJonathan Nieder, Sep 11, 2014
  78. Junio C HamanoSep 11, 2014
  79. Junio C HamanoSep 11, 2014
  80. Jonathan NiederSep 12, 2014
  81. Junio C HamanoSep 12, 2014
  82. Jonathan NiederSep 12, 2014
  83. Junio C HamanoSep 12, 2014
  84. Junio C HamanoSep 12, 2014
  85. Junio C HamanoSep 13, 2014
  86. Michael HaggertySep 12, 2014
  87. Jonathan NiederSep 12, 2014
  88. Michael HaggertySep 17, 2014
  89. Junio C HamanoSep 18, 2014
  90. Jonathan NiederSep 18, 2014
  91. Junio C HamanoSep 18, 2014
  92. Jonathan NiederSep 18, 2014
  93. Junio C HamanoSep 25, 2014
  94. Jonathan NiederSep 25, 2014
  95. Junio C HamanoSep 25, 2014
  96. 0/24 rs/ref-transactionJonathan Nieder, Oct 2, 2014
  97. 01/24 mv test: recreate mod/ directory instead of relying on stale copyJonathan Nieder, Oct 2, 2014
  98. 02/24 wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as successJonathan Nieder, Oct 2, 2014
  99. 03/24 wrapper.c: add a new function unlink_or_msgJonathan Nieder, Oct 2, 2014
  100. 04/24 refs.c: add an err argument to delete_ref_looseJonathan Nieder, Oct 2, 2014
  101. 05/24 refs.c: pass the ref log message to _create/delete/update instead of _commitJonathan Nieder, Oct 2, 2014
  102. 06/24 rename_ref: don't ask read_ref_full where the ref came fromJonathan Nieder, Oct 2, 2014
  103. 07/24 refs.c: refuse to lock badly named refs in lock_ref_sha1_basicJonathan Nieder, Oct 2, 2014
  104. 08/24 refs.c: call lock_ref_sha1_basic directly from commitJonathan Nieder, Oct 2, 2014
  105. 09/24 refs.c: pass a list of names to skip to is_refname_availableJonathan Nieder, Oct 2, 2014
  106. Junio C HamanoOct 2, 2014
  107. Jonathan NiederOct 3, 2014
  108. Junio C HamanoOct 3, 2014
  109. 09/24 refs.c: pass a list of names to skip to is_refname_availableJonathan Nieder, Oct 3, 2014
  110. Junio C HamanoOct 7, 2014
  111. 10/24 refs.c: ref_transaction_commit: distinguish name conflicts from other errorsJonathan Nieder, Oct 2, 2014
  112. 11/24 fetch.c: change s_update_ref to use a ref transactionJonathan Nieder, Oct 2, 2014
  113. 12/24 refs.c: make write_ref_sha1 staticJonathan Nieder, Oct 2, 2014
  114. 13/24 refs.c: change resolve_ref_unsafe reading argument to be a flags fieldJonathan Nieder, Oct 2, 2014
  115. 14/24 reflog test: test interaction with detached HEADJonathan Nieder, Oct 2, 2014
  116. 15/24 branch -d: avoid repeated symref resolutionJonathan Nieder, Oct 2, 2014
  117. 16/24 branch -d: simplify by using RESOLVE_REF_READING flagJonathan Nieder, Oct 2, 2014
  118. 17/24 packed-ref cache: forbid dot-components in refnamesJonathan Nieder, Oct 2, 2014
  119. 18/24 test: put tests for handling of bad ref names in one placeJonathan Nieder, Oct 2, 2014
  120. 19/24 refs.c: allow listing and deleting badly named refsJonathan Nieder, Oct 2, 2014
  121. Junio C HamanoOct 2, 2014
  122. Ronnie SahlbergOct 3, 2014
  123. Ronnie SahlbergOct 3, 2014
  124. Junio C HamanoOct 3, 2014
  125. 20/24 for-each-ref.c: improve message before aborting on broken refJonathan Nieder, Oct 2, 2014
  126. 21/24 remote rm/prune: print a message when writing packed-refs failsJonathan Nieder, Oct 2, 2014
  127. 22/24 refs.c: do not permit err == NULLJonathan Nieder, Oct 2, 2014
  128. 23/24 lockfile: remove unable_to_lock_errorJonathan Nieder, Oct 2, 2014
  129. 24/24 ref_transaction_commit: bail out on failure to remove a refJonathan Nieder, Oct 2, 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.