subject:"\[PATCH v10 13\/44\] tag.c\: use ref transactions when doing updates"

Re: [PATCH v10 13/44] tag.c: use ref transactions when doing updates

2014-05-19 Thread Ronnie Sahlberg

On Sat, May 17, 2014 at 6:09 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
 On 05/16/2014 07:37 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 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/builtin/tag.c b/builtin/tag.c
 index c6e8a71..b05f9a5 100644
 --- a/builtin/tag.c
 +++ b/builtin/tag.c
 @@ -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,11 +702,12 @@ 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();
 + if (!transaction ||
 + ref_transaction_update(transaction, ref.buf, object, prev,
 +0, !is_null_sha1(prev), err) ||
 + ref_transaction_commit(transaction, NULL, err))
 + die(%s, err.buf);

 If ref_transaction_begin() fails, then won't err still be empty?  (I
 know it can't happen, and you know it can't happen, but should the
 caller have to know that?)  It almost seems like ref_transaction_begin()
 should have an err parameter, too.

I add an err argument in the next series. I would prefer we let that
patch remain in the next series to
avoid unbounded growth of the current one.

Ok ?




 It's kindof late for this question to pop into my head, but: have you
 thought about embedding the err strbuf in the transaction object?  I
 admit it would make the problem with ref_transaction_begin() even worse,
 but maybe it would be a net win in terms of boilerplate?

I think it is more flexible to allow the caller to manage the lifetime
of the error buffer independently of the transaction.
It would allow a caller to free the transaction early but delay
printing the error message until later.

Or it could be used for a multi transaction caller to use a single err
buffer for all transactions and finally print
all errors in a single error() call at the end.

struct strbuf err = STRBUF_INIT;
... first transaction (... err)...
... second transaction (... err)...
... third transaction (... err)...
error(%s, err.buf);



