Transaction patch series overview

List, please see here an overview and ordering of the ref transaction patch series.
These series build on each other and needs to be applied in the order listed below.

rs/ref-transaction-0
---------------------------
Early part of the "ref transaction" topic.

* rs/ref-transaction-0:
refs.c: change ref_transaction_update() to do error checking and
return status
refs.c: remove the onerr argument to ref_transaction_commit
update-ref: use err argument to get error from ref_transaction_commit refs.c: make update_ref_write update a strbuf on failure refs.c: make ref_update_reject_duplicates take a strbuf argument
for errors
refs.c: log_ref_write should try to return meaningful errno refs.c: make resolve_ref_unsafe set errno to something meaningful on error refs.c: commit_packed_refs to return a meaningful errno on failure refs.c: make remove_empty_directories always set errno to something sane refs.c: verify_lock should set errno to something meaningful refs.c: make sure log_ref_setup returns a meaningful errno refs.c: add an err argument to repack_without_refs lockfile.c: make lock_file return a meaningful errno on failurei lockfile.c: add a new public function unable_to_lock_message refs.c: add a strbuf argument to ref_transaction_commit for error logging refs.c: allow passing NULL to ref_transaction_free
refs.c: constify the sha arguments for
ref_transaction_create|delete|update
refs.c: ref_transaction_commit should not free the transaction refs.c: remove ref_transaction_rollback

Has been merged into next.

ref-transaction-1 (2014-07-16) 20 commits
-------------------------------------------------------------
Second batch of ref transactions

- refs.c: make delete_ref use a transaction
- refs.c: make prune_ref use a transaction to delete the ref
- refs.c: remove lock_ref_sha1
- refs.c: remove the update_ref_write function
- refs.c: remove the update_ref_lock function
- refs.c: make lock_ref_sha1 static
- walker.c: use ref transaction for ref updates
- fast-import.c: use a ref transaction when dumping tags
- receive-pack.c: use a reference transaction for updating the refs
- refs.c: change update_ref to use a transaction
- branch.c: use ref transaction for all ref updates
- fast-import.c: change update_branch to use ref transactions
- sequencer.c: use ref transactions for all ref updates
- commit.c: use ref transactions for updates
- replace.c: use the ref transaction functions for updates
- tag.c: use ref transactions when doing updates
- refs.c: add transaction.status and track OPEN/CLOSED/ERROR
- refs.c: make ref_transaction_begin take an err argument
- refs.c: update ref_transaction_delete to check for error and return status
- refs.c: change ref_transaction_create to do error checking and return status (this branch is used by rs/ref-transaction, rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename.)

The second batch of the transactional ref update series.

Has been merged into pu

rs/ref-transaction (2014-07-17) 12 commits
-----------------------------------------------------------------
- refs.c: fix handling of badly named refs
- refs.c: make write_ref_sha1 static
- fetch.c: change s_update_ref to use a ref transaction
- refs.c: propagate any errno==ENOTDIR from _commit back to the callers
- refs.c: pass a skip list to name_conflict_fn
- refs.c: call lock_ref_sha1_basic directly from commit
- refs.c: move the check for valid refname to lock_ref_sha1_basic
- refs.c: pass NULL as *flags to read_ref_full
- refs.c: pass the ref log message to _create/delete/update instead of _commit
- refs.c: add an err argument to delete_ref_loose
- wrapper.c: add a new function unlink_or_msg
- wrapper.c: simplify warn_if_unremovable
(this branch is used by rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename; uses rs/ref-transaction-1.)

The third and final part of the basic ref-transaction work.

Has been merged into pu.

rs/ref-transaction-reflog (2014-07-23) 15 commits
-----------------------------------------------------------------------
- refs.c: allow deleting refs with a broken sha1
- refs.c: remove lock_any_ref_for_update
- refs.c: make unlock_ref/close_ref/commit_ref static
- refs.c: rename log_ref_setup to create_reflog
- reflog.c: use a reflog transaction when writing during expire
- refs.c: allow multiple reflog updates during a single transaction
- refs.c: only write reflog update if msg is non-NULL
- refs.c: add a flag to allow reflog updates to truncate the log
- refs.c: add a transaction function to append a reflog entry
- lockfile.c: make hold_lock_file_for_append preserve meaningful errno
- refs.c: add a function to append a reflog entry to a fd
- refs.c: add a new update_type field to ref_update
- refs.c: rename the transaction functions
- refs.c: make ref_transaction_delete a wrapper for ref_transaction_update
- refs.c: make ref_transaction_create a wrapper to ref_transaction_update (this branch is used by rs/ref-transaction-multi and
rs/ref-transaction-rename; uses rs/ref-transaction and rs/ref-transaction-1.)

This patch series adds support for reflog updates to the transaction subsystem. Once it has refactored the builtin/reflog.c to use a transaction instead of accessing the refs and
reflogs directly it allows us to remove unlock_ref/close_ref/commit_ref/lock_any_ref_for_update
from the public API.

As part of the reflog work I also refactor how reflog creation works renaming log_ref_setup to create_reflog. These changes allow us then to reduce the number of places where
we expose the global log_all_ref_updates to the callers.
This leads to a much cleaner api for reflog creation and avoids completely the "manipulate the global log_all_ref_updates before calling log_ref_setup" thing that was done
in checkout.c. Now checkout.c can just call create_reflog() and the right thing will happen.

At the end of the patch series is an unrelated patch for repairing the "allow deletion of a broken ref" functionality that was lost a while back. This patch is independent of the reflog work but is placed here just to avoid creating additional conflicts with this series and series that follow.

Has been merged to pu.

Following are the patch series that has not yet made its way into pu.

ref-transactions-rename
-----------------------------------
refs.c: allow passing raw git_committer_info as email to _update_reflog refs.c: return error instead of dying when locking fails during transaction refs.c: use packed refs when deleting refs during a transaction refs.c: update rename_ref to use a transaction
refs.c: rollback the lockfile before we die() in repack_without_refs

This series focuses on reworking how rename_ref works and to make it use an atomic transaction for the rename.
The first three patches lead up to a point where we can now perform the "delete the old ref and add the new ref" as a single atomic operation to the packed refs file followed by the fourth patch where we rework rename_ref() completely to perform the rename and the reflog updates as a single atomic transaction.

As part of this the rename_ref logic is greatly simplified and we reach several nice goals:
* no need to disallow rename_ref() if the reflog is a symlink
* no need to rename the reflog via a third filename
* make the rename become completely atomic both for the caller but also for any external observers.

The fifth patch is independent to the rename work itself but was discovered during the work for this patch. It could be applied separately but I did not see any urgency to push it separetely and left it as a patch in this series.

ref-transactions-req-packed-refs
----------------------------------------------
refs.c: move reflog updates into its own function
refs.c: write updates to packed refs when a transaction has more than one ref remote.c: use a transaction for deleting refs
refs.c: make repack_without_refs static
refs.c: make the *_packed_refs functions static

These patches expands on the "perform ref operations as a packed refs file commit" and changes the transaction subsystem to always perform multi-ref updates as a packed ref commit instead of discreete operations on loose refs.

The new logic is basically:
IF the transaction only touches a single ref, then just do the normal loose ref operation as before.
This is to make sure that the very common "git commit" command will
still always be fast.

IF the transaction touches multiple refs, then
* first copy/move all affected refs to the packed refs file and
commit, then re-lock the packed
refs file until the transaction has completed.
* then delete any affected loose refs files (they are already in the
packed refs file)
* perform the operation by adding/removing/updating packed refs entries
* finally, commit the packed refs file

This leads to several new nice properties.
First, the transaction subsystem will now do the "right thing" and automatically convert any
multi-ref updates to use the packed refs file. This then means that we no longer need to have
special casing for clone.c to use a special packed_refs api. Clone.c can now just use a transaction for its "add all refs" and the right thing will happen.
Similar optimizations can be done for remote.c which now also can just use a transaction
and know that the right thing will happen.
As we now no longer need to expose the special purpose packed-refs api to external callers we can now make all the packed refs functions private to refs.c and shrink the size of the public api.

Note: we still do have packed refs and they still work the same way as before. It is just that the packed refs are now an internal optimization for the refs code and no longer part of the public api.

Second, all transaction that affects multiple refs will now also become atomic for any external observer regardless of whether they used the packed refs api or not.

The following functions are now static to refs.c and no longer part of
the public api:
add_packed_ref/lock_packed_refs/commit_packed_refs/rollback_packed_refs/repack_without_refs

ref-transactions-req-strbuf-err
-------------------------------------------
This series just contains various changes internally to refs.c to expand on the use of a strbuf *err as argument for passing error messages back to the caller.
Only thing by note here is tgat by doing this to update_ref() we can finally get rid of the action_on_err enum:

-enum action_on_err {

-       UPDATE_REFS_MSG_ON_ERR,
-       UPDATE_REFS_DIE_ON_ERR,
-       UPDATE_REFS_QUIET_ON_ERR

-};

wooohooo

As part of this patch series I noticed that there were two places in transport*.c where we were calling update_ref() and passing '0' as argument instead of UPDATE_REFS_MSG_ON_ERR.
Not a bug and not a change in functionality but it would be nice if the compiler could warn for this.

ref-transactions-send-pack
---------------------------------------
Various changes to allow a new argument --atomic-push, and protocol changes, to allow a client to request/negotiate that a multi-ref push should be atomic. I.e. apply all changes or none.

Not sent to the list yet.

ref-transactions-send-pack-post
----------------------------------------------
Various other changes to add a strbuf *err argument to error handling.

Not sent to the list yet.

And that concludes the basic ref-transaction stuff.

Once this is in I have a very large patch series that is mainly rearranging code and to add support for pluggable ref backends. But that is for when the ref transaction patches are in.

regards
ronnie sahlberg

List, please see here an overview and ordering of the ref transaction patch series.
These series build on each other and needs to be applied in the order listed below.

This is an update.

rs/ref-transaction-0
---------------------------
Early part of the "ref transaction" topic.

* rs/ref-transaction-0:
refs.c: change ref_transaction_update() to do error checking and
return status
refs.c: remove the onerr argument to ref_transaction_commit
update-ref: use err argument to get error from ref_transaction_commit refs.c: make update_ref_write update a strbuf on failure refs.c: make ref_update_reject_duplicates take a strbuf argument
for errors
refs.c: log_ref_write should try to return meaningful errno refs.c: make resolve_ref_unsafe set errno to something meaningful on error refs.c: commit_packed_refs to return a meaningful errno on failure refs.c: make remove_empty_directories always set errno to something sane refs.c: verify_lock should set errno to something meaningful refs.c: make sure log_ref_setup returns a meaningful errno refs.c: add an err argument to repack_without_refs lockfile.c: make lock_file return a meaningful errno on failurei lockfile.c: add a new public function unable_to_lock_message refs.c: add a strbuf argument to ref_transaction_commit for error logging refs.c: allow passing NULL to ref_transaction_free
refs.c: constify the sha arguments for
ref_transaction_create|delete|update
refs.c: ref_transaction_commit should not free the transaction refs.c: remove ref_transaction_rollback

Has been merged into next.

ref-transaction-1 (2014-07-16) 20 commits
-------------------------------------------------------------
Second batch of ref transactions

- refs.c: make delete_ref use a transaction
- refs.c: make prune_ref use a transaction to delete the ref
- refs.c: remove lock_ref_sha1
- refs.c: remove the update_ref_write function
- refs.c: remove the update_ref_lock function
- refs.c: make lock_ref_sha1 static
- walker.c: use ref transaction for ref updates
- fast-import.c: use a ref transaction when dumping tags
- receive-pack.c: use a reference transaction for updating the refs
- refs.c: change update_ref to use a transaction
- branch.c: use ref transaction for all ref updates
- fast-import.c: change update_branch to use ref transactions
- sequencer.c: use ref transactions for all ref updates
- commit.c: use ref transactions for updates
- replace.c: use the ref transaction functions for updates
- tag.c: use ref transactions when doing updates
- refs.c: add transaction.status and track OPEN/CLOSED/ERROR
- refs.c: make ref_transaction_begin take an err argument
- refs.c: update ref_transaction_delete to check for error and return status
- refs.c: change ref_transaction_create to do error checking and return status (this branch is used by rs/ref-transaction, rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename.)

The second batch of the transactional ref update series.

Has been merged into pu

rs/ref-transaction (2014-07-17) 12 commits
-----------------------------------------------------------------
- refs.c: fix handling of badly named refs
- refs.c: make write_ref_sha1 static
- fetch.c: change s_update_ref to use a ref transaction
- refs.c: propagate any errno==ENOTDIR from _commit back to the callers
- refs.c: pass a skip list to name_conflict_fn
- refs.c: call lock_ref_sha1_basic directly from commit
- refs.c: move the check for valid refname to lock_ref_sha1_basic
- refs.c: pass NULL as *flags to read_ref_full
- refs.c: pass the ref log message to _create/delete/update instead of _commit
- refs.c: add an err argument to delete_ref_loose
- wrapper.c: add a new function unlink_or_msg
- wrapper.c: simplify warn_if_unremovable
(this branch is used by rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename; uses rs/ref-transaction-1.)

The third and final part of the basic ref-transaction work.

Has been merged into pu.

