update-ref: utilize rejected error details if available

When git-update-ref(1) received the '--update-ref' flag, the error
details generated in the refs namespace wasn't propagated with failed
updates. Instead only an error code pertaining to the type of rejection
was noted.

This missed detailed error message which the user can act upon. The
previous commits added the required code to propagate these detailed
error messages from the refs namespace. Now that additional details are
available, let's output this additional details to stderr. This allows
users to have additional information over the already present machine
parsable output.

While we're here, improve the existing tests for the machine parsable
output by checking for the entire output string and not just the
rejection reason.

Reported-by: Elijah Newren <newren@gmail.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 0046a87c579122bdb35351b7f6ac5eb2e642e585..2d68c40ecb70108393865adebc608d9854b04f7a 100644 (file)
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -573,16 +573,18 @@ static void print_rejected_refs(const char *refname,
                                const char *old_target,
                                const char *new_target,
                                enum ref_transaction_error err,
-                               const char *details UNUSED,
+                               const char *details,
                                void *cb_data UNUSED)
 {
        struct strbuf sb = STRBUF_INIT;
-       const char *reason = ref_transaction_error_msg(err);
+
+       if (details && *details)
+               error("%s", details);
 
        strbuf_addf(&sb, "rejected %s %s %s %s\n", refname,
                    new_oid ? oid_to_hex(new_oid) : new_target,
                    old_oid ? oid_to_hex(old_oid) : old_target,
-                   reason);
+                   ref_transaction_error_msg(err));
 
        fwrite(sb.buf, sb.len, 1, stdout);
        strbuf_release(&sb);

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index db7f5444da2162155a2b35b0d1717fc5fc9d77d8..db6585b8d828a58bbb94c36925fcf8b3d7e92804 100755 (executable)
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -2093,14 +2093,15 @@ do
 
                        format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin &&
                        format_command $type "update refs/heads/ref2" "$(test_oid 001)" "$head" >>stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref1 >actual &&
                        test_cmp expect actual &&
                        echo $head >expect &&
                        git rev-parse refs/heads/ref2 >actual &&
                        test_cmp expect actual &&
-                       test_grep -q "invalid new value provided" stdout
+                       test_grep "rejected refs/heads/ref2 $(test_oid 001) $head invalid new value provided" stdout &&
+                       test_grep "trying to write ref ${SQ}refs/heads/ref2${SQ} with nonexistent object" err
                )
        '
 
@@ -2119,14 +2120,15 @@ do
 
                        format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin &&
                        format_command $type "update refs/heads/ref2" "$head_tree" "$head" >>stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref1 >actual &&
                        test_cmp expect actual &&
                        echo $head >expect &&
                        git rev-parse refs/heads/ref2 >actual &&
                        test_cmp expect actual &&
-                       test_grep -q "invalid new value provided" stdout
+                       test_grep "rejected refs/heads/ref2 $head_tree $head invalid new value provided" stdout &&
+                       test_grep "trying to write non-commit object $head_tree to branch ${SQ}refs/heads/ref2${SQ}" err
                )
        '
 
@@ -2143,12 +2145,13 @@ do
 
                        format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin &&
                        format_command $type "update refs/heads/ref2" "$old_head" "$head" >>stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref1 >actual &&
                        test_cmp expect actual &&
                        test_must_fail git rev-parse refs/heads/ref2 &&
-                       test_grep -q "reference does not exist" stdout
+                       test_grep "rejected refs/heads/ref2 $old_head $head reference does not exist" stdout &&
+                       test_grep "cannot lock ref ${SQ}refs/heads/ref2${SQ}: unable to resolve reference ${SQ}refs/heads/ref2${SQ}" err
                )
        '
 
@@ -2166,13 +2169,14 @@ do
 
                        format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin &&
                        format_command $type "update refs/heads/ref2" "$old_head" "$head" >>stdin &&
-                       git update-ref $type --no-deref --stdin --batch-updates <stdin >stdout &&
+                       git update-ref $type --no-deref --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref1 >actual &&
                        test_cmp expect actual &&
                        echo $head >expect &&
                        test_must_fail git rev-parse refs/heads/ref2 &&
-                       test_grep -q "reference does not exist" stdout
+                       test_grep "rejected refs/heads/ref2 $old_head $head reference does not exist" stdout &&
+                       test_grep "cannot lock ref ${SQ}refs/heads/ref2${SQ}: reference is missing but expected $head" err
                )
        '
 
@@ -2190,7 +2194,7 @@ do
 
                        format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin &&
                        format_command $type "symref-update refs/heads/ref2" "$old_head" "ref" "refs/heads/nonexistent" >>stdin &&
-                       git update-ref $type --no-deref --stdin --batch-updates <stdin >stdout &&
+                       git update-ref $type --no-deref --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref1 >actual &&
                        test_cmp expect actual &&
@@ -2198,7 +2202,8 @@ do
                        echo $head >expect &&
                        git rev-parse refs/heads/ref2 >actual &&
                        test_cmp expect actual &&
-                       test_grep -q "expected symref but found regular ref" stdout
+                       test_grep "rejected refs/heads/ref2 $ZERO_OID $ZERO_OID expected symref but found regular ref" stdout &&
+                       test_grep "cannot lock ref ${SQ}refs/heads/ref2${SQ}: expected symref with target ${SQ}refs/heads/nonexistent${SQ}: but is a regular ref" err
                )
        '
 
