receive-pack: utilize rejected ref error details

In 9d2962a7c4 (receive-pack: use batched reference updates, 2025-05-19),
git-receive-pack(1) switched to using batched reference updates. This also
introduced a regression wherein instead of providing detailed error
messages for failed referenced updates, the users were provided generic
error messages based on the error type.

Now that the updates also contain detailed error message, propagate
those to the client via 'rp_error'. The detailed error messages can be
very verbose, for e.g. in the files backend, when trying to write a
non-commit object to a branch, you would see:

   ! [remote rejected] 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d ->
   branch (cannot update ref 'refs/heads/branch': trying to write
   non-commit object 3eaec9ccf3a53f168362a6b3fdeb73426fb9813d to branch
   'refs/heads/branch')

Here the refname is repeated multiple times due to how error messages
are propagated and filled over the code stack. This potentially can be
cleaned up in a future commit.

Reported-by: Elijah Newren <newren@gmail.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-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/receive-pack.c b/builtin/receive-pack.c
index 94d3e73cee3ec20f4633d0858e95a1444bef58df..70e04b3efb31895e486893f16c3fa7966da4cf25 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1813,11 +1813,14 @@ static void ref_transaction_rejection_handler(const char *refname,
                                              const char *old_target UNUSED,
                                              const char *new_target UNUSED,
                                              enum ref_transaction_error err,
-                                             const char *details UNUSED,
+                                             const char *details,
                                              void *cb_data)
 {
        struct strmap *failed_refs = cb_data;
 
+       if (details)
+               rp_error("%s", details);
+
        strmap_put(failed_refs, refname, (char *)ref_transaction_error_msg(err));
 }
 
@@ -1884,6 +1887,7 @@ static void execute_commands_non_atomic(struct command *commands,
                }
 
                ref_transaction_for_each_rejected_update(transaction,
+
                                                         ref_transaction_rejection_handler,
                                                         &failed_refs);
 
@@ -1895,7 +1899,7 @@ static void execute_commands_non_atomic(struct command *commands,
                        if (reported_error)
                                cmd->error_string = reported_error;
                        else if (strmap_contains(&failed_refs, cmd->ref_name))
-                               cmd->error_string = strmap_get(&failed_refs, cmd->ref_name);
+                               cmd->error_string = cmd->error_string_owned = xstrdup(strmap_get(&failed_refs, cmd->ref_name));
                }
 
        cleanup:

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 46926e7bbd3a9a95f5e717591bd3b5ad6dbba2a7..45595991c8d5fedf5fa7b2ffe199240a96aa862a 100755 (executable)
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1882,4 +1882,19 @@ test_expect_success 'push with F/D conflict with deletion and creation' '
        git push testrepo :refs/heads/branch/conflict refs/heads/branch
 '
 
+test_expect_success 'pushing non-commit objects should report error' '
+       test_when_finished "rm -rf dest repo" &&
+       git init dest &&
+       git init repo &&
+
+       (
+               cd repo &&
+               test_commit --annotate test &&
+
+               tagsha=$(git rev-parse test^{tag}) &&
+               test_must_fail git push ../dest "$tagsha:refs/heads/branch" 2>err &&
+               test_grep "trying to write non-commit object $tagsha to branch ${SQ}refs/heads/branch${SQ}" err
+       )
+'
+
 test_done