rs/ref-transaction-reflog (2014-07-23) 15 commits
-----------------------------------------------------------------------
- refs.c: allow deleting refs with a broken sha1
- refs.c: remove lock_any_ref_for_update
- refs.c: make unlock_ref/close_ref/commit_ref static
- refs.c: rename log_ref_setup to create_reflog
- reflog.c: use a reflog transaction when writing during expire
- refs.c: allow multiple reflog updates during a single transaction
- refs.c: only write reflog update if msg is non-NULL
- refs.c: add a flag to allow reflog updates to truncate the log
- refs.c: add a transaction function to append a reflog entry
- lockfile.c: make hold_lock_file_for_append preserve meaningful errno
- refs.c: add a function to append a reflog entry to a fd
- refs.c: add a new update_type field to ref_update
- refs.c: rename the transaction functions
- refs.c: make ref_transaction_delete a wrapper for ref_transaction_update
- refs.c: make ref_transaction_create a wrapper to ref_transaction_update (this branch is used by rs/ref-transaction-multi and
rs/ref-transaction-rename; uses rs/ref-transaction and rs/ref-transaction-1.)

This patch series adds support for reflog updates to the transaction subsystem. Once it has refactored the builtin/reflog.c to use a transaction instead of accessing the refs and
reflogs directly it allows us to remove unlock_ref/close_ref/commit_ref/lock_any_ref_for_update
from the public API.

As part of the reflog work I also refactor how reflog creation works renaming log_ref_setup to create_reflog. These changes allow us then to reduce the number of places where
we expose the global log_all_ref_updates to the callers.
This leads to a much cleaner api for reflog creation and avoids completely the "manipulate the global log_all_ref_updates before calling log_ref_setup" thing that was done
in checkout.c. Now checkout.c can just call create_reflog() and the right thing will happen.

At the end of the patch series is an unrelated patch for repairing the "allow deletion of a broken ref" functionality that was lost a while back. This patch is independent of the reflog work but is placed here just to avoid creating additional conflicts with this series and series that follow.

Has been merged to pu.

ref-transactions-rename
-----------------------------------
refs.c: allow passing raw git_committer_info as email to _update_reflog refs.c: return error instead of dying when locking fails during transaction refs.c: use packed refs when deleting refs during a transaction refs.c: update rename_ref to use a transaction
refs.c: rollback the lockfile before we die() in repack_without_refs

This series focuses on reworking how rename_ref works and to make it use an atomic transaction for the rename.
The first three patches lead up to a point where we can now perform the "delete the old ref and add the new ref" as a single atomic operation to the packed refs file followed by the fourth patch where we rework rename_ref() completely to perform the rename and the reflog updates as a single atomic transaction.

As part of this the rename_ref logic is greatly simplified and we reach several nice goals:
* no need to disallow rename_ref() if the reflog is a symlink
* no need to rename the reflog via a third filename
* make the rename become completely atomic both for the caller but also for any external observers.

The fifth patch is independent to the rename work itself but was discovered during the work for this patch. It could be applied separately but I did not see any urgency to push it separetely and left it as a patch in this series.

Has been merged into pu.

ref-transactions-req-packed-refs
----------------------------------------------
refs.c: move reflog updates into its own function
refs.c: write updates to packed refs when a transaction has more than one ref remote.c: use a transaction for deleting refs
refs.c: make repack_without_refs static
refs.c: make the *_packed_refs functions static

These patches expands on the "perform ref operations as a packed refs file commit" and changes the transaction subsystem to always perform multi-ref updates as a packed ref commit instead of discreete operations on loose refs.

The new logic is basically:
IF the transaction only touches a single ref, then just do the normal loose ref operation as before.
This is to make sure that the very common "git commit" command will
still always be fast.

IF the transaction touches multiple refs, then
* first copy/move all affected refs to the packed refs file and
commit, then re-lock the packed
refs file until the transaction has completed.
* then delete any affected loose refs files (they are already in the
packed refs file)
* perform the operation by adding/removing/updating packed refs entries
* finally, commit the packed refs file

This leads to several new nice properties.
First, the transaction subsystem will now do the "right thing" and automatically convert any
multi-ref updates to use the packed refs file. This then means that we no longer need to have
special casing for clone.c to use a special packed_refs api. Clone.c can now just use a transaction for its "add all refs" and the right thing will happen.
Similar optimizations can be done for remote.c which now also can just use a transaction
and know that the right thing will happen.
As we now no longer need to expose the special purpose packed-refs api to external callers we can now make all the packed refs functions private to refs.c and shrink the size of the public api.

Note: we still do have packed refs and they still work the same way as before. It is just that the packed refs are now an internal optimization for the refs code and no longer part of the public api.

Second, all transaction that affects multiple refs will now also become atomic for any external observer regardless of whether they used the packed refs api or not.

The following functions are now static to refs.c and no longer part of
the public api:
add_packed_ref/lock_packed_refs/commit_packed_refs/rollback_packed_refs/repack_without_refs

Has been merged into pu.

Following are the patch series that has not yet made its way into pu.

ref-transactions-req-strbuf-err
-------------------------------------------
This series just contains various changes internally to refs.c to expand on the use of a strbuf *err as argument for passing error messages back to the caller.
Only thing by note here is tgat by doing this to update_ref() we can finally get rid of the action_on_err enum:

-enum action_on_err {

-       UPDATE_REFS_MSG_ON_ERR,
-       UPDATE_REFS_DIE_ON_ERR,
-       UPDATE_REFS_QUIET_ON_ERR

-};

wooohooo

As part of this patch series I noticed that there were two places in transport*.c where we were calling update_ref() and passing '0' as argument instead of UPDATE_REFS_MSG_ON_ERR.
Not a bug and not a change in functionality but it would be nice if the compiler could warn for this.

ref-transactions-send-pack
---------------------------------------
Various changes to allow a new argument --atomic-push, and protocol changes, to allow a client to request/negotiate that a multi-ref push should be atomic. I.e. apply all changes or none.

ref-transactions-send-pack-post
----------------------------------------------
Various other changes to add a strbuf *err argument to error handling.

Not sent to the list yet.

And that concludes the basic ref-transaction stuff.

Once this is in I have a very large patch series that is mainly rearranging code and to add support for pluggable ref backends. But that is for when the ref transaction patches are in.

regards

List, please see here an overview and ordering of the ref transaction patch series.
These series build on each other and needs to be applied in the order listed below.

This is an update.

rs/ref-transaction-0
---------------------------
Early part of the "ref transaction" topic.

* rs/ref-transaction-0:
refs.c: change ref_transaction_update() to do error checking and
return status
refs.c: remove the onerr argument to ref_transaction_commit
update-ref: use err argument to get error from ref_transaction_commit refs.c: make update_ref_write update a strbuf on failure refs.c: make ref_update_reject_duplicates take a strbuf argument
for errors
refs.c: log_ref_write should try to return meaningful errno refs.c: make resolve_ref_unsafe set errno to something meaningful on error refs.c: commit_packed_refs to return a meaningful errno on failure refs.c: make remove_empty_directories always set errno to something sane refs.c: verify_lock should set errno to something meaningful refs.c: make sure log_ref_setup returns a meaningful errno refs.c: add an err argument to repack_without_refs lockfile.c: make lock_file return a meaningful errno on failurei lockfile.c: add a new public function unable_to_lock_message refs.c: add a strbuf argument to ref_transaction_commit for error logging refs.c: allow passing NULL to ref_transaction_free
refs.c: constify the sha arguments for
ref_transaction_create|delete|update
refs.c: ref_transaction_commit should not free the transaction refs.c: remove ref_transaction_rollback

Has been merged into next.

ref-transaction-1 (2014-07-16) 20 commits
-------------------------------------------------------------
Second batch of ref transactions

- refs.c: make delete_ref use a transaction
- refs.c: make prune_ref use a transaction to delete the ref
- refs.c: remove lock_ref_sha1
- refs.c: remove the update_ref_write function
- refs.c: remove the update_ref_lock function
- refs.c: make lock_ref_sha1 static
- walker.c: use ref transaction for ref updates
- fast-import.c: use a ref transaction when dumping tags
- receive-pack.c: use a reference transaction for updating the refs
- refs.c: change update_ref to use a transaction
- branch.c: use ref transaction for all ref updates
- fast-import.c: change update_branch to use ref transactions
- sequencer.c: use ref transactions for all ref updates
- commit.c: use ref transactions for updates
- replace.c: use the ref transaction functions for updates
- tag.c: use ref transactions when doing updates
- refs.c: add transaction.status and track OPEN/CLOSED/ERROR
- refs.c: make ref_transaction_begin take an err argument
- refs.c: update ref_transaction_delete to check for error and return status
- refs.c: change ref_transaction_create to do error checking and return status (this branch is used by rs/ref-transaction, rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename.)

The second batch of the transactional ref update series.

Has been merged into pu

rs/ref-transaction (2014-07-17) 12 commits
-----------------------------------------------------------------
- refs.c: fix handling of badly named refs
- refs.c: make write_ref_sha1 static
- fetch.c: change s_update_ref to use a ref transaction
- refs.c: propagate any errno==ENOTDIR from _commit back to the callers
- refs.c: pass a skip list to name_conflict_fn
- refs.c: call lock_ref_sha1_basic directly from commit
- refs.c: move the check for valid refname to lock_ref_sha1_basic
- refs.c: pass NULL as *flags to read_ref_full
- refs.c: pass the ref log message to _create/delete/update instead of _commit
- refs.c: add an err argument to delete_ref_loose
- wrapper.c: add a new function unlink_or_msg
- wrapper.c: simplify warn_if_unremovable
(this branch is used by rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename; uses rs/ref-transaction-1.)

The third and final part of the basic ref-transaction work.

Has been merged into pu.

rs/ref-transaction-reflog (2014-07-23) 15 commits
-----------------------------------------------------------------------
- refs.c: allow deleting refs with a broken sha1
- refs.c: remove lock_any_ref_for_update
- refs.c: make unlock_ref/close_ref/commit_ref static
- refs.c: rename log_ref_setup to create_reflog
- reflog.c: use a reflog transaction when writing during expire
- refs.c: allow multiple reflog updates during a single transaction
- refs.c: only write reflog update if msg is non-NULL
- refs.c: add a flag to allow reflog updates to truncate the log
- refs.c: add a transaction function to append a reflog entry
- lockfile.c: make hold_lock_file_for_append preserve meaningful errno
- refs.c: add a function to append a reflog entry to a fd
- refs.c: add a new update_type field to ref_update
- refs.c: rename the transaction functions
- refs.c: make ref_transaction_delete a wrapper for ref_transaction_update
- refs.c: make ref_transaction_create a wrapper to ref_transaction_update (this branch is used by rs/ref-transaction-multi and
rs/ref-transaction-rename; uses rs/ref-transaction and rs/ref-transaction-1.)

This patch series adds support for reflog updates to the transaction subsystem. Once it has refactored the builtin/reflog.c to use a transaction instead of accessing the refs and
reflogs directly it allows us to remove unlock_ref/close_ref/commit_ref/lock_any_ref_for_update
from the public API.

As part of the reflog work I also refactor how reflog creation works renaming log_ref_setup to create_reflog. These changes allow us then to reduce the number of places where
we expose the global log_all_ref_updates to the callers.
This leads to a much cleaner api for reflog creation and avoids completely the "manipulate the global log_all_ref_updates before calling log_ref_setup" thing that was done
in checkout.c. Now checkout.c can just call create_reflog() and the right thing will happen.

At the end of the patch series is an unrelated patch for repairing the "allow deletion of a broken ref" functionality that was lost a while back. This patch is independent of the reflog work but is placed here just to avoid creating additional conflicts with this series and series that follow.

Has been merged to pu.

ref-transactions-rename
-----------------------------------
refs.c: allow passing raw git_committer_info as email to _update_reflog refs.c: return error instead of dying when locking fails during transaction refs.c: use packed refs when deleting refs during a transaction refs.c: update rename_ref to use a transaction
refs.c: rollback the lockfile before we die() in repack_without_refs

This series focuses on reworking how rename_ref works and to make it use an atomic transaction for the rename.
The first three patches lead up to a point where we can now perform the "delete the old ref and add the new ref" as a single atomic operation to the packed refs file followed by the fourth patch where we rework rename_ref() completely to perform the rename and the reflog updates as a single atomic transaction.

As part of this the rename_ref logic is greatly simplified and we reach several nice goals:
* no need to disallow rename_ref() if the reflog is a symlink
* no need to rename the reflog via a third filename
* make the rename become completely atomic both for the caller but also for any external observers.

The fifth patch is independent to the rename work itself but was discovered during the work for this patch. It could be applied separately but I did not see any urgency to push it separetely and left it as a patch in this series.

Has been merged into pu.

ref-transactions-req-packed-refs
----------------------------------------------
refs.c: move reflog updates into its own function
refs.c: write updates to packed refs when a transaction has more than one ref remote.c: use a transaction for deleting refs
refs.c: make repack_without_refs static
refs.c: make the *_packed_refs functions static

These patches expands on the "perform ref operations as a packed refs file commit" and changes the transaction subsystem to always perform multi-ref updates as a packed ref commit instead of discreete operations on loose refs.

The new logic is basically:
IF the transaction only touches a single ref, then just do the normal loose ref operation as before.
This is to make sure that the very common "git commit" command will
still always be fast.

IF the transaction touches multiple refs, then
* first copy/move all affected refs to the packed refs file and
commit, then re-lock the packed
refs file until the transaction has completed.
* then delete any affected loose refs files (they are already in the
packed refs file)
* perform the operation by adding/removing/updating packed refs entries
* finally, commit the packed refs file

This leads to several new nice properties.
First, the transaction subsystem will now do the "right thing" and automatically convert any
multi-ref updates to use the packed refs file. This then means that we no longer need to have
special casing for clone.c to use a special packed_refs api. Clone.c can now just use a transaction for its "add all refs" and the right thing will happen.
Similar optimizations can be done for remote.c which now also can just use a transaction
and know that the right thing will happen.
As we now no longer need to expose the special purpose packed-refs api to external callers we can now make all the packed refs functions private to refs.c and shrink the size of the public api.