Similar to how rsync handles errors:
sahlberg@sahlberg1:~$ mkdir foo
sahlberg@sahlberg1:~$ touch foo/foo.1
sahlberg@sahlberg1:~$ touch foo/foo.2
sahlberg@sahlberg1:~$ mkdir bar
sahlberg@sahlberg1:~$ chmod 0500 bar
sahlberg@sahlberg1:~$ rsync -Pav foo/* bar
sending incremental file list
foo.1
   0 100%0.00kB/s0:00:00 (xfer#1, to-check=1/2)
foo.2
   0 100%0.00kB/s0:00:00 (xfer#2, to-check=0/2)
rsync: mkstemp /usr/local/google/home/sahlberg/bar/.foo.1.K7dFIP
failed: Permission denied (13)
rsync: mkstemp /usr/local/google/home/sahlberg/bar/.foo.2.4WdRsW
failed: Permission denied (13)

sent 136 bytes  received 50 bytes  372.00 bytes/sec
total size is 0  speedup is 0.00
rsync error: some files/attrs were not transferred (see previous
errors) (code 23) at main.c(1070) [sender=3.0.9]




   if (force  !is_null_sha1(prev)  hashcmp(prev, object))
   printf(_(Updated tag '%s' (was %s)\n), tag, 
 find_unique_abbrev(prev, DEFAULT_ABBREV));




 --
 Michael Haggerty
 mhag...@alum.mit.edu
 http://softwareswirl.blogspot.com/
--
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 v10 13/44] tag.c: use ref transactions when doing updates

2014-05-19 Thread Ronnie Sahlberg

I have moved the patch to add err to ref_transaction_begin to the
current patch series.

Please see
https://github.com/rsahlberg/git/tree/ref-transactions

On Mon, May 19, 2014 at 10:16 AM, Ronnie Sahlberg sahlb...@google.com wrote:
 On Sat, May 17, 2014 at 6:09 AM, Michael Haggerty mhag...@alum.mit.edu 
 wrote:
 On 05/16/2014 07:37 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 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/builtin/tag.c b/builtin/tag.c
 index c6e8a71..b05f9a5 100644
 --- a/builtin/tag.c
 +++ b/builtin/tag.c
 @@ -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,11 +702,12 @@ 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();
 + if (!transaction ||
 + ref_transaction_update(transaction, ref.buf, object, prev,
 +0, !is_null_sha1(prev), err) ||
 + ref_transaction_commit(transaction, NULL, err))
 + die(%s, err.buf);

 If ref_transaction_begin() fails, then won't err still be empty?  (I
 know it can't happen, and you know it can't happen, but should the
 caller have to know that?)  It almost seems like ref_transaction_begin()
 should have an err parameter, too.

 I add an err argument in the next series. I would prefer we let that
 patch remain in the next series to
 avoid unbounded growth of the current one.

 Ok ?




 It's kindof late for this question to pop into my head, but: have you
 thought about embedding the err strbuf in the transaction object?  I
 admit it would make the problem with ref_transaction_begin() even worse,
 but maybe it would be a net win in terms of boilerplate?

 I think it is more flexible to allow the caller to manage the lifetime
 of the error buffer independently of the transaction.
 It would allow a caller to free the transaction early but delay
 printing the error message until later.

 Or it could be used for a multi transaction caller to use a single err
 buffer for all transactions and finally print
 all errors in a single error() call at the end.

 struct strbuf err = STRBUF_INIT;
 ... first transaction (... err)...
 ... second transaction (... err)...
 ... third transaction (... err)...
 error(%s, err.buf);



 Similar to how rsync handles errors:
 sahlberg@sahlberg1:~$ mkdir foo
 sahlberg@sahlberg1:~$ touch foo/foo.1
 sahlberg@sahlberg1:~$ touch foo/foo.2
 sahlberg@sahlberg1:~$ mkdir bar
 sahlberg@sahlberg1:~$ chmod 0500 bar
 sahlberg@sahlberg1:~$ rsync -Pav foo/* bar
 sending incremental file list
 foo.1
0 100%0.00kB/s0:00:00 (xfer#1, to-check=1/2)
 foo.2
0 100%0.00kB/s0:00:00 (xfer#2, to-check=0/2)
 rsync: mkstemp /usr/local/google/home/sahlberg/bar/.foo.1.K7dFIP
 failed: Permission denied (13)
 rsync: mkstemp /usr/local/google/home/sahlberg/bar/.foo.2.4WdRsW
 failed: Permission denied (13)

 sent 136 bytes  received 50 bytes  372.00 bytes/sec
 total size is 0  speedup is 0.00
 rsync error: some files/attrs were not transferred (see previous
 errors) (code 23) at main.c(1070) [sender=3.0.9]




   if (force  !is_null_sha1(prev)  hashcmp(prev, object))
   printf(_(Updated tag '%s' (was %s)\n), tag, 
 find_unique_abbrev(prev, DEFAULT_ABBREV));




 --
 Michael Haggerty
 mhag...@alum.mit.edu
 http://softwareswirl.blogspot.com/
--
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 v10 13/44] tag.c: use ref transactions when doing updates

2014-05-17 Thread Michael Haggerty

On 05/16/2014 07:37 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 file changed, 8 insertions(+), 6 deletions(-)
 
 diff --git a/builtin/tag.c b/builtin/tag.c
 index c6e8a71..b05f9a5 100644
 --- a/builtin/tag.c
 +++ b/builtin/tag.c
 @@ -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,11 +702,12 @@ 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();
 + if (!transaction ||
 + ref_transaction_update(transaction, ref.buf, object, prev,
 +0, !is_null_sha1(prev), err) ||
 + ref_transaction_commit(transaction, NULL, err))
 + die(%s, err.buf);

If ref_transaction_begin() fails, then won't err still be empty?  (I
know it can't happen, and you know it can't happen, but should the
caller have to know that?)  It almost seems like ref_transaction_begin()
should have an err parameter, too.

It's kindof late for this question to pop into my head, but: have you
thought about embedding the err strbuf in the transaction object?  I
admit it would make the problem with ref_transaction_begin() even worse,
but maybe it would be a net win in terms of boilerplate?

   if (force  !is_null_sha1(prev)  hashcmp(prev, object))
   printf(_(Updated tag '%s' (was %s)\n), tag, 
 find_unique_abbrev(prev, DEFAULT_ABBREV));
  
 


-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 v10 13/44] tag.c: use ref transactions when doing updates

2014-05-16 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 c6e8a71..b05f9a5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -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,11 +702,12 @@ 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();
+   if (!transaction ||
+   ref_transaction_update(transaction, ref.buf, object, prev,
+  0, !is_null_sha1(prev), err) ||
+   ref_transaction_commit(transaction, NULL, err))
+   die(%s, err.buf);
if (force  !is_null_sha1(prev)  hashcmp(prev, object))
printf(_(Updated tag '%s' (was %s)\n), tag, 
find_unique_abbrev(prev, DEFAULT_ABBREV));
 
-- 
2.0.0.rc3.510.g20c254b

--
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 v10 13/44] tag.c: use ref transactions when doing updates

Re: [PATCH v10 13/44] tag.c: use ref transactions when doing updates

Re: [PATCH v10 13/44] tag.c: use ref transactions when doing updates

[PATCH v10 13/44] tag.c: use ref transactions when doing updates

4 matches

Site Navigation

Mail list logo

Footer information