Hi again, Junio C Hamano wrote: > 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'? Sorry for the delay. The following changes on top of "master" (refs.c: change ref_transaction_update() to do error checking and return status, 2014-07-14) are available in the git repository at git://repo.or.cz/git/jrn.git tags/rs/ref-transaction-1 for you to fetch changes up to 432ad1f39fd773b37757dd8f10b3075dc3d0d0a2: refs.c: make delete_ref use a transaction (2014-08-26 14:22:53 -0700) ---------------------------------------------------------------- "Use ref transactions", early part Ronnie explains: This patch series expands on the transaction API. It converts ref updates, inside refs.c as well as external, to use the transaction API for updates. This makes most ref updates atomic when there are failures locking or writing to a ref. This series teaches the tag, replace, commit, cherry-pick, fast-import, checkout -b, branch, receive-pack, and http-fetch commands and all update_ref and delete_ref callers to use the ref transaction API instead of lock_ref_sha1. The main user-visible change should be some cosmetic changes to error messages. The series also combines multiple ref updates into a single transaction in 'git http-fetch -w' and when writing tags in fast-import but otherwise doesn't change the granularity of transactions. Reviewed at https://code-review.googlesource.com/#/q/topic:ref-transaction-1 ---------------------------------------------------------------- Ronnie Sahlberg (20): refs.c: change ref_transaction_create to do error checking and return status refs.c: update ref_transaction_delete to check for error and return status refs.c: make ref_transaction_begin take an err argument refs.c: add transaction.status and track OPEN/CLOSED tag.c: use ref transactions when doing updates replace.c: use the ref transaction functions for updates commit.c: use ref transactions for updates sequencer.c: use ref transactions for all ref updates fast-import.c: change update_branch to use ref transactions branch.c: use ref transaction for all ref updates refs.c: change update_ref to use a transaction receive-pack.c: use a reference transaction for updating the refs fast-import.c: use a ref transaction when dumping tags walker.c: use ref transaction for ref updates refs.c: make lock_ref_sha1 static refs.c: remove the update_ref_lock function refs.c: remove the update_ref_write function refs.c: remove lock_ref_sha1 refs.c: make prune_ref use a transaction to delete the ref refs.c: make delete_ref use a transaction branch.c | 31 ++++--- builtin/commit.c | 25 +++-- builtin/receive-pack.c | 25 +++-- builtin/replace.c | 14 +-- builtin/tag.c | 16 ++-- builtin/update-ref.c | 14 ++- fast-import.c | 54 +++++++---- refs.c | 244 ++++++++++++++++++++++++++++--------------------- refs.h | 77 ++++++++++++---- sequencer.c | 26 ++++-- walker.c | 70 ++++++++------ 11 files changed, 367 insertions(+), 229 deletions(-) [...] > Jonathan Nieder <jrnieder@xxxxxxxxx> writes: >> --- 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 (!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? It's about keeping the message in parentheses on the "! rejected master -> master" line short and simple, since the more detailed diagnosis is redundant next to the full message that is sent over sideband earlier and gets a line to itself. Side effects: * a less invasive patch --- no more need to change the calling convention to return a dynamically allocated string, with assertions throughout the file that check for leaks * using the same "all lowercase and brief" convention makes the message less jarring next to other statuses for "! rejected" lines, like "non-fast-forward" * no more O(n) stall from traversing the linked list to free messages when receive-pack is about to exit [...] >> 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. Ahhh. Thanks for explaining. Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html