Note: we still do have packed refs and they still work the same way as before. It is just that the packed refs are now an internal optimization for the refs code and no longer part of the public api.

Second, all transaction that affects multiple refs will now also become atomic for any external observer regardless of whether they used the packed refs api or not.

The following functions are now static to refs.c and no longer part of
the public api:
add_packed_ref/lock_packed_refs/commit_packed_refs/rollback_packed_refs/repack_without_refs

Has been merged into pu.

Following are the patch series that has not yet made its way into pu.

ref-transactions-req-strbuf-err
-------------------------------------------
This series just contains various changes internally to refs.c to expand on the use of a strbuf *err as argument for passing error messages back to the caller.
Only thing by note here is tgat by doing this to update_ref() we can finally get rid of the action_on_err enum:

-enum action_on_err {

-       UPDATE_REFS_MSG_ON_ERR,
-       UPDATE_REFS_DIE_ON_ERR,
-       UPDATE_REFS_QUIET_ON_ERR

-};

wooohooo

As part of this patch series I noticed that there were two places in transport*.c where we were calling update_ref() and passing '0' as argument instead of UPDATE_REFS_MSG_ON_ERR.
Not a bug and not a change in functionality but it would be nice if the compiler could warn for this.

Sent to the list.

ref-transactions-send-pack
---------------------------------------
Various changes to allow a new argument --atomic-push, and protocol changes, to allow a client to request/negotiate that a multi-ref push should be atomic. I.e. apply all changes or none.

Sent to the list.

backend-struct-db
--------------------------
Various changes that mainly moves functions between files to split the current refs.c file into functions that are backend agnostic and the functions that implement the files based ref backend.
It finally adds a backend structure and the methods that define a backend datastore for refs.

This series concludes the work to define pluggable backends for refs. At this stage it will now be possible to start building and adding new types of ref backends to git.

Sent to the list

regards
ronnie sahlberg

List, please see here an overview and ordering of the ref transaction patch series.
These series build on each other and needs to be applied in the order listed below.

This is an update.

rs/ref-transaction-0
---------------------------
Early part of the "ref transaction" topic.

* rs/ref-transaction-0:
refs.c: change ref_transaction_update() to do error checking and
return status
refs.c: remove the onerr argument to ref_transaction_commit
update-ref: use err argument to get error from ref_transaction_commit refs.c: make update_ref_write update a strbuf on failure refs.c: make ref_update_reject_duplicates take a strbuf argument
for errors
refs.c: log_ref_write should try to return meaningful errno refs.c: make resolve_ref_unsafe set errno to something meaningful on error refs.c: commit_packed_refs to return a meaningful errno on failure refs.c: make remove_empty_directories always set errno to something sane refs.c: verify_lock should set errno to something meaningful refs.c: make sure log_ref_setup returns a meaningful errno refs.c: add an err argument to repack_without_refs lockfile.c: make lock_file return a meaningful errno on failurei lockfile.c: add a new public function unable_to_lock_message refs.c: add a strbuf argument to ref_transaction_commit for error logging refs.c: allow passing NULL to ref_transaction_free
refs.c: constify the sha arguments for
ref_transaction_create|delete|update
refs.c: ref_transaction_commit should not free the transaction refs.c: remove ref_transaction_rollback

Has been merged into next.

ref-transaction-1 (2014-07-16) 20 commits
-------------------------------------------------------------
Second batch of ref transactions

- refs.c: make delete_ref use a transaction
- refs.c: make prune_ref use a transaction to delete the ref
- refs.c: remove lock_ref_sha1
- refs.c: remove the update_ref_write function
- refs.c: remove the update_ref_lock function
- refs.c: make lock_ref_sha1 static
- walker.c: use ref transaction for ref updates
- fast-import.c: use a ref transaction when dumping tags
- receive-pack.c: use a reference transaction for updating the refs
- refs.c: change update_ref to use a transaction
- branch.c: use ref transaction for all ref updates
- fast-import.c: change update_branch to use ref transactions
- sequencer.c: use ref transactions for all ref updates
- commit.c: use ref transactions for updates
- replace.c: use the ref transaction functions for updates
- tag.c: use ref transactions when doing updates
- refs.c: add transaction.status and track OPEN/CLOSED/ERROR
- refs.c: make ref_transaction_begin take an err argument
- refs.c: update ref_transaction_delete to check for error and return status
- refs.c: change ref_transaction_create to do error checking and return status (this branch is used by rs/ref-transaction, rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename.)

The second batch of the transactional ref update series.

Has been merged into pu

rs/ref-transaction (2014-07-17) 12 commits
-----------------------------------------------------------------
- refs.c: fix handling of badly named refs
- refs.c: make write_ref_sha1 static
- fetch.c: change s_update_ref to use a ref transaction
- refs.c: propagate any errno==ENOTDIR from _commit back to the callers
- refs.c: pass a skip list to name_conflict_fn
- refs.c: call lock_ref_sha1_basic directly from commit
- refs.c: move the check for valid refname to lock_ref_sha1_basic
- refs.c: pass NULL as *flags to read_ref_full
- refs.c: pass the ref log message to _create/delete/update instead of _commit
- refs.c: add an err argument to delete_ref_loose
- wrapper.c: add a new function unlink_or_msg
- wrapper.c: simplify warn_if_unremovable
(this branch is used by rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename; uses rs/ref-transaction-1.)

The third and final part of the basic ref-transaction work.

Has been merged into pu.

rs/ref-transaction-reflog (2014-07-23) 15 commits
-----------------------------------------------------------------------
- refs.c: allow deleting refs with a broken sha1
- refs.c: remove lock_any_ref_for_update
- refs.c: make unlock_ref/close_ref/commit_ref static
- refs.c: rename log_ref_setup to create_reflog
- reflog.c: use a reflog transaction when writing during expire
- refs.c: allow multiple reflog updates during a single transaction
- refs.c: only write reflog update if msg is non-NULL
- refs.c: add a flag to allow reflog updates to truncate the log
- refs.c: add a transaction function to append a reflog entry
- lockfile.c: make hold_lock_file_for_append preserve meaningful errno
- refs.c: add a function to append a reflog entry to a fd
- refs.c: add a new update_type field to ref_update
- refs.c: rename the transaction functions
- refs.c: make ref_transaction_delete a wrapper for ref_transaction_update
- refs.c: make ref_transaction_create a wrapper to ref_transaction_update (this branch is used by rs/ref-transaction-multi and
rs/ref-transaction-rename; uses rs/ref-transaction and rs/ref-transaction-1.)

This patch series adds support for reflog updates to the transaction subsystem. Once it has refactored the builtin/reflog.c to use a transaction instead of accessing the refs and
reflogs directly it allows us to remove unlock_ref/close_ref/commit_ref/lock_any_ref_for_update
from the public API.

As part of the reflog work I also refactor how reflog creation works renaming log_ref_setup to create_reflog. These changes allow us then to reduce the number of places where
we expose the global log_all_ref_updates to the callers.
This leads to a much cleaner api for reflog creation and avoids completely the "manipulate the global log_all_ref_updates before calling log_ref_setup" thing that was done
in checkout.c. Now checkout.c can just call create_reflog() and the right thing will happen.

At the end of the patch series is an unrelated patch for repairing the "allow deletion of a broken ref" functionality that was lost a while back. This patch is independent of the reflog work but is placed here just to avoid creating additional conflicts with this series and series that follow.

Has been merged to pu.

ref-transactions-rename
-----------------------------------
refs.c: allow passing raw git_committer_info as email to _update_reflog refs.c: return error instead of dying when locking fails during transaction refs.c: use packed refs when deleting refs during a transaction refs.c: update rename_ref to use a transaction
refs.c: rollback the lockfile before we die() in repack_without_refs

This series focuses on reworking how rename_ref works and to make it use an atomic transaction for the rename.
The first three patches lead up to a point where we can now perform the "delete the old ref and add the new ref" as a single atomic operation to the packed refs file followed by the fourth patch where we rework rename_ref() completely to perform the rename and the reflog updates as a single atomic transaction.

As part of this the rename_ref logic is greatly simplified and we reach several nice goals:
* no need to disallow rename_ref() if the reflog is a symlink
* no need to rename the reflog via a third filename
* make the rename become completely atomic both for the caller but also for any external observers.

The fifth patch is independent to the rename work itself but was discovered during the work for this patch. It could be applied separately but I did not see any urgency to push it separetely and left it as a patch in this series.

Has been merged into pu.

ref-transactions-req-packed-refs
----------------------------------------------
refs.c: move reflog updates into its own function
refs.c: write updates to packed refs when a transaction has more than one ref remote.c: use a transaction for deleting refs
refs.c: make repack_without_refs static
refs.c: make the *_packed_refs functions static

These patches expands on the "perform ref operations as a packed refs file commit" and changes the transaction subsystem to always perform multi-ref updates as a packed ref commit instead of discreete operations on loose refs.

The new logic is basically:
IF the transaction only touches a single ref, then just do the normal loose ref operation as before.
This is to make sure that the very common "git commit" command will
still always be fast.

IF the transaction touches multiple refs, then
* first copy/move all affected refs to the packed refs file and
commit, then re-lock the packed
refs file until the transaction has completed.
* then delete any affected loose refs files (they are already in the
packed refs file)
* perform the operation by adding/removing/updating packed refs entries
* finally, commit the packed refs file

This leads to several new nice properties.
First, the transaction subsystem will now do the "right thing" and automatically convert any
multi-ref updates to use the packed refs file. This then means that we no longer need to have
special casing for clone.c to use a special packed_refs api. Clone.c can now just use a transaction for its "add all refs" and the right thing will happen.
Similar optimizations can be done for remote.c which now also can just use a transaction
and know that the right thing will happen.
As we now no longer need to expose the special purpose packed-refs api to external callers we can now make all the packed refs functions private to refs.c and shrink the size of the public api.

Note: we still do have packed refs and they still work the same way as before. It is just that the packed refs are now an internal optimization for the refs code and no longer part of the public api.

Second, all transaction that affects multiple refs will now also become atomic for any external observer regardless of whether they used the packed refs api or not.

The following functions are now static to refs.c and no longer part of
the public api:
add_packed_ref/lock_packed_refs/commit_packed_refs/rollback_packed_refs/repack_without_refs

Has been merged into pu.

Following are the patch series that has not yet made its way into pu.

ref-transactions-req-strbuf-err
-------------------------------------------
This series just contains various changes internally to refs.c to expand on the use of a strbuf *err as argument for passing error messages back to the caller.
Only thing by note here is tgat by doing this to update_ref() we can finally get rid of the action_on_err enum:

-enum action_on_err {

-       UPDATE_REFS_MSG_ON_ERR,
-       UPDATE_REFS_DIE_ON_ERR,
-       UPDATE_REFS_QUIET_ON_ERR

-};

wooohooo

As part of this patch series I noticed that there were two places in transport*.c where we were calling update_ref() and passing '0' as argument instead of UPDATE_REFS_MSG_ON_ERR.
Not a bug and not a change in functionality but it would be nice if the compiler could warn for this.

Sent to the list.

ref-transactions-send-pack
---------------------------------------
Various changes to allow a new argument --atomic-push, and protocol changes, to allow a client to request/negotiate that a multi-ref push should be atomic. I.e. apply all changes or none.

Sent to the list.

backend-struct-db
--------------------------
Various changes that mainly moves functions between files to split the current refs.c file into functions that are backend agnostic and the functions that implement the files based ref backend.
It finally adds a backend structure and the methods that define a backend datastore for refs.

This series concludes the work to define pluggable backends for refs. At this stage it will now be possible to start building and adding new types of ref backends to git.

Sent to the list

In addition to these patches, there is one more patchseries with 4 patches at :
https://github.com/rsahlberg/git/tree/backend-struct-db-2

This series is for once the previous transaction changes are in and this series will add
a new backend for refs: refs-be-db.c which offloads all refs access to an external database daemon.
An example reference implementation for an external daemon is also provided and can be used as basis for
creating a daemon to plug into, say, a SQL database or similar.

regards
ronnie sahlberg

I gave a quick re-read-thru on this and did not find anything questionable, nor recall any issue pending. Unless somebody objects in the coming few days, let's move it to 'master' as part of the first batch for 2.2.

I'll re-read these two before deciding to merge them to 'next'. Any pending issues from the previous discussions?

A filesystem based backend/external daemon that is compatible with the traditional on-disk format might be another fun demonstration.

Looking forward to shepherding the whole thing ahead ;-) The most fun part awaits us but there is a long way.

Thanks.

Hi,

Thanks much for this.

[...]

[...]

This is even part of "master" now, so if people have review comments then they can make them most easily by submitting new patches on top.

Last reviewed at
http://thread.gmane.org/gmane.comp.version-control.git/252226, if I am using gmane search correctly.

Are there any pending questions for this part?

I've having trouble keeping track of how patches change, which patches have been reviewed and which haven't, unaddressed comments, and so on, so as an experiment I've pushed this part of the series to the Gerrit server at

https://code-review.googlesource.com/#/q/topic:ref-transaction-1

