Re: [PATCH v6 13/42] commit.c: use ref transactions for updates
On Wed, May 14, 2014 at 6:11 PM, Jonathan Nieder jrnie...@gmail.com wrote:
Ronnie Sahlberg wrote:
[...]
+++ b/builtin/commit.c
@@ -1541,11 +1541,12 @@ int cmd_commit(int argc, const char **argv, const
char *prefix)
[...]
@@ -1667,16 +1668,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);
else
strbuf_addch(sb, '\n');
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();
+ if (!transaction ||
+ ref_transaction_update(transaction, HEAD, sha1,
+current_head ?
+current_head-object.sha1 : NULL,
+0, !!current_head) ||
+ ref_transaction_commit(transaction, sb.buf, err)) {
rollback_index_files();
- die(_(cannot update HEAD ref));
+ die(_(HEAD: cannot update ref: %s), err.buf);
Same question about !transaction (it also applies to later patches but I
won't mention it any more).
The error message changed from
fatal: cannot lock HEAD ref
to
fatal: HEAD: cannot update ref: Cannot lock the ref 'HEAD'.
Does the message from ref_transaction_commit always say what ref
was being updated when it failed? If so, it's tempting to just use
the message as-is:
fatal: Cannot lock the ref 'HEAD'
If the caller should add to the message, it could say something about
the context --- e.g.,
fatal: cannot update HEAD with new commit: cannot lock the ref 'HEAD'
Looking at that,
die(%s, err.buf)
seems simplest since even if git commit was being called in a loop,
it's already clear that git was trying to lock HEAD to advance it.
Changed it to
die(%s, err.buf)
as you suggested.
Many thanks for the reviews so far!
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
Re: [PATCH v6 13/42] commit.c: use ref transactions for updates
Ronnie Sahlberg wrote:
[...]
+++ b/builtin/commit.c
@@ -1541,11 +1541,12 @@ int cmd_commit(int argc, const char **argv, const
char *prefix)
[...]
@@ -1667,16 +1668,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);
else
strbuf_addch(sb, '\n');
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();
+ if (!transaction ||
+ ref_transaction_update(transaction, HEAD, sha1,
+current_head ?
+current_head-object.sha1 : NULL,
+0, !!current_head) ||
+ ref_transaction_commit(transaction, sb.buf, err)) {
rollback_index_files();
- die(_(cannot update HEAD ref));
+ die(_(HEAD: cannot update ref: %s), err.buf);
Same question about !transaction (it also applies to later patches but I
won't mention it any more).
The error message changed from
fatal: cannot lock HEAD ref
to
fatal: HEAD: cannot update ref: Cannot lock the ref 'HEAD'.
Does the message from ref_transaction_commit always say what ref
was being updated when it failed? If so, it's tempting to just use
the message as-is:
fatal: Cannot lock the ref 'HEAD'
If the caller should add to the message, it could say something about
the context --- e.g.,
fatal: cannot update HEAD with new commit: cannot lock the ref 'HEAD'
Looking at that,
die(%s, err.buf)
seems simplest since even if git commit was being called in a loop,
it's already clear that git was trying to lock HEAD to advance it.
Thanks,
Jonathan
--
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 v6 13/42] commit.c: use ref transactions for updates
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(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f0b7906..6117123 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1541,11 +1541,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);
@@ -1667,16 +1668,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);
@@ -1685,9 +1676,15 @@ 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();
+ if (!transaction ||
+ ref_transaction_update(transaction, HEAD, sha1,
+ current_head ?
+ current_head-object.sha1 : NULL,
+ 0, !!current_head) ||
+ ref_transaction_commit(transaction, sb.buf, err)) {
rollback_index_files();
- die(_(cannot update HEAD ref));
+ die(_(HEAD: cannot update ref: %s), err.buf);
}
unlink(git_path(CHERRY_PICK_HEAD));
--
2.0.0.rc1.351.g4d2c8e4
--
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
