Re: [PATCH v2 18/27] update-ref --stdin: Harmonize error messages
Michael Haggerty mhag...@alum.mit.edu writes: Junio, I incorporated your feedback (which so far has only affected commit messages). I also rebased the patch series to the current master. I pushed the result to GitHub [1]. I'll refrain from spamming the list with v3 yet. Thanks; let us know when you are ready ;-) I finished reading the remainder of the v2, and I think I sent out what I found worth commenting on (either positive or negative). I think the next thing to convert to the transaction API would be the ok we know the set of updates from the pusher; let's update all of them in receive-pack? In a sense that is of a lot more real-world impact than the update-ref plumbing. Side note: honestly speaking, I was dissapointed to see that the ref updates by the receive-pack process was not included in the series when I saw the cover letter that said this was a series about transactional updates to refs. Anyway... There are a few things that need to be thought through. Making the update in receive-pack all-or-none is a behaviour change, even though it may be a good one. We may want to allow the user a way to ask for the traditional reject only the ones that cannot be updated. It probably goes like this: - On the wire, a new ref-update-aon capability is advertised from receive-pack to send-pack and can be requested in the opposite direction. - On the git push side, a new --all-or-none option, and optionally a new push.allOrNone configuration, is used to request the ref-update-aon capability over the wire. - On the receive-pack side, a new receive.allOrNone configuration can be used to always update refs in all-or-none fashion, no matter what the pusher says. - The receive-pack uses the ref transaction to update the refs in all-or-none fashion if it has receive.allOrNone, or both sides agree to use ref-update-aon in the capability exchange. If not, it updates the refs in some-may-succeed-some-may-fail fashion, one by one. Or something like that. -- 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 v2 18/27] update-ref --stdin: Harmonize error messages
On 04/01/2014 12:37 AM, Michael Haggerty wrote: On 03/31/2014 11:51 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Make (most of) the error messages for invalid input have the same format [1]: $COMMAND [SP $REFNAME]: $MESSAGE Update the tests accordingly. [1] A few error messages are left with their old form, because $COMMAND and $REFNAME aren't passed all the way down the call stack. Maybe those sites should be changed some day, too. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Up to this point, modulo nits that have been pointed out separately, the series looked reasonably well done. Thanks for the feedback! Would you like me to expand the commit messages to answer the questions that you asked about the previous patches? And if so, do you want a v3 sent to the list already or should I wait for you to review patches 19-27 first? Junio, I incorporated your feedback (which so far has only affected commit messages). I also rebased the patch series to the current master. I pushed the result to GitHub [1]. I'll refrain from spamming the list with v3 yet. Michael [1] Branch ref-transactions at https://github.com/mhagger/git -- 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 v2 18/27] update-ref --stdin: Harmonize error messages
Michael Haggerty mhag...@alum.mit.edu writes:
Make (most of) the error messages for invalid input have the same
format [1]:
$COMMAND [SP $REFNAME]: $MESSAGE
Update the tests accordingly.
[1] A few error messages are left with their old form, because
$COMMAND and $REFNAME aren't passed all the way down the call
stack. Maybe those sites should be changed some day, too.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
Up to this point, modulo nits that have been pointed out separately,
the series looked reasonably well done.
Thanks.
builtin/update-ref.c | 24
t/t1400-update-ref.sh | 32
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index b49a5b0..bbc04b2 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -202,19 +202,19 @@ static const char *parse_cmd_update(struct strbuf
*input, const char *next)
update-ref_name = parse_refname(input, next);
if (!update-ref_name)
- die(update line missing ref);
+ die(update: missing ref);
if (parse_next_sha1(input, next, update-new_sha1,
update, update-ref_name,
PARSE_SHA1_ALLOW_EMPTY))
- die(update %s missing newvalue, update-ref_name);
+ die(update %s: missing newvalue, update-ref_name);
update-have_old = !parse_next_sha1(input, next, update-old_sha1,
update, update-ref_name,
PARSE_SHA1_OLD);
if (*next != line_termination)
- die(update %s has extra input: %s, update-ref_name, next);
+ die(update %s: extra input: %s, update-ref_name, next);
return next;
}
@@ -227,17 +227,17 @@ static const char *parse_cmd_create(struct strbuf
*input, const char *next)
update-ref_name = parse_refname(input, next);
if (!update-ref_name)
- die(create line missing ref);
+ die(create: missing ref);
if (parse_next_sha1(input, next, update-new_sha1,
create, update-ref_name, 0))
- die(create %s missing newvalue, update-ref_name);
+ die(create %s: missing newvalue, update-ref_name);
if (is_null_sha1(update-new_sha1))
- die(create %s given zero newvalue, update-ref_name);
+ die(create %s: zero newvalue, update-ref_name);
if (*next != line_termination)
- die(create %s has extra input: %s, update-ref_name, next);
+ die(create %s: extra input: %s, update-ref_name, next);
return next;
}
@@ -250,19 +250,19 @@ static const char *parse_cmd_delete(struct strbuf
*input, const char *next)
update-ref_name = parse_refname(input, next);
if (!update-ref_name)
- die(delete line missing ref);
+ die(delete: missing ref);
if (parse_next_sha1(input, next, update-old_sha1,
delete, update-ref_name, PARSE_SHA1_OLD)) {
update-have_old = 0;
} else {
if (is_null_sha1(update-old_sha1))
- die(delete %s given zero oldvalue,
update-ref_name);
+ die(delete %s: zero oldvalue, update-ref_name);
update-have_old = 1;
}
if (*next != line_termination)
- die(delete %s has extra input: %s, update-ref_name, next);
+ die(delete %s: extra input: %s, update-ref_name, next);
return next;
}
@@ -275,7 +275,7 @@ static const char *parse_cmd_verify(struct strbuf *input,
const char *next)
update-ref_name = parse_refname(input, next);
if (!update-ref_name)
- die(verify line missing ref);
+ die(verify: missing ref);
if (parse_next_sha1(input, next, update-old_sha1,
verify, update-ref_name, PARSE_SHA1_OLD)) {
@@ -286,7 +286,7 @@ static const char *parse_cmd_verify(struct strbuf *input,
const char *next)
}
if (*next != line_termination)
- die(verify %s has extra input: %s, update-ref_name, next);
+ die(verify %s: extra input: %s, update-ref_name, next);
return next;
}
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 1db0689..48ccc4d 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -371,7 +371,7 @@ test_expect_success 'stdin fails on junk after quoted
argument' '
test_expect_success 'stdin fails create with no ref' '
echo create stdin
test_must_fail git update-ref --stdin stdin 2err
- grep fatal: create line missing ref err
+ grep fatal: create: missing ref err
'
test_expect_success 'stdin fails create with bad ref name' '
@@ -383,19 +383,19 @@ test_expect_success