It's running a copy of gerrit code review
(https://code.google.com/p/gerrit).

Maybe this can be useful as a way of keeping track of the state of long and long-lived series like this one. It works something like this:

Preparation
-----------
Get a password from https://code-review.googlesource.com/new-password
and put it in netrc.

Install the hook from
https://code-review.googlesource.com/tools/hooks/commit-msg to .git/hooks/commit-msg. This automatically populates a Change-Id line in the commit message if none is present so gerrit can tell when you are uploading a new version of an existing patch.

(The commit-msg hook can be annoying when not working with gerrit code review. To turn it off, use 'git config gerrit.createChangeId false'.)

Uploading patches
-----------------
Write a patch series against 'maint' or 'master' as usual, then push:

	git push https://code.googlesource.com/git \
		HEAD:refs/for/master; # or refs/for/maint

There can be a parameter '%topic=<name>' after the refs/for/<branch> (e.g., refs/for/master%topic=ref-transaction-1) if the series should be labelled with a topic name, or that can be set in the web UI or using the HTTP API after the fact.

Commenting on patches
---------------------
Visit https://code-review.googlesource.com. Patches can be found under "All" -> "Open". Patches you're involved with somehow are at "My" -> "Changes".

To prepare inline comments, choose a file, highlight the text to comment on or click a line number, and comment.

To publish comments, go back up to the change overview screen (using the up arrow button or by pressing "u"), click "Reply", and say something.

'?' brings up keyboard shortcuts.

Downloading patches
-------------------
In the change overview screen, there's a 'Download' menu in the top-right corner with commands to download the patch.

Revising patches
----------------
After modifying a patch locally using the usual tools such as rebase --interactive, push again:

	git push https://code.googlesource.com/git \
		HEAD:refs/for/master; # or refs/for/maint

When a patch seems polished
---------------------------
Get rid of the Change-Id lines --- they shouldn't be needed any more. Send the patches or a request to pull from a public git repository, as usual. A link (in the top-left corner where it says 'Change 1000', the "1000" is a link to the change) can be helpful for letting people on-list see how the patch got into the form it's in today.

Maybe it can be useful.

Thanks,
Jonathan

[...]

Outcome of the experiment: patches gained some minor changes and then

1-12

13

14

15-16

17

18

19

20

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. I'm still hoping for a tweak in response to a minor comment and then I can put up a copy of the updated series to pull.

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.

index c1eae00..37ac555 100644
@@ -305,6 +305,7 @@ void create_branch(const char *head,
 		    ref_transaction_commit(transaction, msg, &err))
 			die("%s", err.buf);
 		ref_transaction_free(transaction);
+		strbuf_release(&err);
 	}
 
 	if (real_ref && track)

index 668fa6a..9bf1003 100644
@@ -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();

@@ -1801,5 +1801,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (!quiet)
 		print_summary(prefix, sha1, !current_head);
 
+	strbuf_release(&err);
 	return 0;
 }

index 7528f3d..1fcd06d 100644
@@ -169,8 +169,7 @@ static int replace_object_sha1(const char *object_ref,
 
 	transaction = ref_transaction_begin(&err);
 	if (!transaction ||
-	    ref_transaction_update(transaction, ref, repl, prev,
-				   0, !is_null_sha1(prev), &err) ||
+	    ref_transaction_update(transaction, ref, repl, prev, 0, 1, &err) ||
 	    ref_transaction_commit(transaction, NULL, &err))
 		die("%s", err.buf);
 

index 1aa88a2..f3f172f 100644
@@ -712,6 +712,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (force && !is_null_sha1(prev) && hashcmp(prev, object))
 		printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV));
 
+	strbuf_release(&err);
 	strbuf_release(&buf);
 	strbuf_release(&ref);
 	return 0;

index c6ad0be..96a53b9 100644
@@ -366,6 +366,8 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 
 	if (read_stdin) {
 		transaction = ref_transaction_begin(&err);
+		if (!transaction)
+			die("%s", err.buf);
 		if (delete || no_deref || argc > 0)
 			usage_with_options(git_update_ref_usage, options);
 		if (end_null)

@@ -374,6 +376,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		if (ref_transaction_commit(transaction, msg, &err))
 			die("%s", err.buf);
 		ref_transaction_free(transaction);
+		strbuf_release(&err);
 		return 0;
 	}
 

index a95e1be..e7f6e37 100644
@@ -1716,6 +1716,7 @@ static int update_branch(struct branch *b)
 		return -1;
 	}
 	ref_transaction_free(transaction);
+	strbuf_release(&err);
 	return 0;
 }
 

index 0017d9c..819e25f 100644
@@ -2074,7 +2074,10 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 	return logs_found;
 }
 
-/* This function should make sure errno is meaningful on error */
+/*
+ * Locks a "refs/" ref returning the lock on success and NULL on failure.
+ * On failure errno is set to something meaningful.
+ */
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 					    const unsigned char *old_sha1,
 					    int flags, int *type_p)

@@ -2401,6 +2404,7 @@ static void prune_ref(struct ref_to_prune *r)
 		return;
 	}
 	ref_transaction_free(transaction);
+	strbuf_release(&err);
 	try_remove_empty_parents(r->name);
 }
 

@@ -2575,6 +2579,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 		return 1;
 	}
 	ref_transaction_free(transaction);
+	strbuf_release(&err);
 	return 0;
 }
 

@@ -3352,19 +3357,15 @@ struct ref_update {
  * Transaction states.
  * OPEN:   The transaction is in a valid state and can accept new updates.
  *         An OPEN transaction can be committed.
- * CLOSED: If an open transaction is successfully committed the state will
- *         change to CLOSED. No further changes can be made to a CLOSED
- *         transaction.
- *         CLOSED means that all updates have been successfully committed and
- *         the only thing that remains is to free the completed transaction.
- * ERROR:  The transaction has failed and is no longer committable.
- *         No further changes can be made to a CLOSED transaction and it must
- *         be rolled back using transaction_free.
+ * CLOSED: A closed transaction is no longer active and no other operations
+ *         than free can be used on it in this state.
+ *         A transaction can either become closed by successfully committing
+ *         an active transaction or if there is a failure while building
+ *         the transaction thus rendering it failed/inactive.
  */
 enum ref_transaction_state {
 	REF_TRANSACTION_OPEN   = 0,
-	REF_TRANSACTION_CLOSED = 1,
-	REF_TRANSACTION_ERROR  = 2,
+	REF_TRANSACTION_CLOSED = 1
 };
 
 /*

@@ -3509,6 +3510,7 @@ int update_ref(const char *action, const char *refname,
 		strbuf_release(&err);
 		return 1;
 	}
+	strbuf_release(&err);
 	return 0;
 }
 

@@ -3588,10 +3590,10 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 					     msg);
 			update->lock = NULL; /* freed by write_ref_sha1 */
 			if (ret) {
-				const char *str = "Cannot update the ref '%s'.";
-
 				if (err)
-					strbuf_addf(err, str, update->refname);
+					strbuf_addf(err, "Cannot update the "
+						    "ref '%s'.",
+						    update->refname);
 				goto cleanup;
 			}
 		}

@@ -3614,8 +3616,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	clear_loose_ref_cache(&ref_cache);
 
 cleanup:
-	transaction->state = ret ? REF_TRANSACTION_ERROR
-		: REF_TRANSACTION_CLOSED;
+	transaction->state = REF_TRANSACTION_CLOSED;
 
 	for (i = 0; i < n; i++)
 		if (updates[i]->lock)

index aad846c..69ef28c 100644
@@ -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,

index cf17c69..5e93b6a 100644
@@ -296,6 +296,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
 	}
 
 	strbuf_release(&sb);
+	strbuf_release(&err);
 	ref_transaction_free(transaction);
 	return 0;
 }

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'?

Thanks.

Hi again,

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(-)

[...]

[...]

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

[...]

Ahhh. Thanks for explaining.

Jonathan

And here are the patches.

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:26:44 -0700

Do basic error checking in ref_transaction_create() and make it return non-zero on error. Update all callers to check the result of ref_transaction_create(). There are currently no conditions in _create that will return error but there will be in the future. Add an err argument that will be updated on failure.

---
 builtin/update-ref.c |  4 +++-
 refs.c               | 18 ++++++++++++------
 refs.h               | 48 +++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 56 insertions(+), 14 deletions(-)

index 3067b11..41121fa 100644
@@ -226,7 +226,9 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)
 	if (*next != line_termination)
 		die("create %s: extra input: %s", refname, next);
 
-	ref_transaction_create(transaction, refname, new_sha1, update_flags);
+	if (ref_transaction_create(transaction, refname, new_sha1,
+				   update_flags, &err))
+		die("%s", err.buf);
 
 	update_flags = 0;
 	free(refname);

index 3f05e88..c49f1c6 100644
@@ -3449,18 +3449,24 @@ int ref_transaction_update(struct ref_transaction *transaction,
 	return 0;
 }
 
-void ref_transaction_create(struct ref_transaction *transaction,
-			    const char *refname,
-			    const unsigned char *new_sha1,
-			    int flags)
+int ref_transaction_create(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *new_sha1,
+			   int flags,
+			   struct strbuf *err)
 {
-	struct ref_update *update = add_update(transaction, refname);
+	struct ref_update *update;
+
+	if (!new_sha1 || is_null_sha1(new_sha1))
+		die("BUG: create ref with null new_sha1");
+
+	update = add_update(transaction, refname);
 
-	assert(!is_null_sha1(new_sha1));
 	hashcpy(update->new_sha1, new_sha1);
 	hashclr(update->old_sha1);
 	update->flags = flags;
 	update->have_old = 1;
+	return 0;
 }
 
 void ref_transaction_delete(struct ref_transaction *transaction,

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:27:45 -0700

Change ref_transaction_delete() to do basic error checking and return non-zero of error. Update all callers to check the return for ref_transaction_delete(). There are currently no conditions in _delete that will return error but there will be in the future. Add an err argument that will be updated on failure.

---
 builtin/update-ref.c |  5 +++--
 refs.c               | 16 +++++++++++-----
 refs.h               | 12 ++++++++----
 3 files changed, 22 insertions(+), 11 deletions(-)

index 41121fa..7c9c248 100644
@@ -258,8 +258,9 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)
 	if (*next != line_termination)
 		die("delete %s: extra input: %s", refname, next);
 
-	ref_transaction_delete(transaction, refname, old_sha1,
-			       update_flags, have_old);
+	if (ref_transaction_delete(transaction, refname, old_sha1,
+				   update_flags, have_old, &err))
+		die("%s", err.buf);
 
 	update_flags = 0;
 	free(refname);

index c49f1c6..40f04f4 100644
@@ -3469,19 +3469,25 @@ int ref_transaction_create(struct ref_transaction *transaction,
 	return 0;
 }
 
