Re: What's cooking in git.git (Mar 2014, #07; Fri, 28)

2014-03-31 Thread Ronnie Sahlberg
(now without HTML formatting) I am new to git, so sorry If I overlooked something. I think there might be a race in ref_transaction_commit() when deleting references. /* Perform deletes now that updates are safely completed */ for (i = 0; i n; i++) { struct ref_update *update = updates[i];

[PATCH] Remove the close_ref function.

2014-04-08 Thread Ronnie Sahlberg
function. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/reflog.c | 3 +-- refs.c | 11 +-- refs.h | 3 --- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index c12a9784..b67fbe6 100644 --- a/builtin

[PATCH] Remove redundant close_ref function

2014-04-08 Thread Ronnie Sahlberg
the api to refs slightly. This also means that the lifetime of the filedescriptor becomes the same as the lifetime for the 'struct ref_lock' object. The filedescriptor is opened at the same time ref_lock is allocated and the descriptor is closed when ref_lock is released. regards ronnie sahlberg

Re: [PATCH] Remove the close_ref function.

2014-04-08 Thread Ronnie Sahlberg
On Tue, Apr 8, 2014 at 2:50 PM, Junio C Hamano gits...@pobox.com wrote: Ronnie Sahlberg sahlb...@google.com writes: @@ -2824,8 +2816,7 @@ int write_ref_sha1(struct ref_lock *lock, return -1; } if (write_in_full(lock-lock_fd, sha1_to_hex(sha1), 40) != 40

[PATCH 0/3] Make update-refs more atomic

2014-04-08 Thread Ronnie Sahlberg
, but I would still like some review and feedback on these changes. Ronnie Sahlberg (3): Split writing and commiting a ref into two separate functions. Split delete_ref_loose() into a separate flag-for-deletion and commit phase Change update_refs to run a single commit loop once all work

[PATCH 1/3] Split writing and commiting a ref into two separate functions.

2014-04-08 Thread Ronnie Sahlberg
and delete the lock file withouth applying it, or call commit_ref_lock() which will rename the lock file onto the ref file. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 10 -- builtin/commit.c | 5 + builtin/fetch.c| 7 ++- builtin

[PATCH 2/3] Split delete_ref_loose() into a separate flag-for-deletion and commit phase

2014-04-08 Thread Ronnie Sahlberg
. The new pattern for deleting loose refs thus become: lock = lock_ref_sha1_basic() (or varient of) delete_ref_loose(lock) unlock_ref(lock) | commit_ref_lock(lock) Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 31 --- refs.h | 2 ++ 2 files changed, 22

[PATCH 3/3] Change update_refs to run a single commit loop once all work is finished.

2014-04-08 Thread Ronnie Sahlberg
/deleting multiple refs 'more atomic' since we will not start the commit phase until all the preparations have completed successfully. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 30 ++ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git

[PATCH 2/4] refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase

2014-04-10 Thread Ronnie Sahlberg
. The new pattern for deleting loose refs thus become: lock = lock_ref_sha1_basic() (or varient of) delete_ref_loose(lock) unlock_ref(lock) | commit_ref_lock(lock) Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 31 --- refs.h | 2 ++ 2 files changed, 22

[PATCH 3/4] refs.c: change update_refs to run the commit loops once all work is finished

2014-04-10 Thread Ronnie Sahlberg
/deleting multiple refs 'more atomic' since we will not start the commit phase until all the preparations have completed successfully. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 38 +- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git

[PATCH 1/4] refs.c: split writing and commiting a ref into two separate functions

2014-04-10 Thread Ronnie Sahlberg
and delete the lock file withouth applying it, or call commit_ref_lock() which will rename the lock file onto the ref file. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 10 -- builtin/commit.c | 5 + builtin/fetch.c| 7 ++- builtin

[PATCH 4/4] refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one

2014-04-10 Thread Ronnie Sahlberg
the deletes. If we sort the new sha1 for all the refs so that a deleted ref, with sha1 0{40} comes at the end of the array, then we can just run a single loop and still guarantee that all updates happen before any deletes. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 18

[PATCH 0/4] Make update_refs more atomic V2

2014-04-10 Thread Ronnie Sahlberg
updates first, then perform the deletes. * Add an additional patch that allows us to do the update/delete in the correct order from within a single loop by first sorting the refs so that deletes are after all non-deletes. Ronnie Sahlberg (4): refs.c: split writing and commiting a ref into two

Re: [PATCH 4/4] refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one

2014-04-10 Thread Ronnie Sahlberg
On Thu, Apr 10, 2014 at 12:01 PM, Junio C Hamano gits...@pobox.com wrote: Ronnie Sahlberg sahlb...@google.com writes: We want to make sure that we update all refs before we delete any refs so that there is no point in time where the tips may not be reachable from any ref in the system. We

Re: Our official home page and logo for the Git project

2014-04-11 Thread Ronnie Sahlberg
On Fri, Apr 11, 2014 at 12:25 PM, Junio C Hamano gits...@pobox.com wrote: Please help us by letting us answer Yup, that is a logo (among others) that represents our project, and we are OK with you using it to help promote our project instead. That is what I meant by our official

Re: [PATCH v3 00/27] Clean up update-refs --stdin and implement ref_transaction

2014-04-11 Thread Ronnie Sahlberg
Nice. Once this is in I can add transactional support to receive-pack. On Mon, Apr 7, 2014 at 6:47 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Here is v3. It is also available on GitHub [1]. Thanks to Junio and Brad for their comments about v2. I think I have addressed all of your

[PATCH v3 0/3] Make update refs more atomic

2014-04-11 Thread Ronnie Sahlberg
. * Add an additional patch that allows us to do the update/delete in the correct order from within a single loop by first sorting the refs so that deletes are after all non-deletes. Ronnie Sahlberg (3): refs.c: split writing and commiting a ref into two separate functions refs.c: split

[PATCH v3 2/3] refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase

2014-04-11 Thread Ronnie Sahlberg
. The new pattern for deleting loose refs thus become: lock = lock_ref_sha1_basic() (or varient of) delete_ref_loose(lock) unlock_ref(lock) | commit_ref_lock(lock) Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 31 --- refs.h | 2 ++ 2 files changed, 22

[PATCH v3 1/3] refs.c: split writing and commiting a ref into two separate functions

2014-04-11 Thread Ronnie Sahlberg
and delete the lock file withouth applying it, or call commit_ref_lock() which will rename the lock file onto the ref file. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 10 -- builtin/commit.c | 5 + builtin/fetch.c| 7 ++- builtin

[PATCH v3 3/3] refs.c: change ref_transaction_commit to run the commit loops once all work is finished

2014-04-11 Thread Ronnie Sahlberg
we will not start the commit phase until all the preparations have completed successfully. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 39 ++- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/refs.c b/refs.c index ec104f2

[PATCH v4 2/3] refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase

2014-04-14 Thread Ronnie Sahlberg
. The new pattern for deleting loose refs thus become: lock = lock_ref_sha1_basic() (or varient of) delete_ref_loose(lock) unlock_ref(lock) | commit_ref_lock(lock) Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 32 refs.h | 2 ++ 2 files changed, 22

[PATCH v4 3/3] refs.c: change ref_transaction_commit to run the commit loops once all work is finished

2014-04-14 Thread Ronnie Sahlberg
we will not start the commit phase until all the preparations have completed successfully. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 39 ++- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/refs.c b/refs.c index a14addb

[PATCH v4 0/3] Make update refs more atomic

2014-04-14 Thread Ronnie Sahlberg
order of operations. Perform all updates first, then perform the deletes. * Add an additional patch that allows us to do the update/delete in the correct order from within a single loop by first sorting the refs so that deletes are after all non-deletes. Ronnie Sahlberg (3): refs.c: split

[PATCH v4 1/3] refs.c: split writing and commiting a ref into two separate functions

2014-04-14 Thread Ronnie Sahlberg
and delete the lock file withouth applying it, or call commit_ref_lock() which will rename the lock file onto the ref file. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 10 -- builtin/commit.c | 5 + builtin/fetch.c| 7 ++- builtin

Re: [PATCH v4 0/3] Make update refs more atomic

2014-04-15 Thread Ronnie Sahlberg
version of this patch series. regards ronnie sahlberg -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] sequencer.c: check for lock failure and bail early in fast_forward_to

2014-04-15 Thread Ronnie Sahlberg
lock_any_ref_for_update() feels obfuscated. This change should not change any functionality or logic aside from adding an extra error message when this condition is triggered. (write_ref_sha1() returns an error silently for this condition) Signed-off-by: Ronnie Sahlberg sahlb...@google.com

[PATCH 0/2] Check for lock failures early

2014-04-15 Thread Ronnie Sahlberg
to read by being more consistent. Ronnie Sahlberg (2): sequencer.c: check for lock failure and bail early in fast_forward_to commit.c: check for lock error and return early builtin/commit.c | 8 sequencer.c | 7 +++ 2 files changed, 11 insertions(+), 4 deletions

[PATCH 2/2] commit.c: check for lock error and return early

2014-04-15 Thread Ronnie Sahlberg
to read and makes it follow the pattern of try-to-take-a-lock() if (check-if-lock-failed){ error } Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/commit.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index

Re: [PATCH v4 0/3] Make update refs more atomic

2014-04-16 Thread Ronnie Sahlberg
On Tue, Apr 15, 2014 at 1:32 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/15/2014 06:33 PM, Ronnie Sahlberg wrote: On Mon, Apr 14, 2014 at 11:36 PM, Michael Haggerty mhag...@alum.mit.edu wrote: [...] I wonder, however, whether your approach of changing callers from lock

[PATCH v2 2/2] commit.c: check for lock error and return early

2014-04-16 Thread Ronnie Sahlberg
Move the check for the lock failure to happen immediately after lock_any_ref_for_update(). Previously the lock and the check-if-lock-failed was separated by a handful of string manipulation statements. Moving the check to occur immediately after the failed lock makes the code slightly easier to

[PATCH v2 0/2] Check for lock failures early

2014-04-16 Thread Ronnie Sahlberg
to read by being more consistent. Version 2: * Simplify the return on error case in sequencer.c. Ronnie Sahlberg (2): sequencer.c: check for lock failure and bail early in fast_forward_to commit.c: check for lock error and return early builtin/commit.c | 8 sequencer.c | 4

[PATCH v2 1/2] sequencer.c: check for lock failure and bail early in fast_forward_to

2014-04-16 Thread Ronnie Sahlberg
lock_any_ref_for_update() feels obfuscated. This change should not change any functionality or logic aside from adding an extra error message when this condition is triggered. (write_ref_sha1() returns an error silently for this condition) Signed-off-by: Ronnie Sahlberg sahlb...@google.com

Re: [PATCH v4 0/3] Make update refs more atomic

2014-04-16 Thread Ronnie Sahlberg
On Wed, Apr 16, 2014 at 12:31 PM, Junio C Hamano gits...@pobox.com wrote: Ronnie Sahlberg sahlb...@google.com writes: Currently any locking of refs in a transaction only happens during the commit phase. I think it would be useful to have a mechanism where you could optionally take out locks

[PATCH 00/11] Use ref transactions from most callers

2014-04-17 Thread Ronnie Sahlberg
changes belong in a different patch series. Ronnie Sahlberg (11): refs.c: constify the sha arguments for ref_transaction_create|delete|update refs.c: change ref_transaction_update() to do error checking and return status refs.c: change ref_transaction_create to do error checking

[PATCH 04/11] refs.c: ref_transaction_delete to check for error and return status

2014-04-17 Thread Ronnie Sahlberg
Change ref_transaction_delete() to do basic error checking and return status. Update all callers to check the return for ref_transaction_delete() Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 5 +++-- refs.c | 15 ++- refs.h

[PATCH 03/11] refs.c: change ref_transaction_create to do error checking and return status

2014-04-17 Thread Ronnie Sahlberg
Do basic error checking in ref_transaction_create() and make it return status. Update all callers to check the result of ref_transaction_create() Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 4 +++- refs.c | 17 +++-- refs.h

[PATCH 05/11] tag.c: use ref transactions when doing updates

2014-04-17 Thread Ronnie Sahlberg
Change tag.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/tag.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 40356e3..dbeacc5 100644 --- a/builtin/tag.c +++ b

[PATCH 02/11] refs.c: change ref_transaction_update() to do error checking and return status

2014-04-17 Thread Ronnie Sahlberg
Update ref_transaction_update() do some basic error checking and return true on error. Update all callers to check ref_transaction_update() for error. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 11 +++ refs.c | 9 +++-- refs.h

[PATCH 01/11] refs.c: constify the sha arguments for ref_transaction_create|delete|update

2014-04-17 Thread Ronnie Sahlberg
ref_transaction_update() in which case this change is required. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 7 --- refs.h | 7 --- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c index 728a761..138ab70 100644 --- a/refs.c +++ b/refs.c

[PATCH 08/11] sequencer.c: use ref transactions for all ref updates

2014-04-17 Thread Ronnie Sahlberg
Change to use ref transactions for all updates to refs. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- sequencer.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/sequencer.c b/sequencer.c index bde5f04..fa14ac0 100644 --- a/sequencer.c +++ b

[PATCH 10/11] branch.c: use ref transaction for all ref updates

2014-04-17 Thread Ronnie Sahlberg
Change branch.c to use ref transactions when doing updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/branch.c b/branch.c index 660097b..45c7766 100644 --- a/branch.c +++ b/branch.c

[PATCH 09/11] fast-import.c: change update_branch to use ref transactions

2014-04-17 Thread Ronnie Sahlberg
Change update_branch() to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- fast-import.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/fast-import.c b/fast-import.c index fb4738d..466dfe3 100644 --- a/fast

[PATCH 07/11] commit.c: use ref transactions for updates

2014-04-17 Thread Ronnie Sahlberg
Change commit.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/commit.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index d9550c5..b8e4389 100644

[PATCH 11/11] walker.c: use ref transaction for ref updates

2014-04-17 Thread Ronnie Sahlberg
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. Signed-off-by: Ronnie

[PATCH 06/11] replace.c: use the ref transaction functions for updates

2014-04-17 Thread Ronnie Sahlberg
Update replace.c to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/replace.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index b62420a..d8bd6ee 100644 --- a/builtin

Re: [PATCH 07/11] commit.c: use ref transactions for updates

2014-04-21 Thread Ronnie Sahlberg
On Sat, Apr 19, 2014 at 12:23 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/17/2014 09:46 PM, Ronnie Sahlberg wrote: Change commit.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/commit.c | 22 -- 1

Re: [PATCH 11/11] walker.c: use ref transaction for ref updates

2014-04-21 Thread Ronnie Sahlberg
I have updated the commit message with some text why I do not think this change is critical for this case. I will resend v2 of the patch series in a little while. On Sat, Apr 19, 2014 at 12:48 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/17/2014 09:46 PM, Ronnie Sahlberg wrote: Switch

[PATCH v2 03/13] refs.c: change ref_transaction_update() to do error checking and return status

2014-04-21 Thread Ronnie Sahlberg
Update ref_transaction_update() do some basic error checking and return true on error. Update all callers to check ref_transaction_update() for error. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 10 ++ refs.c | 9 +++-- refs.h

[PATCH v2 00/13] Use ref transactions from most callers

2014-04-21 Thread Ronnie Sahlberg
for current_head-object.sha1 and relied on the fact that ref_transaction_update would not try to dereference this pointer if !!current_head was 0. - Updated commit message for the walker_fetch change to try to justify why the change in locking semantics should not be harmful. Ronnie Sahlberg (13

[PATCH v2 04/13] refs.c: change ref_transaction_create to do error checking and return status

2014-04-21 Thread Ronnie Sahlberg
Do basic error checking in ref_transaction_create() and make it return status. Update all callers to check the result of ref_transaction_create() Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 4 +++- refs.c | 17 +++-- refs.h

[PATCH v2 07/13] replace.c: use the ref transaction functions for updates

2014-04-21 Thread Ronnie Sahlberg
Update replace.c to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/replace.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index b62420a..b91e550 100644 --- a/builtin

[PATCH v2 02/13] refs.c: use a single exit path from transaction commit and handle onerr

2014-04-21 Thread Ronnie Sahlberg
for this case, but the code would just look too awful. I think it is acceptable to log two error messages for those two cases than to badify the commit code. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/refs.c

[PATCH v2 05/13] refs.c: ref_transaction_delete to check for error and return status

2014-04-21 Thread Ronnie Sahlberg
Change ref_transaction_delete() to do basic error checking and return status. Update all callers to check the return for ref_transaction_delete() Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 5 +++-- refs.c | 15 ++- refs.h

[PATCH v2 12/13] walker.c: use ref transaction for ref updates

2014-04-21 Thread Ronnie Sahlberg
) repository which likely means that the type of collissions that the previous locking would protect against and cause the fetch to fail for to be even more rare. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- walker.c | 45 - 1 file changed, 20 insertions

[PATCH v2 10/13] fast-import.c: change update_branch to use ref transactions

2014-04-21 Thread Ronnie Sahlberg
Change update_branch() to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- fast-import.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/fast-import.c b/fast-import.c index fb4738d..466dfe3 100644 --- a/fast

[PATCH v2 01/13] refs.c: constify the sha arguments for ref_transaction_create|delete|update

2014-04-21 Thread Ronnie Sahlberg
ref_transaction_update() in which case this change is required. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 7 --- refs.h | 7 --- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c index 728a761..138ab70 100644 --- a/refs.c +++ b/refs.c

[PATCH v2 09/13] sequencer.c: use ref transactions for all ref updates

2014-04-21 Thread Ronnie Sahlberg
Change to use ref transactions for all updates to refs. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- sequencer.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/sequencer.c b/sequencer.c index bde5f04..fa14ac0 100644 --- a/sequencer.c +++ b

[PATCH v2 06/13] tag.c: use ref transactions when doing updates

2014-04-21 Thread Ronnie Sahlberg
Change tag.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/tag.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 40356e3..3c40957 100644 --- a/builtin/tag.c +++ b

[PATCH v2 08/13] commit.c: use ref transactions for updates

2014-04-21 Thread Ronnie Sahlberg
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. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/commit.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions

[PATCH v2 11/13] branch.c: use ref transaction for all ref updates

2014-04-21 Thread Ronnie Sahlberg
Change branch.c to use ref transactions when doing updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/branch.c b/branch.c index 660097b..45c7766 100644 --- a/branch.c +++ b/branch.c

[PATCH v2 13/13] refs.c: change update_ref to use a transaction

2014-04-21 Thread Ronnie Sahlberg
Change the update_ref helper function to use a ref transaction internally. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 28 +++- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index 878948a..e52b6bf 100644 --- a/refs.c

[PATCH 0/3] Use ref transactions for fetch

2014-04-22 Thread Ronnie Sahlberg
. Use a single ref transaction for all updates and only commit the transaction if all other checks and oeprations have been successful. This makes the ref updates during a fetch (mostly) atomic. Ronnie Sahlberg (3): fetch.c: clear errno before calling functions that might set it fetch.c: change

[PATCH 1/3] fetch.c: clear errno before calling functions that might set it

2014-04-22 Thread Ronnie Sahlberg
and unrelated ENOTDIT may cause us to return the wrong error. Clear errno before calling any functions if we check errno afterwards. Also skip initializing a static variable to 0. Sstatics live in .bss and are all automatically initialized to 0. Signed-off-by: Ronnie Sahlberg sahlb...@google.com

[PATCH 2/3] fetch.c: change s_update_ref to use a ref transaction

2014-04-22 Thread Ronnie Sahlberg
Change s_update_ref to use a ref transaction for the ref update. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/fetch.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/builtin/fetch.c b/builtin

[PATCH 3/3] fetch.c: use a single ref transaction for all ref updates

2014-04-22 Thread Ronnie Sahlberg
not even try to commit the transaction but need to highlight this change in how/what errors are reported. This change in what error is returned only occurs if there are multiple refs that fail to update and only some, but not all, of them fail due to ENOTDIR. Signed-off-by: Ronnie Sahlberg sahlb

Re: [PATCH v2 00/13] Use ref transactions from most callers

2014-04-22 Thread Ronnie Sahlberg
I will look at this once i finish the next respin. On Tue, Apr 22, 2014 at 12:34 PM, Junio C Hamano gits...@pobox.com wrote: Ronnie Sahlberg sahlb...@google.com writes: This patch series changes most of the places where the ref functions for locking and writing refs to instead use the new ref

Re: [PATCH v2 02/13] refs.c: use a single exit path from transaction commit and handle onerr

2014-04-22 Thread Ronnie Sahlberg
On Tue, Apr 22, 2014 at 12:11 PM, Junio C Hamano gits...@pobox.com wrote: Ronnie Sahlberg sahlb...@google.com writes: diff --git a/refs.c b/refs.c index 138ab70..9daf89e 100644 --- a/refs.c +++ b/refs.c @@ -3414,12 +3414,12 @@ int ref_transaction_commit(struct ref_transaction *transaction

Re: [PATCH 2/3] fetch.c: change s_update_ref to use a ref transaction

2014-04-24 Thread Ronnie Sahlberg
Fixed. Thanks. On Wed, Apr 23, 2014 at 1:12 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg sahlb...@google.com wrote: Change s_update_ref to use a ref transaction for the ref update. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed

Re: [PATCH 1/3] fetch.c: clear errno before calling functions that might set it

2014-04-24 Thread Ronnie Sahlberg
Fixed. Thanks. On Wed, Apr 23, 2014 at 1:12 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg sahlb...@google.com wrote: In s_update_ref there are two calls that when they fail we return an error based on the errno value. In particular we want

Re: [PATCH 3/3] fetch.c: use a single ref transaction for all ref updates

2014-04-24 Thread Ronnie Sahlberg
Fixed, thanks! On Wed, Apr 23, 2014 at 1:17 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg sahlb...@google.com wrote: Change store_updated_refs to use a single ref transaction for all refs that are updated during the fetch. This makes

[PATCH v3 11/19] tag.c: use ref transactions when doing updates

2014-04-25 Thread Ronnie Sahlberg
Change tag.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/tag.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 40356e3..dd53fb4 100644 --- a/builtin/tag.c +++ b

[PATCH v3 07/19] refs.c: remove the onerr argument to ref_transaction_commit

2014-04-25 Thread Ronnie Sahlberg
Since we now pass meaningful error messages back from ref_transaction_commit on failures, we no longer need to provide a onerr argument. The callers can now on commit failures die() with a meaningful, and in most cases even better than before, error message. Signed-off-by: Ronnie Sahlberg sahlb

[PATCH v3 02/19] refs.c: allow passing NULL to ref_transaction_free

2014-04-25 Thread Ronnie Sahlberg
() then transaction is untouched and then ref_transaction_rollback(transaction) will rollback the failed transaction. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/refs.c b/refs.c index 138ab70..2d83ef6 100644 --- a/refs.c +++ b/refs.c

[PATCH v3 12/19] replace.c: use the ref transaction functions for updates

2014-04-25 Thread Ronnie Sahlberg
Update replace.c to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/replace.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index b62420a..cf0f56d 100644 --- a/builtin

[PATCH v3 19/19] refs.c: pass **transaction to commit and have it clear the pointer

2014-04-25 Thread Ronnie Sahlberg
)) { ref_transaction_rollback(t); Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 4 ++-- builtin/commit.c | 2 +- builtin/replace.c| 2 +- builtin/tag.c| 2 +- builtin/update-ref.c | 2 +- fast-import.c| 7 +++ refs.c | 18

[PATCH v3 06/19] refs.c: make update_ref_write to return error string on failure

2014-04-25 Thread Ronnie Sahlberg
Change update_ref_write to also return an error string on failure. This makes the error avaialbel to ref_transaction_commit callers if the transaction failed dur to update_ref_sha1/write_ref_sha1 failures. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 10 +++--- 1 file

[PATCH v3 00/19] Use ref transactions from most callers

2014-04-25 Thread Ronnie Sahlberg
that ref_transaction_update would not try to dereference this pointer if !!current_head was 0. - Updated commit message for the walker_fetch change to try to justify why the change in locking semantics should not be harmful. Ronnie Sahlberg (19): refs.c: constify the sha arguments for ref_transaction_create

[PATCH v3 17/19] refs.c: change update_ref to use a transaction

2014-04-25 Thread Ronnie Sahlberg
Change the update_ref helper function to use a ref transaction internally. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 1557c3c..95df4a9 100644 --- a/refs.c +++ b

[PATCH v3 18/19] refs.c: free the transaction before returning when number of updates is 0

2014-04-25 Thread Ronnie Sahlberg
We have to free the transaction before returning in the early check for 'return early if number of updates == 0' or else the following code would create a memory leak with the transaction never being freed : t = ref_transaction_begin() ref_transaction_commit(t) Signed-off-by: Ronnie

[PATCH v3 04/19] refs.c: return error string from ref_update_reject_duplicates on failure

2014-04-25 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 0712912..9d9eab8 100644 --- a/refs.c +++ b/refs.c @@ -3393,6 +3393,7 @@ static int ref_update_compare(const void *r1, const void *r2

[PATCH v3 10/19] refs.c: ref_transaction_delete to check for error and return status

2014-04-25 Thread Ronnie Sahlberg
Change ref_transaction_delete() to do basic error checking and return status. Update all callers to check the return for ref_transaction_delete() Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 5 +++-- refs.c | 15 ++- refs.h

[PATCH v3 15/19] fast-import.c: change update_branch to use ref transactions

2014-04-25 Thread Ronnie Sahlberg
Change update_branch() to use ref transactions for updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- fast-import.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/fast-import.c b/fast-import.c index fb4738d..a2b05fa 100644 --- a/fast

[PATCH v3 13/19] commit.c: use ref transactions for updates

2014-04-25 Thread Ronnie Sahlberg
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. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/commit.c | 23 ++- 1 file changed, 10 insertions(+), 13 deletions

[PATCH v3 14/19] sequencer.c: use ref transactions for all ref updates

2014-04-25 Thread Ronnie Sahlberg
Change to use ref transactions for all updates to refs. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- sequencer.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/sequencer.c b/sequencer.c index bde5f04..7d59f58 100644 --- a/sequencer.c +++ b

[PATCH v3 01/19] refs.c: constify the sha arguments for ref_transaction_create|delete|update

2014-04-25 Thread Ronnie Sahlberg
ref_transaction_update() in which case this change is required. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 7 --- refs.h | 7 --- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c index 728a761..138ab70 100644 --- a/refs.c +++ b/refs.c

[PATCH v3 16/19] branch.c: use ref transaction for all ref updates

2014-04-25 Thread Ronnie Sahlberg
the lock_any_ref_for_update/write_ref_sha1 did not protect against the ref already existsing. I.e. one thread could end up overwriting a branch even if the forcing flag is false. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 39 +-- 1 file changed, 25

[PATCH v3 05/19] update-ref.c: use the error string from _commit to print better message

2014-04-25 Thread Ronnie Sahlberg
the previous fatal: Cannot lock the ref 'some ref' Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 12 +++- t/t1400-update-ref.sh | 16 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/builtin/update-ref.c b/builtin/update

[PATCH v3 09/19] refs.c: change ref_transaction_create to do error checking and return status

2014-04-25 Thread Ronnie Sahlberg
Do basic error checking in ref_transaction_create() and make it return status. Update all callers to check the result of ref_transaction_create() Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 4 +++- refs.c | 17 +++-- refs.h

[PATCH v3 03/19] refs.c: make ref_transaction_commit return an error string

2014-04-25 Thread Ronnie Sahlberg
fails. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 2 +- refs.c | 12 +++- refs.h | 5 - 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 405267f..aaa06aa

[PATCH v3 08/19] refs.c: change ref_transaction_update() to do error checking and return status

2014-04-25 Thread Ronnie Sahlberg
Update ref_transaction_update() do some basic error checking and return true on error. Update all callers to check ref_transaction_update() for error. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/update-ref.c | 10 ++ refs.c | 9 +++-- refs.h

Re: [PATCH v3 00/19] Use ref transactions from most callers

2014-04-25 Thread Ronnie Sahlberg
On Fri, Apr 25, 2014 at 2:53 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: This patch series changes most of the places where the ref functions for locking and writing refs to instead use the new ref transaction API. Thanks. Is this series based against mh/ref

Re: [PATCH v3 05/19] update-ref.c: use the error string from _commit to print better message

2014-04-28 Thread Ronnie Sahlberg
Good ideas. Applied, thanks. On Fri, Apr 25, 2014 at 3:36 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Call ref_transaction_commit with QUIET_ON_ERR and use the error string that is returned to print a better log message if/after the transaction fails. Ah, so that's

Re: [PATCH v3 11/19] tag.c: use ref transactions when doing updates

2014-04-28 Thread Ronnie Sahlberg
Done. Thanks. On Fri, Apr 25, 2014 at 3:58 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/25/2014 06:14 PM, Ronnie Sahlberg wrote: Change tag.c to use ref transactions for all ref updates. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/tag.c | 14 -- 1

Re: [PATCH v3 16/19] branch.c: use ref transaction for all ref updates

2014-04-28 Thread Ronnie Sahlberg
On Fri, Apr 25, 2014 at 4:16 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/25/2014 06:14 PM, Ronnie Sahlberg wrote: Change create_branch to use a ref transaction when creating the new branch. ref_transaction_create will check that the ref does not already exist and fail otherwise

Re: [PATCH v3 04/19] refs.c: return error string from ref_update_reject_duplicates on failure

2014-04-28 Thread Ronnie Sahlberg
Thanks. Replaced with strbuf_addf() instead. On Fri, Apr 25, 2014 at 3:12 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: --- a/refs.c +++ b/refs.c @@ -3393,6 +3393,7 @@ static int ref_update_compare(const void *r1, const void *r2) } static int

Re: [PATCH v3 06/19] refs.c: make update_ref_write to return error string on failure

2014-04-28 Thread Ronnie Sahlberg
Done, thanks On Fri, Apr 25, 2014 at 3:45 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Change update_ref_write to also return an error string on failure. This makes the error avaialbel to ref_transaction_commit callers if the transaction failed dur to update_ref_sha1

Re: [PATCH v3 03/19] refs.c: make ref_transaction_commit return an error string

2014-04-28 Thread Ronnie Sahlberg
Good points. On Fri, Apr 25, 2014 at 3:10 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Let ref_transaction_commit return an optional error string that describes the transaction failure. Start by returning the same error as update_ref_lock returns, modulo

Re: [PATCH v3 07/19] refs.c: remove the onerr argument to ref_transaction_commit

2014-04-28 Thread Ronnie Sahlberg
Thanks. Reworded it that we remove it since all callers now use QUIET_ON_ERR On Fri, Apr 25, 2014 at 3:47 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Since we now pass meaningful error messages back from ref_transaction_commit on failures, we no longer need

Re: [PATCH v3 17/19] refs.c: change update_ref to use a transaction

2014-04-28 Thread Ronnie Sahlberg
Thanks. On Fri, Apr 25, 2014 at 4:28 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/25/2014 06:14 PM, Ronnie Sahlberg wrote: Change the update_ref helper function to use a ref transaction internally. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 23

Re: [PATCH v3 19/19] refs.c: pass **transaction to commit and have it clear the pointer

2014-04-28 Thread Ronnie Sahlberg
On Fri, Apr 25, 2014 at 6:31 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/25/2014 06:14 PM, Ronnie Sahlberg wrote: Change ref_transaction_commit to take a pointer to a pointer for the transaction. This allows us to clear the transaction pointer from within ref_transaction_commit so

  1   2   3   4   5   6   7   8   9   10   >