Re: [PATCH v20 30/48] refs.c: change update_ref to use a transaction

2014-07-14 Thread Ronnie Sahlberg
On Tue, Jul 8, 2014 at 5:54 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
 Change the update_ref helper function to use a ref transaction internally.

 Reviewed-by: Jonathan Nieder jrnie...@gmail.com
 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  refs.c | 28 
  1 file changed, 24 insertions(+), 4 deletions(-)

 diff --git a/refs.c b/refs.c
 index 8c695ba..4bdccc5 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -3524,11 +3524,31 @@ 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);
 + }
 + return 0;
  }


 Should this function be scheduled for the take strbuf *err argument
 treatment instead of continuing to use an action_on_err parameter?
 (Maybe you've changed this later in the patch series?)

 I'm not saying this change has to be part of the current patch series,
 but let's consider it for the future.

There is a patch that does that in a later series. At that stage we
get rid of all action_on_err arguments.



 Michael

 --
 Michael Haggerty
 mhag...@alum.mit.edu

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


Re: [PATCH v20 30/48] refs.c: change update_ref to use a transaction

2014-07-08 Thread Michael Haggerty
On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
 Change the update_ref helper function to use a ref transaction internally.
 
 Reviewed-by: Jonathan Nieder jrnie...@gmail.com
 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---
  refs.c | 28 
  1 file changed, 24 insertions(+), 4 deletions(-)
 
 diff --git a/refs.c b/refs.c
 index 8c695ba..4bdccc5 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -3524,11 +3524,31 @@ 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);
 + }
 + return 0;
  }
  

Should this function be scheduled for the take strbuf *err argument
treatment instead of continuing to use an action_on_err parameter?
(Maybe you've changed this later in the patch series?)

I'm not saying this change has to be part of the current patch series,
but let's consider it for the future.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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 v20 30/48] refs.c: change update_ref to use a transaction

2014-06-20 Thread Ronnie Sahlberg
Change the update_ref helper function to use a ref transaction internally.

Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 28 
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 8c695ba..4bdccc5 100644
--- a/refs.c
+++ b/refs.c
@@ -3524,11 +3524,31 @@ 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);
+   }
+   return 0;
 }
 
 static int ref_update_compare(const void *r1, const void *r2)
-- 
2.0.0.420.g181e020.dirty

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