-void ref_transaction_delete(struct ref_transaction *transaction,
-			    const char *refname,
-			    const unsigned char *old_sha1,
-			    int flags, int have_old)
+int ref_transaction_delete(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *old_sha1,
+			   int flags, int have_old,
+			   struct strbuf *err)
 {
-	struct ref_update *update = add_update(transaction, refname);
+	struct ref_update *update;
 
+	if (have_old && !old_sha1)
+		die("BUG: have_old is true but old_sha1 is NULL");
+
+	update = add_update(transaction, refname);
 	update->flags = flags;
 	update->have_old = have_old;
 	if (have_old) {
 		assert(!is_null_sha1(old_sha1));
 		hashcpy(update->old_sha1, old_sha1);
 	}
+	return 0;
 }
 
 int update_ref(const char *action, const char *refname,

index b648819..71389a1 100644
@@ -308,11 +308,15 @@ int ref_transaction_create(struct ref_transaction *transaction,
  * Add a reference deletion to transaction.  If have_old is true, then
  * old_sha1 holds the value that the reference should have had before
  * the update (which must not be the null SHA-1).
+ * Function returns 0 on success and non-zero on failure. A failure to delete
+ * means that the transaction as a whole has failed and will need to be
+ * rolled back.
  */
-void ref_transaction_delete(struct ref_transaction *transaction,
-			    const char *refname,
-			    const unsigned char *old_sha1,
-			    int flags, int have_old);
+int ref_transaction_delete(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *old_sha1,
+			   int flags, int have_old,
+			   struct strbuf *err);
 
 /*
  * Commit all of the changes that have been queued in transaction, as

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Mon, 19 May 2014 10:42:34 -0700

Add an err argument to _begin so that on non-fatal failures in future ref backends we can report a nice error back to the caller.
While _begin can currently never fail for other reasons than OOM, in which case we die() anyway, we may add other types of backends in the future. For example, a hypothetical MySQL backend could fail in _begin with "Can not connect to MySQL server. No route to host".

---
 builtin/update-ref.c | 5 ++++-
 refs.c               | 2 +-
 refs.h               | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

index 7c9c248..96a53b9 100644
@@ -365,7 +365,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		die("Refusing to perform update with empty message.");
 
 	if (read_stdin) {
-		transaction = ref_transaction_begin();
+		transaction = ref_transaction_begin(&err);
+		if (!transaction)
+			die("%s", err.buf);
 		if (delete || no_deref || argc > 0)
 			usage_with_options(git_update_ref_usage, options);
 		if (end_null)

@@ -374,6 +376,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		if (ref_transaction_commit(transaction, msg, &err))
 			die("%s", err.buf);
 		ref_transaction_free(transaction);
+		strbuf_release(&err);
 		return 0;
 	}
 

index 40f04f4..9cb7908 100644
@@ -3397,7 +3397,7 @@ struct ref_transaction {
 	size_t nr;
 };
 
-struct ref_transaction *ref_transaction_begin(void)
+struct ref_transaction *ref_transaction_begin(struct strbuf *err)
 {
 	return xcalloc(1, sizeof(struct ref_transaction));
 }

index 71389a1..3f37c65 100644
@@ -262,7 +262,7 @@ enum action_on_err {
  * Begin a reference transaction.  The reference transaction must
  * be freed by calling ref_transaction_free().
  */
-struct ref_transaction *ref_transaction_begin(void);
+struct ref_transaction *ref_transaction_begin(struct strbuf *err);
 
 /*
  * The following functions add a reference check or update to a

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Tue, 29 Apr 2014 12:06:19 -0700

Track the state of a transaction in a new state field. Check the field for sanity, i.e. that state must be OPEN when _commit/_create/_delete or _update is called or else die(BUG:...)

---
 refs.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

index 9cb7908..cc63056 100644
@@ -3387,6 +3387,21 @@ struct ref_update {
 };
 
 /*
+ * Transaction states.
+ * OPEN:   The transaction is in a valid state and can accept new updates.
+ *         An OPEN transaction can be committed.
+ * CLOSED: A closed transaction is no longer active and no other operations
+ *         than free can be used on it in this state.
+ *         A transaction can either become closed by successfully committing
+ *         an active transaction or if there is a failure while building
+ *         the transaction thus rendering it failed/inactive.
+ */
+enum ref_transaction_state {
+	REF_TRANSACTION_OPEN   = 0,
+	REF_TRANSACTION_CLOSED = 1
+};
+
+/*
  * Data structure for holding a reference transaction, which can
  * consist of checks and updates to multiple references, carried out
  * as atomically as possible.  This structure is opaque to callers.

@@ -3395,6 +3410,7 @@ struct ref_transaction {
 	struct ref_update **updates;
 	size_t alloc;
 	size_t nr;
+	enum ref_transaction_state state;
 };
 
 struct ref_transaction *ref_transaction_begin(struct strbuf *err)

@@ -3437,6 +3453,9 @@ int ref_transaction_update(struct ref_transaction *transaction,
 {
 	struct ref_update *update;
 
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: update called for transaction that is not open");
+
 	if (have_old && !old_sha1)
 		die("BUG: have_old is true but old_sha1 is NULL");
 

@@ -3457,6 +3476,9 @@ int ref_transaction_create(struct ref_transaction *transaction,
 {
 	struct ref_update *update;
 
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: create called for transaction that is not open");
+
 	if (!new_sha1 || is_null_sha1(new_sha1))
 		die("BUG: create ref with null new_sha1");
 

@@ -3477,6 +3499,9 @@ int ref_transaction_delete(struct ref_transaction *transaction,
 {
 	struct ref_update *update;
 
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: delete called for transaction that is not open");
+
 	if (have_old && !old_sha1)
 		die("BUG: have_old is true but old_sha1 is NULL");
 

@@ -3532,8 +3557,13 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	int n = transaction->nr;
 	struct ref_update **updates = transaction->updates;
 
-	if (!n)
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: commit called for transaction that is not open");
+
+	if (!n) {
+		transaction->state = REF_TRANSACTION_CLOSED;
 		return 0;
+	}
 
 	/* Allocate work space */
 	delnames = xmalloc(sizeof(*delnames) * n);

@@ -3595,6 +3625,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	clear_loose_ref_cache(&ref_cache);
 
 cleanup:
+	transaction->state = REF_TRANSACTION_CLOSED;
+
 	for (i = 0; i < n; i++)
 		if (updates[i]->lock)
 			unlock_ref(updates[i]->lock);

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:30:41 -0700

Change tag.c to use ref transactions for all ref updates.

---
 builtin/tag.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index c6e8a71..f3f172f 100644
@@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct strbuf ref = STRBUF_INIT;
 	unsigned char object[20], prev[20];
 	const char *object_ref, *tag;
-	struct ref_lock *lock;
 	struct create_tag_options opt;
 	char *cleanup_arg = NULL;
 	int annotate = 0, force = 0, lines = -1;

@@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	const char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct commit_list *with_commit = NULL;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
 	struct option options[] = {
 		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
 		{ OPTION_INTEGER, 'n', NULL, &lines, N_("n"),

@@ -701,14 +702,17 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (annotate)
 		create_tag(object, tag, &buf, &opt, prev, object);
 
-	lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
-	if (!lock)
-		die(_("%s: cannot lock the ref"), ref.buf);
-	if (write_ref_sha1(lock, object, NULL) < 0)
-		die(_("%s: cannot update the ref"), ref.buf);
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, ref.buf, object, prev,
+				   0, 1, &err) ||
+	    ref_transaction_commit(transaction, NULL, &err))
+		die("%s", err.buf);
+	ref_transaction_free(transaction);
 	if (force && !is_null_sha1(prev) && hashcmp(prev, object))
 		printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV));
 
+	strbuf_release(&err);
 	strbuf_release(&buf);
 	strbuf_release(&ref);
 	return 0;

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:32:29 -0700

Update replace.c to use ref transactions for updates.

---
 builtin/replace.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

index 1bb491d..1fcd06d 100644
@@ -153,7 +153,8 @@ static int replace_object_sha1(const char *object_ref,
 	unsigned char prev[20];
 	enum object_type obj_type, repl_type;
 	char ref[PATH_MAX];
-	struct ref_lock *lock;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
 
 	obj_type = sha1_object_info(object, NULL);
 	repl_type = sha1_object_info(repl, NULL);

@@ -166,12 +167,13 @@ static int replace_object_sha1(const char *object_ref,
 
 	check_ref_valid(object, prev, ref, sizeof(ref), force);
 
-	lock = lock_any_ref_for_update(ref, prev, 0, NULL);
-	if (!lock)
-		die("%s: cannot lock the ref", ref);
-	if (write_ref_sha1(lock, repl, NULL) < 0)
-		die("%s: cannot update the ref", ref);
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, ref, repl, prev, 0, 1, &err) ||
+	    ref_transaction_commit(transaction, NULL, &err))
+		die("%s", err.buf);
 
+	ref_transaction_free(transaction);
 	return 0;
 }
 

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:34:19 -0700

Change commit.c to use ref transactions for all ref updates. Make sure we pass a NULL pointer to ref_transaction_update if have_old is false.

---
 builtin/commit.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

index 5e2221c..9bf1003 100644
@@ -1627,11 +1627,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	const char *index_file, *reflog_msg;
 	char *nl;
 	unsigned char sha1[20];
-	struct ref_lock *ref_lock;
 	struct commit_list *parents = NULL, **pptr = &parents;
 	struct stat statbuf;
 	struct commit *current_head = NULL;
 	struct commit_extra_header *extra = NULL;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(builtin_commit_usage, builtin_commit_options);

@@ -1753,16 +1754,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	strbuf_release(&author_ident);
 	free_commit_extra_headers(extra);
 
-	ref_lock = lock_any_ref_for_update("HEAD",
-					   !current_head
-					   ? NULL
-					   : current_head->object.sha1,
-					   0, NULL);
-	if (!ref_lock) {
-		rollback_index_files();
-		die(_("cannot lock HEAD ref"));
-	}
-
 	nl = strchr(sb.buf, '\n');
 	if (nl)
 		strbuf_setlen(&sb, nl + 1 - sb.buf);

@@ -1771,10 +1762,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));
 	strbuf_insert(&sb, strlen(reflog_msg), ": ", 2);
 
-	if (write_ref_sha1(ref_lock, sha1, sb.buf) < 0) {
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, "HEAD", sha1,
+				   current_head
+				   ? current_head->object.sha1 : NULL,
+				   0, !!current_head, &err) ||
+	    ref_transaction_commit(transaction, sb.buf, &err)) {
 		rollback_index_files();
-		die(_("cannot update HEAD ref"));
+		die("%s", err.buf);
 	}
+	ref_transaction_free(transaction);
 
 	unlink(git_path("CHERRY_PICK_HEAD"));
 	unlink(git_path("REVERT_HEAD"));

@@ -1803,5 +1801,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (!quiet)
 		print_summary(prefix, sha1, !current_head);
 
+	strbuf_release(&err);
 	return 0;
 }

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:37:45 -0700

Change to use ref transactions for all updates to refs.

---
 sequencer.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

index 0a80c58..5e93b6a 100644
@@ -272,23 +272,33 @@ static int error_dirty_index(struct replay_opts *opts)
 static int fast_forward_to(const unsigned char *to, const unsigned char *from,
 			int unborn, struct replay_opts *opts)
 {
-	struct ref_lock *ref_lock;
+	struct ref_transaction *transaction;
 	struct strbuf sb = STRBUF_INIT;
-	int ret;
+	struct strbuf err = STRBUF_INIT;
 
 	read_cache();
 	if (checkout_fast_forward(from, to, 1))
 		exit(1); /* the callee should have complained already */
-	ref_lock = lock_any_ref_for_update("HEAD", unborn ? null_sha1 : from,
-					   0, NULL);
-	if (!ref_lock)
-		return error(_("Failed to lock HEAD during fast_forward_to"));
 
 	strbuf_addf(&sb, "%s: fast-forward", action_name(opts));
-	ret = write_ref_sha1(ref_lock, to, sb.buf);
+
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, "HEAD",
+				   to, unborn ? null_sha1 : from,
+				   0, 1, &err) ||
+	    ref_transaction_commit(transaction, sb.buf, &err)) {
+		ref_transaction_free(transaction);
+		error("%s", err.buf);
+		strbuf_release(&sb);
+		strbuf_release(&err);
+		return -1;
+	}
 
 	strbuf_release(&sb);
-	return ret;
+	strbuf_release(&err);
+	ref_transaction_free(transaction);
+	return 0;
 }
 
 static int do_recursive_merge(struct commit *base, struct commit *next,

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 16:21:13 -0700

Change update_branch() to use ref transactions for updates.

---
 fast-import.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

index 6707a66..79160d5 100644
@@ -1679,8 +1679,9 @@ found_entry:
 static int update_branch(struct branch *b)
 {
 	static const char *msg = "fast-import";
-	struct ref_lock *lock;
+	struct ref_transaction *transaction;
 	unsigned char old_sha1[20];
+	struct strbuf err = STRBUF_INIT;
 
 	if (read_ref(b->name, old_sha1))
 		hashclr(old_sha1);

@@ -1689,29 +1690,33 @@ static int update_branch(struct branch *b)
 			delete_ref(b->name, old_sha1, 0);
 		return 0;
 	}
-	lock = lock_any_ref_for_update(b->name, old_sha1, 0, NULL);
-	if (!lock)
-		return error("Unable to lock %s", b->name);
 	if (!force_update && !is_null_sha1(old_sha1)) {
 		struct commit *old_cmit, *new_cmit;
 
 		old_cmit = lookup_commit_reference_gently(old_sha1, 0);
 		new_cmit = lookup_commit_reference_gently(b->sha1, 0);
-		if (!old_cmit || !new_cmit) {
-			unlock_ref(lock);
+		if (!old_cmit || !new_cmit)
 			return error("Branch %s is missing commits.", b->name);
-		}
 
 		if (!in_merge_bases(old_cmit, new_cmit)) {
-			unlock_ref(lock);
 			warning("Not updating %s"
 				" (new tip %s does not contain %s)",
 				b->name, sha1_to_hex(b->sha1), sha1_to_hex(old_sha1));
 			return -1;
 		}
 	}
-	if (write_ref_sha1(lock, b->sha1, msg) < 0)
-		return error("Unable to update %s", b->name);
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, b->name, b->sha1, old_sha1,
+				   0, 1, &err) ||
+	    ref_transaction_commit(transaction, msg, &err)) {
+		ref_transaction_free(transaction);
+		error("%s", err.buf);
+		strbuf_release(&err);
+		return -1;
+	}
+	ref_transaction_free(transaction);
+	strbuf_release(&err);
 	return 0;
 }
 

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 16:21:53 -0700

Change create_branch to use a ref transaction when creating the new branch.

This also fixes a race condition in the old code where two concurrent create_branch could race since the lock_any_ref_for_update/write_ref_sha1 did not protect against the ref already existing. I.e. one thread could end up overwriting a branch even if the forcing flag is false.

---
 branch.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