@@ -2216,14 +2221,15 @@ do
 
                        format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin &&
                        format_command $type "update refs/heads/ref2" "$old_head" "$Z" >>stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref1 >actual &&
                        test_cmp expect actual &&
                        echo $head >expect &&
                        git rev-parse refs/heads/ref2 >actual &&
                        test_cmp expect actual &&
-                       test_grep -q "reference already exists" stdout
+                       test_grep "rejected refs/heads/ref2 $old_head $ZERO_OID reference already exists" stdout &&
+                       test_grep "cannot lock ref ${SQ}refs/heads/ref2${SQ}: reference already exists" err
                )
        '
 
@@ -2241,14 +2247,15 @@ do
 
                        format_command $type "update refs/heads/ref1" "$old_head" "$head" >stdin &&
                        format_command $type "update refs/heads/ref2" "$head" "$old_head" >>stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref1 >actual &&
                        test_cmp expect actual &&
                        echo $head >expect &&
                        git rev-parse refs/heads/ref2 >actual &&
                        test_cmp expect actual &&
-                       test_grep -q "incorrect old value provided" stdout
+                       test_grep "rejected refs/heads/ref2 $head $old_head incorrect old value provided" stdout &&
+                       test_grep "cannot lock ref ${SQ}refs/heads/ref2${SQ}: is at $head but expected $old_head" err
                )
        '
 
@@ -2264,12 +2271,13 @@ do
                        git update-ref refs/heads/ref/foo $head &&
 
                        format_command $type "update refs/heads/ref/foo" "$old_head" "$head" >stdin &&
-                       format_command $type "update refs/heads/ref" "$old_head" "" >>stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
+                       format_command $type "update refs/heads/ref" "$old_head" "$ZERO_OID" >>stdin &&
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref/foo >actual &&
                        test_cmp expect actual &&
-                       test_grep -q "refname conflict" stdout
+                       test_grep "rejected refs/heads/ref $old_head $ZERO_OID refname conflict" stdout &&
+                       test_grep "${SQ}refs/heads/ref/foo${SQ} exists; cannot create ${SQ}refs/heads/ref${SQ}" err
                )
        '
 
@@ -2284,13 +2292,14 @@ do
                        head=$(git rev-parse HEAD) &&
                        git update-ref refs/heads/ref/foo $head &&
 
-                       format_command $type "update refs/heads/foo" "$old_head" "" >stdin &&
-                       format_command $type "update refs/heads/ref" "$old_head" "" >>stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
+                       format_command $type "update refs/heads/foo" "$old_head" "$ZERO_OID" >stdin &&
+                       format_command $type "update refs/heads/ref" "$old_head" "$ZERO_OID" >>stdin &&
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/foo >actual &&
                        test_cmp expect actual &&
-                       test_grep -q "refname conflict" stdout
+                       test_grep "rejected refs/heads/ref $old_head $ZERO_OID refname conflict" stdout &&
+                       test_grep "${SQ}refs/heads/ref/foo${SQ} exists; cannot create ${SQ}refs/heads/ref${SQ}" err
                )
        '
 
@@ -2309,14 +2318,15 @@ do
                                format_command $type "create refs/heads/ref" "$old_head" &&
                                format_command $type "create refs/heads/Foo" "$old_head"
                        } >stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
 
                        echo $head >expect &&
                        git rev-parse refs/heads/foo >actual &&
                        echo $old_head >expect &&
                        git rev-parse refs/heads/ref >actual &&
                        test_cmp expect actual &&
-                       test_grep -q "reference conflict due to case-insensitive filesystem" stdout
+                       test_grep "rejected refs/heads/Foo $old_head $ZERO_OID reference conflict due to case-insensitive filesystem" stdout &&
+                       test_grep -e "cannot lock ref ${SQ}refs/heads/Foo${SQ}: Unable to create" -e "Foo.lock" err
                )
        '
 
@@ -2357,8 +2367,9 @@ do
                        git symbolic-ref refs/heads/symbolic refs/heads/non-existent &&
 
                        format_command $type "delete refs/heads/symbolic" "$head" >stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
-                       test_grep "reference does not exist" stdout
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
+                       test_grep "rejected refs/heads/non-existent $ZERO_OID $head reference does not exist" stdout &&
+                       test_grep "cannot lock ref ${SQ}refs/heads/symbolic${SQ}: unable to resolve reference ${SQ}refs/heads/non-existent${SQ}" err
                )
        '
 
@@ -2373,8 +2384,9 @@ do
                        head=$(git rev-parse HEAD) &&
 
                        format_command $type "delete refs/heads/new-branch" "$head" >stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
-                       test_grep "incorrect old value provided" stdout
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
+                       test_grep "rejected refs/heads/new-branch $ZERO_OID $head incorrect old value provided" stdout &&
+                       test_grep "cannot lock ref ${SQ}refs/heads/new-branch${SQ}: is at $(git rev-parse new-branch) but expected $head" err
                )
        '
 
@@ -2387,8 +2399,9 @@ do
                        head=$(git rev-parse HEAD) &&
 
                        format_command $type "delete refs/heads/non-existent" "$head" >stdin &&
-                       git update-ref $type --stdin --batch-updates <stdin >stdout &&
-                       test_grep "reference does not exist" stdout
+                       git update-ref $type --stdin --batch-updates <stdin >stdout 2>err &&
+                       test_grep "rejected refs/heads/non-existent $ZERO_OID $head reference does not exist" stdout &&
+                       test_grep "cannot lock ref ${SQ}refs/heads/non-existent${SQ}: unable to resolve reference ${SQ}refs/heads/non-existent${SQ}" err
                )
        '
 done