index 660097b..37ac555 100644
@@ -226,7 +226,6 @@ void create_branch(const char *head,
 		   int force, int reflog, int clobber_head,
 		   int quiet, enum branch_track track)
 {
-	struct ref_lock *lock = NULL;
 	struct commit *commit;
 	unsigned char sha1[20];
 	char *real_ref, msg[PATH_MAX + 20];

@@ -285,15 +284,6 @@ void create_branch(const char *head,
 		die(_("Not a valid branch point: '%s'."), start_name);
 	hashcpy(sha1, commit->object.sha1);
 
-	if (!dont_change_ref) {
-		lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
-		if (!lock)
-			die_errno(_("Failed to lock ref for update"));
-	}
-
-	if (reflog)
-		log_all_ref_updates = 1;
-
 	if (forcing)
 		snprintf(msg, sizeof msg, "branch: Reset to %s",
 			 start_name);

@@ -301,13 +291,26 @@ void create_branch(const char *head,
 		snprintf(msg, sizeof msg, "branch: Created from %s",
 			 start_name);
 
+	if (reflog)
+		log_all_ref_updates = 1;
+
+	if (!dont_change_ref) {
+		struct ref_transaction *transaction;
+		struct strbuf err = STRBUF_INIT;
+
+		transaction = ref_transaction_begin(&err);
+		if (!transaction ||
+		    ref_transaction_update(transaction, ref.buf, sha1,
+					   null_sha1, 0, !forcing, &err) ||
+		    ref_transaction_commit(transaction, msg, &err))
+			die("%s", err.buf);
+		ref_transaction_free(transaction);
+		strbuf_release(&err);
+	}
+
 	if (real_ref && track)
 		setup_tracking(ref.buf + 11, real_ref, track, quiet);
 
-	if (!dont_change_ref)
-		if (write_ref_sha1(lock, sha1, msg) < 0)
-			die_errno(_("Failed to write ref"));
-
 	strbuf_release(&ref);
 	free(real_ref);
 }

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Thu, 24 Apr 2014 16:36:55 -0700

Change the update_ref helper function to use a ref transaction internally.

---
 refs.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

index cc63056..dcc877b 100644
@@ -3519,11 +3519,32 @@ int update_ref(const char *action, const char *refname,
 	       const unsigned char *sha1, const unsigned char *oldval,
 	       int flags, enum action_on_err onerr)
 {
-	struct ref_lock *lock;
-	lock = update_ref_lock(refname, oldval, flags, NULL, onerr);
-	if (!lock)
+	struct ref_transaction *t;
+	struct strbuf err = STRBUF_INIT;
+
+	t = ref_transaction_begin(&err);
+	if (!t ||
+	    ref_transaction_update(t, refname, sha1, oldval, flags,
+				   !!oldval, &err) ||
+	    ref_transaction_commit(t, action, &err)) {
+		const char *str = "update_ref failed for ref '%s': %s";
+
+		ref_transaction_free(t);
+		switch (onerr) {
+		case UPDATE_REFS_MSG_ON_ERR:
+			error(str, refname, err.buf);
+			break;
+		case UPDATE_REFS_DIE_ON_ERR:
+			die(str, refname, err.buf);
+			break;
+		case UPDATE_REFS_QUIET_ON_ERR:
+			break;
+		}
+		strbuf_release(&err);
 		return 1;
-	return update_ref_write(action, refname, sha1, lock, NULL, onerr);
+	}
+	strbuf_release(&err);
+	return 0;
 }
 
 static int ref_update_compare(const void *r1, const void *r2)

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Mon, 28 Apr 2014 14:36:15 -0700

Wrap all the ref updates inside a transaction.

In the new API there is no distinction between failure to lock and failure to write a ref. Both can be permanent (e.g., a ref "refs/heads/topic" is blocking creation of the lock file "refs/heads/topic/1.lock") or transient (e.g., file system full) and there's no clear difference in how the client should respond, so replace the two statuses "failed to lock" and "failed to write" with a single status "failed to update ref". In both cases a more detailed message is sent by sideband to diagnose the problem.

Example, before:

error: there are still refs under 'refs/heads/topic'
remote: error: failed to lock refs/heads/topic To foo
! [remote rejected] HEAD -> topic (failed to lock)

After:

error: there are still refs under 'refs/heads/topic'
remote: error: Cannot lock the ref 'refs/heads/topic'. To foo
! [remote rejected] HEAD -> topic (failed to update ref)

---
 builtin/receive-pack.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

index c323081..224fadc 100644
@@ -475,7 +475,6 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 	const char *namespaced_name;
 	unsigned char *old_sha1 = cmd->old_sha1;
 	unsigned char *new_sha1 = cmd->new_sha1;
-	struct ref_lock *lock;
 
 	/* only refs/... are allowed */
 	if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {

@@ -576,19 +575,27 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 		return NULL; /* good */
 	}
 	else {
+		struct strbuf err = STRBUF_INIT;
+		struct ref_transaction *transaction;
+
 		if (shallow_update && si->shallow_ref[cmd->index] &&
 		    update_shallow_ref(cmd, si))
 			return "shallow error";
 
-		lock = lock_any_ref_for_update(namespaced_name, old_sha1,
-					       0, NULL);
-		if (!lock) {
-			rp_error("failed to lock %s", name);
-			return "failed to lock";
-		}
-		if (write_ref_sha1(lock, new_sha1, "push")) {
-			return "failed to write"; /* error() already called */
+		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)) {
+			ref_transaction_free(transaction);
+
+			rp_error("%s", err.buf);
+			strbuf_release(&err);
+			return "failed to update ref";
 		}
+
+		ref_transaction_free(transaction);
+		strbuf_release(&err);
 		return NULL; /* good */
 	}
 }

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Mon, 28 Apr 2014 15:23:58 -0700

---
 fast-import.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

index 79160d5..e7f6e37 100644
@@ -1735,15 +1735,32 @@ static void dump_tags(void)
 {
 	static const char *msg = "fast-import";
 	struct tag *t;
-	struct ref_lock *lock;
-	char ref_name[PATH_MAX];
+	struct strbuf ref_name = STRBUF_INIT;
+	struct strbuf err = STRBUF_INIT;
+	struct ref_transaction *transaction;
 
+	transaction = ref_transaction_begin(&err);
+	if (!transaction) {
+		failure |= error("%s", err.buf);
+		goto cleanup;
+	}
 	for (t = first_tag; t; t = t->next_tag) {
-		sprintf(ref_name, "tags/%s", t->name);
-		lock = lock_ref_sha1(ref_name, NULL);
-		if (!lock || write_ref_sha1(lock, t->sha1, msg) < 0)
-			failure |= error("Unable to update %s", ref_name);
+		strbuf_reset(&ref_name);
+		strbuf_addf(&ref_name, "refs/tags/%s", t->name);
+
+		if (ref_transaction_update(transaction, ref_name.buf, t->sha1,
+					   NULL, 0, 0, &err)) {
+			failure |= error("%s", err.buf);
+			goto cleanup;
+		}
 	}
+	if (ref_transaction_commit(transaction, msg, &err))
+		failure |= error("%s", err.buf);
+
+ cleanup:
+	ref_transaction_free(transaction);
+	strbuf_release(&ref_name);
+	strbuf_release(&err);
 }
 
 static void dump_marks_helper(FILE *f,

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Thu, 17 Apr 2014 11:31:06 -0700

Switch to using ref transactions in walker_fetch(). As part of the refactoring to use ref transactions we also fix a potential memory leak where in the original code if write_ref_sha1() would fail we would end up returning from the function without free()ing the msg string.

Note that this function is only called when fetching from a remote HTTP repository onto the local (most of the time single-user) repository which likely means that the type of collisions that the previous locking would protect against and cause the fetch to fail for are even more rare.

---
 walker.c | 72 ++++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 41 insertions(+), 31 deletions(-)

index 1dd86b8..b8a5441 100644
@@ -251,40 +251,40 @@ void walker_targets_free(int targets, char **target, const char **write_ref)
 int walker_fetch(struct walker *walker, int targets, char **target,
 		 const char **write_ref, const char *write_ref_log_details)
 {
-	struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));
+	struct strbuf refname = STRBUF_INIT;
+	struct strbuf err = STRBUF_INIT;
+	struct ref_transaction *transaction = NULL;
 	unsigned char *sha1 = xmalloc(targets * 20);
-	char *msg;
-	int ret;
-	int i;
+	char *msg = NULL;
+	int i, ret = -1;
 
 	save_commit_buffer = 0;
 
-	for (i = 0; i < targets; i++) {
-		if (!write_ref || !write_ref[i])
-			continue;
-
-		lock[i] = lock_ref_sha1(write_ref[i], NULL);
-		if (!lock[i]) {
-			error("Can't lock ref %s", write_ref[i]);
-			goto unlock_and_fail;
+	if (write_ref) {
+		transaction = ref_transaction_begin(&err);
+		if (!transaction) {
+			error("%s", err.buf);
+			goto done;
 		}
 	}
-
 	if (!walker->get_recover)
 		for_each_ref(mark_complete, NULL);
 
 	for (i = 0; i < targets; i++) {
 		if (interpret_target(walker, target[i], &sha1[20 * i])) {
 			error("Could not interpret response from server '%s' as something to pull", target[i]);
-			goto unlock_and_fail;
+			goto done;
 		}
 		if (process(walker, lookup_unknown_object(&sha1[20 * i])))
-			goto unlock_and_fail;
+			goto done;
 	}
 
 	if (loop(walker))
-		goto unlock_and_fail;
-
+		goto done;
+	if (!write_ref) {
+		ret = 0;
+		goto done;
+	}
 	if (write_ref_log_details) {
 		msg = xmalloc(strlen(write_ref_log_details) + 12);
 		sprintf(msg, "fetch from %s", write_ref_log_details);

@@ -292,23 +292,33 @@ int walker_fetch(struct walker *walker, int targets, char **target,
 		msg = NULL;
 	}
 	for (i = 0; i < targets; i++) {
-		if (!write_ref || !write_ref[i])
+		if (!write_ref[i])
 			continue;
-		ret = write_ref_sha1(lock[i], &sha1[20 * i], msg ? msg : "fetch (unknown)");
-		lock[i] = NULL;
-		if (ret)
-			goto unlock_and_fail;
+		strbuf_reset(&refname);
+		strbuf_addf(&refname, "refs/%s", write_ref[i]);
+		if (ref_transaction_update(transaction, refname.buf,
+					   &sha1[20 * i], NULL, 0, 0,
+					   &err)) {
+			error("%s", err.buf);
+			goto done;
+		}
 	}
+	if (ref_transaction_commit(transaction,
+				   msg ? msg : "fetch (unknown)",
+				   &err)) {
+		error("%s", err.buf);
+		goto done;
+	}
+
+	ret = 0;
+
+done:
+	ref_transaction_free(transaction);
 	free(msg);
-
-	return 0;
-
-unlock_and_fail:
-	for (i = 0; i < targets; i++)
-		if (lock[i])
-			unlock_ref(lock[i]);
-
-	return -1;
+	free(sha1);
+	strbuf_release(&err);
+	strbuf_release(&refname);
+	return ret;
 }
 
 void walker_free(struct walker *walker)

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Mon, 28 Apr 2014 15:38:47 -0700

No external callers reference lock_ref_sha1 any more so let's declare it static.

---
 refs.c | 7 +++++--
 refs.h | 6 ------
 2 files changed, 5 insertions(+), 8 deletions(-)

index dcc877b..0dc093c 100644
@@ -2069,7 +2069,10 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 	return logs_found;
 }
 
-/* This function should make sure errno is meaningful on error */
+/*
+ * Locks a "refs/" ref returning the lock on success and NULL on failure.
+ * On failure errno is set to something meaningful.
+ */
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 					    const unsigned char *old_sha1,
 					    int flags, int *type_p)

@@ -2170,7 +2173,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 	return NULL;
 }
 
-struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1)
+static struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1)
 {
 	char refpath[PATH_MAX];
 	if (check_refname_format(refname, 0))

index 3f37c65..65dd593 100644
@@ -170,12 +170,6 @@ extern int ref_exists(const char *);
  */
 extern int peel_ref(const char *refname, unsigned char *sha1);
 
-/*
- * Locks a "refs/" ref returning the lock on success and NULL on failure.
- * On failure errno is set to something meaningful.
- */
-extern struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1);
-
 /** Locks any ref (for 'HEAD' type refs). */
 #define REF_NODEREF	0x01
 /* errno is set to something meaningful on failure */

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Tue, 29 Apr 2014 12:14:47 -0700

Since we now only call update_ref_lock with onerr==QUIET_ON_ERR we no longer need this function and can replace it with just calling lock_any_ref_for_update directly.

---
 refs.c | 30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

index 0dc093c..de07791 100644
@@ -3336,24 +3336,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
 	return retval;
 }
 
-static struct ref_lock *update_ref_lock(const char *refname,
-					const unsigned char *oldval,
-					int flags, int *type_p,
-					enum action_on_err onerr)
-{
-	struct ref_lock *lock;
-	lock = lock_any_ref_for_update(refname, oldval, flags, type_p);
-	if (!lock) {
-		const char *str = "Cannot lock the ref '%s'.";
-		switch (onerr) {
-		case UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;
-		case UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;
-		case UPDATE_REFS_QUIET_ON_ERR: break;
-		}
-	}
-	return lock;
-}
-
 static int update_ref_write(const char *action, const char *refname,
 			    const unsigned char *sha1, struct ref_lock *lock,
 			    struct strbuf *err, enum action_on_err onerr)

@@ -3602,12 +3584,12 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	for (i = 0; i < n; i++) {
 		struct ref_update *update = updates[i];
 
-		update->lock = update_ref_lock(update->refname,
-					       (update->have_old ?
-						update->old_sha1 : NULL),
-					       update->flags,
-					       &update->type,
-					       UPDATE_REFS_QUIET_ON_ERR);
+		update->lock = lock_any_ref_for_update(update->refname,
+						       (update->have_old ?
+							update->old_sha1 :
+							NULL),
+						       update->flags,
+						       &update->type);
 		if (!update->lock) {
 			if (err)
 				strbuf_addf(err, "Cannot lock the ref '%s'.",

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Tue, 29 Apr 2014 13:42:07 -0700

Since we only call update_ref_write from a single place and we only call it with onerr==QUIET_ON_ERR we can just as well get rid of it and just call write_ref_sha1 directly. This changes the return status for _commit from 1 to -1 on failures when writing to the ref. Eventually we will want _commit to start returning more detailed error conditions than the current simple success/failure.

---
 refs.c | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

index de07791..ef7660a 100644
@@ -3336,25 +3336,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
 	return retval;
 }
 
-static int update_ref_write(const char *action, const char *refname,
-			    const unsigned char *sha1, struct ref_lock *lock,
-			    struct strbuf *err, enum action_on_err onerr)
-{
-	if (write_ref_sha1(lock, sha1, action) < 0) {
-		const char *str = "Cannot update the ref '%s'.";
-		if (err)
-			strbuf_addf(err, str, refname);
-
-		switch (onerr) {
-		case UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;
-		case UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;
-		case UPDATE_REFS_QUIET_ON_ERR: break;
-		}
-		return 1;
-	}
-	return 0;
-}
-
 /**
  * Information needed for a single ref update.  Set new_sha1 to the
  * new value or to zero to delete the ref.  To check the old value

@@ -3604,14 +3585,15 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 		struct ref_update *update = updates[i];
 
 		if (!is_null_sha1(update->new_sha1)) {
-			ret = update_ref_write(msg,
-					       update->refname,
-					       update->new_sha1,
-					       update->lock, err,
-					       UPDATE_REFS_QUIET_ON_ERR);
-			update->lock = NULL; /* freed by update_ref_write */
-			if (ret)
+			ret = write_ref_sha1(update->lock, update->new_sha1,
+					     msg);
+			update->lock = NULL; /* freed by write_ref_sha1 */
+			if (ret) {
+				if (err)
+					strbuf_addf(err, "Cannot update the ref '%s'.",
+						    update->refname);
 				goto cleanup;
+			}
 		}
 	}
 

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Tue, 29 Apr 2014 15:45:52 -0700

lock_ref_sha1 was only called from one place in refs.c and only provided a check that the refname was sane before adding back the initial "refs/" part of the ref path name, the initial "refs/" that this caller had already stripped off before calling lock_ref_sha1.

---
 refs.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

index ef7660a..f0883d0 100644
@@ -2173,15 +2173,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 	return NULL;
 }
 
-static struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1)
-{
-	char refpath[PATH_MAX];
-	if (check_refname_format(refname, 0))
-		return NULL;
-	strcpy(refpath, mkpath("refs/%s", refname));
-	return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
-}
-
 struct ref_lock *lock_any_ref_for_update(const char *refname,
 					 const unsigned char *old_sha1,
 					 int flags, int *type_p)

@@ -2391,8 +2382,12 @@ static void try_remove_empty_parents(char *name)
 /* make sure nobody touched the ref, and unlink */
 static void prune_ref(struct ref_to_prune *r)
 {
-	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
+	struct ref_lock *lock;
 
+	if (check_refname_format(r->name + 5, 0))
+		return;
+
+	lock = lock_ref_sha1_basic(r->name, r->sha1, 0, NULL);
 	if (lock) {
 		unlink_or_warn(git_path("%s", r->name));
 		unlock_ref(lock);

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 30 Apr 2014 09:03:36 -0700

Change prune_ref to delete the ref using a ref transaction. To do this we also need to add a new flag REF_ISPRUNING that will tell the transaction that we do not want to delete this ref from the packed refs. This flag is private to refs.c and not exposed to external callers.

---
 refs.c | 28 +++++++++++++++++++++-------
 refs.h | 13 +++++++++++--
 2 files changed, 32 insertions(+), 9 deletions(-)

index f0883d0..5b2d335 100644
@@ -25,6 +25,11 @@ static unsigned char refname_disposition[256] = {
 };
 
 /*
+ * Used as a flag to ref_transaction_delete when a loose ref is being
+ * pruned.
+ */
+#define REF_ISPRUNING	0x0100
+/*
  * Try to read one refname component from the front of refname.
  * Return the length of the component found, or -1 if the component is
  * not legal.  It is legal if it is something reasonable to have under

@@ -2382,17 +2387,25 @@ static void try_remove_empty_parents(char *name)
 /* make sure nobody touched the ref, and unlink */
 static void prune_ref(struct ref_to_prune *r)
 {
-	struct ref_lock *lock;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
 
 	if (check_refname_format(r->name + 5, 0))
 		return;
 
-	lock = lock_ref_sha1_basic(r->name, r->sha1, 0, NULL);
-	if (lock) {
-		unlink_or_warn(git_path("%s", r->name));
-		unlock_ref(lock);
-		try_remove_empty_parents(r->name);
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_delete(transaction, r->name, r->sha1,
+				   REF_ISPRUNING, 1, &err) ||
+	    ref_transaction_commit(transaction, NULL, &err)) {
+		ref_transaction_free(transaction);
+		error("%s", err.buf);
+		strbuf_release(&err);
+		return;
 	}
+	ref_transaction_free(transaction);
+	strbuf_release(&err);
+	try_remove_empty_parents(r->name);
 }
 
 static void prune_refs(struct ref_to_prune *r)

@@ -3597,8 +3610,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 		struct ref_update *update = updates[i];
 
 		if (update->lock) {
-			delnames[delnum++] = update->lock->ref_name;
 			ret |= delete_ref_loose(update->lock, update->type);
+			if (!(update->flags & REF_ISPRUNING))
+				delnames[delnum++] = update->lock->ref_name;
 		}
 	}
 

index 65dd593..69ef28c 100644
@@ -170,9 +170,18 @@ extern int ref_exists(const char *);
  */
 extern int peel_ref(const char *refname, unsigned char *sha1);
 
-/** Locks any ref (for 'HEAD' type refs). */
+/*
+ * Flags controlling lock_any_ref_for_update(), ref_transaction_update(),
+ * ref_transaction_create(), etc.
+ * REF_NODEREF: act on the ref directly, instead of dereferencing
+ *              symbolic references.
+ *
+ * Flags >= 0x100 are reserved for internal use.
+ */
 #define REF_NODEREF	0x01
-/* errno is set 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,
 						int flags, int *type_p);

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 30 Apr 2014 09:22:45 -0700

Change delete_ref to use a ref transaction for the deletion. At the same time since we no longer have any callers of repack_without_ref we can now delete this function.

Change delete_ref to return 0 on success and 1 on failure instead of the previous 0 on success either 1 or -1 on failure.

---
Thanks for reading.

 refs.c | 35 ++++++++++++++---------------------
 1 file changed, 14 insertions(+), 21 deletions(-)

index 5b2d335..7996be9 100644
@@ -2548,11 +2548,6 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)
 	return ret;
 }
 
-static int repack_without_ref(const char *refname)
-{
-	return repack_without_refs(&refname, 1, NULL);
-}
-
 static int delete_ref_loose(struct ref_lock *lock, int flag)
 {
 	if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {

@@ -2570,24 +2565,22 @@ static int delete_ref_loose(struct ref_lock *lock, int flag)
 
 int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 {
-	struct ref_lock *lock;
-	int ret = 0, flag = 0;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
 
-	lock = lock_ref_sha1_basic(refname, sha1, delopt, &flag);
-	if (!lock)
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_delete(transaction, refname, sha1, delopt,
+				   sha1 && !is_null_sha1(sha1), &err) ||
+	    ref_transaction_commit(transaction, NULL, &err)) {
+		error("%s", err.buf);
+		ref_transaction_free(transaction);
+		strbuf_release(&err);
 		return 1;
-	ret |= delete_ref_loose(lock, flag);
-
-	/* removing the loose one could have resurrected an earlier
-	 * packed one.  Also, if it was not loose we need to repack
-	 * without it.
-	 */
-	ret |= repack_without_ref(lock->ref_name);
-
-	unlink_or_warn(git_path("logs/%s", lock->ref_name));
-	clear_loose_ref_cache(&ref_cache);
-	unlock_ref(lock);
-	return ret;
+	}
+	ref_transaction_free(transaction);
+	strbuf_release(&err);
+	return 0;
 }
 
 /*

2.1.0.rc2.206.gedb03e5

Thanks.

Will queue, but the some other topics may have to disappear from my tree while I try to rebase them on top (or while I wait for you guys to send a reroll, making it unnecesary to do the rebase myself).

I managed to rebase all the rs/ref-transaction* topics and pushed out the result in 'pu'. Please eyeball the result.

I had to bump nd/multiple-work-trees, which Duy asked to kick out of 'next' earlier, out of 'pu' because I did not want to deal with conflict resolution that will go to waste when the topic is rerolled which would happen soon.

Hi Junio,

Michael reviewed the rest of rs/ref-transaction-1 (thanks!) and some more small tweaks resulted --- in particular, there was a missing call to ref_transaction_free. Interdiff below. This is meant to replace rs/ref-transaction-1 in 'pu' and should be ready for 'next'.

These patches are also available from the git repository at

git://repo.or.cz/git/jrn.git tags/rs/ref-transaction-1

"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.

Jonathan Nieder (2):
update-ref --stdin: narrow scope of err strbuf update-ref --stdin: pass transaction around explicitly

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   |  52 +++++++----
 fast-import.c          |  54 +++++++----
 refs.c                 | 245 ++++++++++++++++++++++++++++---------------------
 refs.h                 |  77 ++++++++++++----
 sequencer.c            |  26 ++++--
 walker.c               |  70 ++++++++------
 11 files changed, 393 insertions(+), 242 deletions(-)

---
Changes since v21:

 builtin/update-ref.c | 38 +++++++++++++++++++++++++-------------
 refs.c               |  1 +
 2 files changed, 26 insertions(+), 13 deletions(-)

index 96a53b9..54a48c0 100644
@@ -12,11 +12,8 @@ static const char * const git_update_ref_usage[] = {
 	NULL
 };
 
-static struct ref_transaction *transaction;
-
 static char line_termination = '\n';
 static int update_flags;
-static struct strbuf err = STRBUF_INIT;
 
 /*
  * Parse one whitespace- or NUL-terminated, possibly C-quoted argument

@@ -177,8 +174,10 @@ static int parse_next_sha1(struct strbuf *input, const char **next,
  * depending on how line_termination is set.
  */
 
-static const char *parse_cmd_update(struct strbuf *input, const char *next)
+static const char *parse_cmd_update(struct ref_transaction *transaction,
+				    struct strbuf *input, const char *next)
 {
+	struct strbuf err = STRBUF_INIT;
 	char *refname;
 	unsigned char new_sha1[20];
 	unsigned char old_sha1[20];

@@ -204,12 +203,15 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)
 
 	update_flags = 0;
 	free(refname);
+	strbuf_release(&err);
 
 	return next;
 }
 
-static const char *parse_cmd_create(struct strbuf *input, const char *next)
+static const char *parse_cmd_create(struct ref_transaction *transaction,
+				    struct strbuf *input, const char *next)
 {
+	struct strbuf err = STRBUF_INIT;
 	char *refname;
 	unsigned char new_sha1[20];
 

@@ -232,12 +234,15 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)
 
 	update_flags = 0;
 	free(refname);
+	strbuf_release(&err);
 
 	return next;
 }
 
-static const char *parse_cmd_delete(struct strbuf *input, const char *next)
+static const char *parse_cmd_delete(struct ref_transaction *transaction,
+				    struct strbuf *input, const char *next)
 {
+	struct strbuf err = STRBUF_INIT;
 	char *refname;
 	unsigned char old_sha1[20];
 	int have_old;

@@ -264,12 +269,15 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)
 
 	update_flags = 0;
 	free(refname);
+	strbuf_release(&err);
 
 	return next;
 }
 
-static const char *parse_cmd_verify(struct strbuf *input, const char *next)
+static const char *parse_cmd_verify(struct ref_transaction *transaction,
+				    struct strbuf *input, const char *next)
 {
+	struct strbuf err = STRBUF_INIT;
 	char *refname;
 	unsigned char new_sha1[20];
 	unsigned char old_sha1[20];

@@ -297,6 +305,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)
 
 	update_flags = 0;
 	free(refname);
+	strbuf_release(&err);
 
 	return next;
 }

@@ -310,7 +319,7 @@ static const char *parse_cmd_option(struct strbuf *input, const char *next)
 	return next + 8;
 }
 
-static void update_refs_stdin(void)
+static void update_refs_stdin(struct ref_transaction *transaction)
 {
 	struct strbuf input = STRBUF_INIT;
 	const char *next;

@@ -325,13 +334,13 @@ static void update_refs_stdin(void)
 		else if (isspace(*next))
 			die("whitespace before command: %s", next);
 		else if (starts_with(next, "update "))
-			next = parse_cmd_update(&input, next + 7);
+			next = parse_cmd_update(transaction, &input, next + 7);
 		else if (starts_with(next, "create "))
-			next = parse_cmd_create(&input, next + 7);
+			next = parse_cmd_create(transaction, &input, next + 7);
 		else if (starts_with(next, "delete "))
-			next = parse_cmd_delete(&input, next + 7);
+			next = parse_cmd_delete(transaction, &input, next + 7);
 		else if (starts_with(next, "verify "))
-			next = parse_cmd_verify(&input, next + 7);
+			next = parse_cmd_verify(transaction, &input, next + 7);
 		else if (starts_with(next, "option "))
 			next = parse_cmd_option(&input, next + 7);
 		else

@@ -365,6 +374,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		die("Refusing to perform update with empty message.");
 
 	if (read_stdin) {
+		struct strbuf err = STRBUF_INIT;
+		struct ref_transaction *transaction;
+
 		transaction = ref_transaction_begin(&err);
 		if (!transaction)
 			die("%s", err.buf);

@@ -372,7 +384,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 			usage_with_options(git_update_ref_usage, options);
 		if (end_null)
 			line_termination = '\0';
-		update_refs_stdin();
+		update_refs_stdin(transaction);
 		if (ref_transaction_commit(transaction, msg, &err))
 			die("%s", err.buf);
 		ref_transaction_free(transaction);

index 7996be9..7235574 100644
@@ -3511,6 +3511,7 @@ int update_ref(const char *action, const char *refname,
 		return 1;
 	}
 	strbuf_release(&err);
+	ref_transaction_free(t);
 	return 0;
 }
 

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:26:44 -0700

Do basic error checking in ref_transaction_create() and make it return non-zero on error. Update all callers to check the result of ref_transaction_create(). There are currently no conditions in _create that will return error but there will be in the future. Add an err argument that will be updated on failure.

---
 builtin/update-ref.c |  4 +++-
 refs.c               | 18 ++++++++++++------
 refs.h               | 48 +++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 56 insertions(+), 14 deletions(-)

index 3067b11..41121fa 100644
@@ -226,7 +226,9 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)
 	if (*next != line_termination)
 		die("create %s: extra input: %s", refname, next);
 
-	ref_transaction_create(transaction, refname, new_sha1, update_flags);
+	if (ref_transaction_create(transaction, refname, new_sha1,
+				   update_flags, &err))
+		die("%s", err.buf);
 
 	update_flags = 0;
 	free(refname);

index 3f05e88..c49f1c6 100644
@@ -3449,18 +3449,24 @@ int ref_transaction_update(struct ref_transaction *transaction,
 	return 0;
 }
 
-void ref_transaction_create(struct ref_transaction *transaction,
-			    const char *refname,
-			    const unsigned char *new_sha1,
-			    int flags)
+int ref_transaction_create(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *new_sha1,
+			   int flags,
+			   struct strbuf *err)
 {
-	struct ref_update *update = add_update(transaction, refname);
+	struct ref_update *update;
+
+	if (!new_sha1 || is_null_sha1(new_sha1))
+		die("BUG: create ref with null new_sha1");
+
+	update = add_update(transaction, refname);
 
-	assert(!is_null_sha1(new_sha1));
 	hashcpy(update->new_sha1, new_sha1);
 	hashclr(update->old_sha1);
 	update->flags = flags;
 	update->have_old = 1;
+	return 0;
 }
 
 void ref_transaction_delete(struct ref_transaction *transaction,

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:27:45 -0700

Change ref_transaction_delete() to do basic error checking and return non-zero on error. Update all callers to check the return for ref_transaction_delete(). There are currently no conditions in _delete that will return error but there will be in the future. Add an err argument that will be updated on failure.

---
 builtin/update-ref.c |  5 +++--
 refs.c               | 16 +++++++++++-----
 refs.h               | 12 ++++++++----
 3 files changed, 22 insertions(+), 11 deletions(-)

index 41121fa..7c9c248 100644
@@ -258,8 +258,9 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)
 	if (*next != line_termination)
 		die("delete %s: extra input: %s", refname, next);
 
-	ref_transaction_delete(transaction, refname, old_sha1,
-			       update_flags, have_old);
+	if (ref_transaction_delete(transaction, refname, old_sha1,
+				   update_flags, have_old, &err))
+		die("%s", err.buf);
 
 	update_flags = 0;
 	free(refname);

index c49f1c6..40f04f4 100644
@@ -3469,19 +3469,25 @@ int ref_transaction_create(struct ref_transaction *transaction,
 	return 0;
 }
 
-void ref_transaction_delete(struct ref_transaction *transaction,
-			    const char *refname,
-			    const unsigned char *old_sha1,
-			    int flags, int have_old)
+int ref_transaction_delete(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *old_sha1,
+			   int flags, int have_old,
+			   struct strbuf *err)
 {
-	struct ref_update *update = add_update(transaction, refname);
+	struct ref_update *update;
 
+	if (have_old && !old_sha1)
+		die("BUG: have_old is true but old_sha1 is NULL");
+
+	update = add_update(transaction, refname);
 	update->flags = flags;
 	update->have_old = have_old;
 	if (have_old) {
 		assert(!is_null_sha1(old_sha1));
 		hashcpy(update->old_sha1, old_sha1);
 	}
+	return 0;
 }
 
 int update_ref(const char *action, const char *refname,

index b648819..71389a1 100644
@@ -308,11 +308,15 @@ int ref_transaction_create(struct ref_transaction *transaction,
  * Add a reference deletion to transaction.  If have_old is true, then
  * old_sha1 holds the value that the reference should have had before
  * the update (which must not be the null SHA-1).
+ * Function returns 0 on success and non-zero on failure. A failure to delete
+ * means that the transaction as a whole has failed and will need to be
+ * rolled back.
  */
-void ref_transaction_delete(struct ref_transaction *transaction,
-			    const char *refname,
-			    const unsigned char *old_sha1,
-			    int flags, int have_old);
+int ref_transaction_delete(struct ref_transaction *transaction,
+			   const char *refname,
+			   const unsigned char *old_sha1,
+			   int flags, int have_old,
+			   struct strbuf *err);
 
 /*
  * Commit all of the changes that have been queued in transaction, as

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Mon, 19 May 2014 10:42:34 -0700

Add an err argument to _begin so that on non-fatal failures in future ref backends we can report a nice error back to the caller.
While _begin can currently never fail for other reasons than OOM, in which case we die() anyway, we may add other types of backends in the future. For example, a hypothetical MySQL backend could fail in _begin with "Can not connect to MySQL server. No route to host".

---
 builtin/update-ref.c | 5 ++++-
 refs.c               | 2 +-
 refs.h               | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

index 7c9c248..96a53b9 100644
@@ -365,7 +365,9 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		die("Refusing to perform update with empty message.");
 
 	if (read_stdin) {
-		transaction = ref_transaction_begin();
+		transaction = ref_transaction_begin(&err);
+		if (!transaction)
+			die("%s", err.buf);
 		if (delete || no_deref || argc > 0)
 			usage_with_options(git_update_ref_usage, options);
 		if (end_null)

@@ -374,6 +376,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 		if (ref_transaction_commit(transaction, msg, &err))
 			die("%s", err.buf);
 		ref_transaction_free(transaction);
+		strbuf_release(&err);
 		return 0;
 	}
 

index 40f04f4..9cb7908 100644
@@ -3397,7 +3397,7 @@ struct ref_transaction {
 	size_t nr;
 };
 
-struct ref_transaction *ref_transaction_begin(void)
+struct ref_transaction *ref_transaction_begin(struct strbuf *err)
 {
 	return xcalloc(1, sizeof(struct ref_transaction));
 }

index 71389a1..3f37c65 100644
@@ -262,7 +262,7 @@ enum action_on_err {
  * Begin a reference transaction.  The reference transaction must
  * be freed by calling ref_transaction_free().
  */
-struct ref_transaction *ref_transaction_begin(void);
+struct ref_transaction *ref_transaction_begin(struct strbuf *err);
 
 /*
  * The following functions add a reference check or update to a

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Tue, 29 Apr 2014 12:06:19 -0700

Track the state of a transaction in a new state field. Check the field for sanity, i.e. that state must be OPEN when _commit/_create/_delete or _update is called or else die(BUG:...)

---
 refs.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

index 9cb7908..cc63056 100644
@@ -3387,6 +3387,21 @@ struct ref_update {
 };
 
 /*
+ * Transaction states.
+ * OPEN:   The transaction is in a valid state and can accept new updates.
+ *         An OPEN transaction can be committed.
+ * CLOSED: A closed transaction is no longer active and no other operations
+ *         than free can be used on it in this state.
+ *         A transaction can either become closed by successfully committing
+ *         an active transaction or if there is a failure while building
+ *         the transaction thus rendering it failed/inactive.
+ */
+enum ref_transaction_state {
+	REF_TRANSACTION_OPEN   = 0,
+	REF_TRANSACTION_CLOSED = 1
+};
+
+/*
  * Data structure for holding a reference transaction, which can
  * consist of checks and updates to multiple references, carried out
  * as atomically as possible.  This structure is opaque to callers.

@@ -3395,6 +3410,7 @@ struct ref_transaction {
 	struct ref_update **updates;
 	size_t alloc;
 	size_t nr;
+	enum ref_transaction_state state;
 };
 
 struct ref_transaction *ref_transaction_begin(struct strbuf *err)

@@ -3437,6 +3453,9 @@ int ref_transaction_update(struct ref_transaction *transaction,
 {
 	struct ref_update *update;
 
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: update called for transaction that is not open");
+
 	if (have_old && !old_sha1)
 		die("BUG: have_old is true but old_sha1 is NULL");
 

@@ -3457,6 +3476,9 @@ int ref_transaction_create(struct ref_transaction *transaction,
 {
 	struct ref_update *update;
 
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: create called for transaction that is not open");
+
 	if (!new_sha1 || is_null_sha1(new_sha1))
 		die("BUG: create ref with null new_sha1");
 

@@ -3477,6 +3499,9 @@ int ref_transaction_delete(struct ref_transaction *transaction,
 {
 	struct ref_update *update;
 
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: delete called for transaction that is not open");
+
 	if (have_old && !old_sha1)
 		die("BUG: have_old is true but old_sha1 is NULL");
 

@@ -3532,8 +3557,13 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	int n = transaction->nr;
 	struct ref_update **updates = transaction->updates;
 
-	if (!n)
+	if (transaction->state != REF_TRANSACTION_OPEN)
+		die("BUG: commit called for transaction that is not open");
+
+	if (!n) {
+		transaction->state = REF_TRANSACTION_CLOSED;
 		return 0;
+	}
 
 	/* Allocate work space */
 	delnames = xmalloc(sizeof(*delnames) * n);

@@ -3595,6 +3625,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 	clear_loose_ref_cache(&ref_cache);
 
 cleanup:
+	transaction->state = REF_TRANSACTION_CLOSED;
+
 	for (i = 0; i < n; i++)
 		if (updates[i]->lock)
 			unlock_ref(updates[i]->lock);

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:30:41 -0700

Change tag.c to use ref transactions for all ref updates.

---
 builtin/tag.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index c6e8a71..f3f172f 100644
@@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct strbuf ref = STRBUF_INIT;
 	unsigned char object[20], prev[20];
 	const char *object_ref, *tag;
-	struct ref_lock *lock;
 	struct create_tag_options opt;
 	char *cleanup_arg = NULL;
 	int annotate = 0, force = 0, lines = -1;

@@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	const char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct commit_list *with_commit = NULL;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
 	struct option options[] = {
 		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
 		{ OPTION_INTEGER, 'n', NULL, &lines, N_("n"),

@@ -701,14 +702,17 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (annotate)
 		create_tag(object, tag, &buf, &opt, prev, object);
 
-	lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
-	if (!lock)
-		die(_("%s: cannot lock the ref"), ref.buf);
-	if (write_ref_sha1(lock, object, NULL) < 0)
-		die(_("%s: cannot update the ref"), ref.buf);
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, ref.buf, object, prev,
+				   0, 1, &err) ||
+	    ref_transaction_commit(transaction, NULL, &err))
+		die("%s", err.buf);
+	ref_transaction_free(transaction);
 	if (force && !is_null_sha1(prev) && hashcmp(prev, object))
 		printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV));
 
+	strbuf_release(&err);
 	strbuf_release(&buf);
 	strbuf_release(&ref);
 	return 0;

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:32:29 -0700

Update replace.c to use ref transactions for updates.

---
 builtin/replace.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

index 1bb491d..1fcd06d 100644
@@ -153,7 +153,8 @@ static int replace_object_sha1(const char *object_ref,
 	unsigned char prev[20];
 	enum object_type obj_type, repl_type;
 	char ref[PATH_MAX];
-	struct ref_lock *lock;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
 
 	obj_type = sha1_object_info(object, NULL);
 	repl_type = sha1_object_info(repl, NULL);

@@ -166,12 +167,13 @@ static int replace_object_sha1(const char *object_ref,
 
 	check_ref_valid(object, prev, ref, sizeof(ref), force);
 
-	lock = lock_any_ref_for_update(ref, prev, 0, NULL);
-	if (!lock)
-		die("%s: cannot lock the ref", ref);
-	if (write_ref_sha1(lock, repl, NULL) < 0)
-		die("%s: cannot update the ref", ref);
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, ref, repl, prev, 0, 1, &err) ||
+	    ref_transaction_commit(transaction, NULL, &err))
+		die("%s", err.buf);
 
+	ref_transaction_free(transaction);
 	return 0;
 }
 

2.1.0.rc2.206.gedb03e5

From: Ronnie Sahlberg <[email protected]>
Date: Wed, 16 Apr 2014 15:34:19 -0700

Change commit.c to use ref transactions for all ref updates. Make sure we pass a NULL pointer to ref_transaction_update if have_old is false.

---
 builtin/commit.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

index 5e2221c..9bf1003 100644
@@ -1627,11 +1627,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	const char *index_file, *reflog_msg;
 	char *nl;
 	unsigned char sha1[20];
-	struct ref_lock *ref_lock;
 	struct commit_list *parents = NULL, **pptr = &parents;
 	struct stat statbuf;
 	struct commit *current_head = NULL;
 	struct commit_extra_header *extra = NULL;
+	struct ref_transaction *transaction;
+	struct strbuf err = STRBUF_INIT;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(builtin_commit_usage, builtin_commit_options);

@@ -1753,16 +1754,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	strbuf_release(&author_ident);
 	free_commit_extra_headers(extra);
 
-	ref_lock = lock_any_ref_for_update("HEAD",
-					   !current_head
-					   ? NULL
-					   : current_head->object.sha1,
-					   0, NULL);
-	if (!ref_lock) {
-		rollback_index_files();
-		die(_("cannot lock HEAD ref"));
-	}
-
 	nl = strchr(sb.buf, '\n');
 	if (nl)
 		strbuf_setlen(&sb, nl + 1 - sb.buf);

@@ -1771,10 +1762,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));
 	strbuf_insert(&sb, strlen(reflog_msg), ": ", 2);
 
-	if (write_ref_sha1(ref_lock, sha1, sb.buf) < 0) {
+	transaction = ref_transaction_begin(&err);
+	if (!transaction ||
+	    ref_transaction_update(transaction, "HEAD", sha1,
+				   current_head
+				   ? current_head->object.sha1 : NULL,
+				   0, !!current_head, &err) ||
+	    ref_transaction_commit(transaction, sb.buf, &err)) {
 		rollback_index_files();
-		die(_("cannot update HEAD ref"));
+		die("%s", err.buf);
 	}
+	ref_transaction_free(transaction);
 
 	unlink(git_path("CHERRY_PICK_HEAD"));
 	unlink(git_path("REVERT_HEAD"));

@@ -1803,5 +1801,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (!quiet)
 		print_summary(prefix, sha1, !current_head);
 
+	strbuf_release(&err);
 	return 0;
 }

2.1.0.rc2.206.gedb03e5