diff --git a/builtin/fetch.c b/builtin/fetch.c index 288d3772ea..3a3f1d8914 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -861,12 +861,87 @@ static void display_ref_update(struct display_state *display_state, char code, fputs(display_state->buf.buf, f); } +struct ref_update_display_info { + bool failed; + char success_code; + char fail_code; + const char *summary; + const char *fail_detail; + const char *success_detail; + const char *ref; + const char *remote; + struct object_id old_oid; + struct object_id new_oid; +}; + +static struct ref_update_display_info *ref_update_display_info_append( + struct ref_update_display_info **list, + size_t *count, + char success_code, + char fail_code, + const char *summary, + const char *success_detail, + const char *fail_detail, + const char *ref, + const char *remote, + const struct object_id *old_oid, + const struct object_id *new_oid) +{ + struct ref_update_display_info *info; + size_t index = *count; + + (*count)++; + REALLOC_ARRAY(*list, *count); + + info = &(*list)[index]; + + info->failed = false; + info->success_code = success_code; + info->fail_code = fail_code; + info->summary = xstrdup(summary); + info->success_detail = xstrdup_or_null(success_detail); + info->fail_detail = xstrdup_or_null(fail_detail); + info->remote = xstrdup(remote); + info->ref = xstrdup(ref); + + oidcpy(&info->old_oid, old_oid); + oidcpy(&info->new_oid, new_oid); + + return info; +} + +static void ref_update_display_info_set_failed(struct ref_update_display_info *info) +{ + info->failed = true; +} + +static void ref_update_display_info_free(struct ref_update_display_info *info) +{ + free((char *)info->summary); + free((char *)info->success_detail); + free((char *)info->fail_detail); + free((char *)info->remote); + free((char *)info->ref); +} + +static void ref_update_display_info_display(struct ref_update_display_info *info, + struct display_state *display_state, + int summary_width) +{ + display_ref_update(display_state, + info->failed ? info->fail_code : info->success_code, + info->summary, + info->failed ? info->fail_detail : info->success_detail, + info->remote, info->ref, &info->old_oid, + &info->new_oid, summary_width); +} + static int update_local_ref(struct ref *ref, struct ref_transaction *transaction, - struct display_state *display_state, const struct ref *remote_ref, - int summary_width, - const struct fetch_config *config) + const struct fetch_config *config, + struct ref_update_display_info **display_list, + size_t *display_count) { struct commit *current = NULL, *updated; int fast_forward = 0; @@ -877,41 +952,56 @@ static int update_local_ref(struct ref *ref, if (oideq(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) - display_ref_update(display_state, '=', _("[up to date]"), NULL, - remote_ref->name, ref->name, - &ref->old_oid, &ref->new_oid, summary_width); + ref_update_display_info_append(display_list, display_count, + '=', '=', _("[up to date]"), + NULL, NULL, ref->name, + remote_ref->name, &ref->old_oid, + &ref->new_oid); return 0; } if (!update_head_ok && !is_null_oid(&ref->old_oid) && branch_checked_out(ref->name)) { + struct ref_update_display_info *info; /* * If this is the head, and it's not okay to update * the head, and the old value of the head isn't empty... */ - display_ref_update(display_state, '!', _("[rejected]"), - _("can't fetch into checked-out branch"), - remote_ref->name, ref->name, - &ref->old_oid, &ref->new_oid, summary_width); + info = ref_update_display_info_append(display_list, display_count, + '!', '!', _("[rejected]"), + NULL, _("can't fetch into checked-out branch"), + ref->name, remote_ref->name, + &ref->old_oid, &ref->new_oid); + ref_update_display_info_set_failed(info); return 1; } if (!is_null_oid(&ref->old_oid) && starts_with(ref->name, "refs/tags/")) { + struct ref_update_display_info *info; + if (force || ref->force) { int r; + r = s_update_ref("updating tag", ref, transaction, 0); - display_ref_update(display_state, r ? '!' : 't', _("[tag update]"), - r ? _("unable to update local ref") : NULL, - remote_ref->name, ref->name, - &ref->old_oid, &ref->new_oid, summary_width); + + info = ref_update_display_info_append(display_list, display_count, + 't', '!', _("[tag update]"), NULL, + _("unable to update local ref"), + ref->name, remote_ref->name, + &ref->old_oid, &ref->new_oid); + if (r) + ref_update_display_info_set_failed(info); + return r; } else { - display_ref_update(display_state, '!', _("[rejected]"), - _("would clobber existing tag"), - remote_ref->name, ref->name, - &ref->old_oid, &ref->new_oid, summary_width); + info = ref_update_display_info_append(display_list, display_count, + '!', '!', _("[rejected]"), NULL, + _("would clobber existing tag"), + ref->name, remote_ref->name, + &ref->old_oid, &ref->new_oid); + ref_update_display_info_set_failed(info); return 1; } } @@ -921,6 +1011,7 @@ static int update_local_ref(struct ref *ref, updated = lookup_commit_reference_gently(the_repository, &ref->new_oid, 1); if (!current || !updated) { + struct ref_update_display_info *info; const char *msg; const char *what; int r; @@ -941,10 +1032,15 @@ static int update_local_ref(struct ref *ref, } r = s_update_ref(msg, ref, transaction, 0); - display_ref_update(display_state, r ? '!' : '*', what, - r ? _("unable to update local ref") : NULL, - remote_ref->name, ref->name, - &ref->old_oid, &ref->new_oid, summary_width); + + info = ref_update_display_info_append(display_list, display_count, + '*', '!', what, NULL, + _("unable to update local ref"), + ref->name, remote_ref->name, + &ref->old_oid, &ref->new_oid); + if (r) + ref_update_display_info_set_failed(info); + return r; } @@ -960,6 +1056,7 @@ static int update_local_ref(struct ref *ref, } if (fast_forward) { + struct ref_update_display_info *info; struct strbuf quickref = STRBUF_INIT; int r; @@ -967,29 +1064,46 @@ static int update_local_ref(struct ref *ref, strbuf_addstr(&quickref, ".."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); r = s_update_ref("fast-forward", ref, transaction, 1); - display_ref_update(display_state, r ? '!' : ' ', quickref.buf, - r ? _("unable to update local ref") : NULL, - remote_ref->name, ref->name, - &ref->old_oid, &ref->new_oid, summary_width); + + info = ref_update_display_info_append(display_list, display_count, + ' ', '!', quickref.buf, NULL, + _("unable to update local ref"), + ref->name, remote_ref->name, + &ref->old_oid, &ref->new_oid); + if (r) + ref_update_display_info_set_failed(info); + strbuf_release(&quickref); return r; } else if (force || ref->force) { + struct ref_update_display_info *info; struct strbuf quickref = STRBUF_INIT; int r; + strbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV); strbuf_addstr(&quickref, "..."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); r = s_update_ref("forced-update", ref, transaction, 1); - display_ref_update(display_state, r ? '!' : '+', quickref.buf, - r ? _("unable to update local ref") : _("forced update"), - remote_ref->name, ref->name, - &ref->old_oid, &ref->new_oid, summary_width); + + info = ref_update_display_info_append(display_list, display_count, + '+', '!', quickref.buf, _("forced update"), + _("unable to update local ref"), + ref->name, remote_ref->name, + &ref->old_oid, &ref->new_oid); + + if (r) + ref_update_display_info_set_failed(info); + strbuf_release(&quickref); return r; } else { - display_ref_update(display_state, '!', _("[rejected]"), _("non-fast-forward"), - remote_ref->name, ref->name, - &ref->old_oid, &ref->new_oid, summary_width); + struct ref_update_display_info *info; + info = ref_update_display_info_append(display_list, display_count, + '!', '!', _("[rejected]"), NULL, + _("non-fast-forward"), + ref->name, remote_ref->name, + &ref->old_oid, &ref->new_oid); + ref_update_display_info_set_failed(info); return 1; } } @@ -1103,17 +1217,15 @@ static int store_updated_refs(struct display_state *display_state, int connectivity_checked, struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head, - const struct fetch_config *config) + const struct fetch_config *config, + struct ref_update_display_info **display_list, + size_t *display_count) { int rc = 0; struct strbuf note = STRBUF_INIT; const char *what, *kind; struct ref *rm; int want_status; - int summary_width = 0; - - if (verbosity >= 0) - summary_width = transport_summary_width(ref_map); if (!connectivity_checked) { struct check_connected_options opt = CHECK_CONNECTED_INIT; @@ -1218,8 +1330,9 @@ static int store_updated_refs(struct display_state *display_state, display_state->url_len); if (ref) { - rc |= update_local_ref(ref, transaction, display_state, - rm, summary_width, config); + rc |= update_local_ref(ref, transaction, rm, + config, display_list, + display_count); free(ref); } else if (write_fetch_head || dry_run) { /* @@ -1227,12 +1340,11 @@ static int store_updated_refs(struct display_state *display_state, * would be written to FETCH_HEAD, if --dry-run * is set). */ - display_ref_update(display_state, '*', - *kind ? kind : "branch", NULL, - rm->name, - "FETCH_HEAD", - &rm->new_oid, &rm->old_oid, - summary_width); + + ref_update_display_info_append(display_list, display_count, + '*', '*', *kind ? kind : "branch", + NULL, NULL, "FETCH_HEAD", rm->name, + &rm->new_oid, &rm->old_oid); } } } @@ -1300,7 +1412,9 @@ static int fetch_and_consume_refs(struct display_state *display_state, struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head, - const struct fetch_config *config) + const struct fetch_config *config, + struct ref_update_display_info **display_list, + size_t *display_count) { int connectivity_checked = 1; int ret; @@ -1322,7 +1436,8 @@ static int fetch_and_consume_refs(struct display_state *display_state, trace2_region_enter("fetch", "consume_refs", the_repository); ret = store_updated_refs(display_state, connectivity_checked, - transaction, ref_map, fetch_head, config); + transaction, ref_map, fetch_head, config, + display_list, display_count); trace2_region_leave("fetch", "consume_refs", the_repository); out: @@ -1493,7 +1608,9 @@ static int backfill_tags(struct display_state *display_state, struct ref_transaction *transaction, struct ref *ref_map, struct fetch_head *fetch_head, - const struct fetch_config *config) + const struct fetch_config *config, + struct ref_update_display_info **display_list, + size_t *display_count) { int retcode, cannot_reuse; @@ -1515,7 +1632,7 @@ static int backfill_tags(struct display_state *display_state, transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); retcode = fetch_and_consume_refs(display_state, transport, transaction, ref_map, - fetch_head, config); + fetch_head, config, display_list, display_count); if (gsecondary) { transport_disconnect(gsecondary); @@ -1641,6 +1758,7 @@ struct ref_rejection_data { bool conflict_msg_shown; bool case_sensitive_msg_shown; const char *remote_name; + struct strmap *rejected_refs; }; static void ref_transaction_rejection_handler(const char *refname, @@ -1649,6 +1767,7 @@ 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, void *cb_data) { struct ref_rejection_data *data = cb_data; @@ -1673,11 +1792,14 @@ static void ref_transaction_rejection_handler(const char *refname, "branches"), data->remote_name); data->conflict_msg_shown = true; } else { - const char *reason = ref_transaction_error_msg(err); - - error(_("fetching ref %s failed: %s"), refname, reason); + if (details) + error("%s", details); + else + error(_("fetching ref %s failed: %s"), + refname, ref_transaction_error_msg(err)); } + strmap_put(data->rejected_refs, refname, NULL); *data->retcode = 1; } @@ -1687,6 +1809,7 @@ static void ref_transaction_rejection_handler(const char *refname, */ static int commit_ref_transaction(struct ref_transaction **transaction, bool is_atomic, const char *remote_name, + struct strmap *rejected_refs, struct strbuf *err) { int retcode = ref_transaction_commit(*transaction, err); @@ -1698,6 +1821,7 @@ static int commit_ref_transaction(struct ref_transaction **transaction, .conflict_msg_shown = 0, .remote_name = remote_name, .retcode = &retcode, + .rejected_refs = rejected_refs, }; ref_transaction_for_each_rejected_update(*transaction, @@ -1726,6 +1850,10 @@ static int do_fetch(struct transport *transport, struct fetch_head fetch_head = { 0 }; struct strbuf err = STRBUF_INIT; int do_set_head = 0; + struct ref_update_display_info *display_list = NULL; + struct strmap rejected_refs = STRMAP_INIT; + size_t display_count = 0; + int summary_width = 0; if (tags == TAGS_DEFAULT) { if (transport->remote->fetch_tags == 2) @@ -1850,7 +1978,7 @@ static int do_fetch(struct transport *transport, } if (fetch_and_consume_refs(&display_state, transport, transaction, ref_map, - &fetch_head, config)) { + &fetch_head, config, &display_list, &display_count)) { retcode = 1; goto cleanup; } @@ -1873,7 +2001,7 @@ static int do_fetch(struct transport *transport, * the transaction and don't commit anything. */ if (backfill_tags(&display_state, transport, transaction, tags_ref_map, - &fetch_head, config)) + &fetch_head, config, &display_list, &display_count)) retcode = 1; } @@ -1883,8 +2011,12 @@ static int do_fetch(struct transport *transport, if (retcode) goto cleanup; + if (verbosity >= 0) + summary_width = transport_summary_width(ref_map); + retcode = commit_ref_transaction(&transaction, atomic_fetch, - transport->remote->name, &err); + transport->remote->name, + &rejected_refs, &err); /* * With '--atomic', bail out if the transaction fails. Without '--atomic', * continue to fetch head and perform other post-fetch operations. @@ -1962,7 +2094,17 @@ cleanup: */ if (retcode && !atomic_fetch && transaction) commit_ref_transaction(&transaction, false, - transport->remote->name, &err); + transport->remote->name, + &rejected_refs, &err); + + for (size_t i = 0; i < display_count; i++) { + struct ref_update_display_info *info = &display_list[i]; + + if (!info->failed && strmap_contains(&rejected_refs, info->ref)) + ref_update_display_info_set_failed(info); + ref_update_display_info_display(info, &display_state, summary_width); + ref_update_display_info_free(info); + } if (retcode) { if (err.len) { @@ -1977,6 +2119,9 @@ cleanup: if (transaction) ref_transaction_free(transaction); + + free(display_list); + strmap_clear(&rejected_refs, 0); display_state_release(&display_state); close_fetch_head(&fetch_head); strbuf_release(&err); diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9c49174616..d1aacf8b2a 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1853,10 +1853,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, 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)); } @@ -1923,6 +1927,7 @@ static void execute_commands_non_atomic(struct command *commands, } ref_transaction_for_each_rejected_update(transaction, + ref_transaction_rejection_handler, &failed_refs); @@ -1934,7 +1939,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/builtin/update-ref.c b/builtin/update-ref.c index 195437e7c6..2d68c40ecb 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -573,15 +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, 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/refs.c b/refs.c index 627b7f8698..600913b99f 100644 --- a/refs.c +++ b/refs.c @@ -1267,6 +1267,7 @@ void ref_transaction_free(struct ref_transaction *transaction) free(transaction->updates[i]->committer_info); free((char *)transaction->updates[i]->new_target); free((char *)transaction->updates[i]->old_target); + free((char *)transaction->updates[i]->rejection_details); free(transaction->updates[i]); } @@ -1281,7 +1282,8 @@ void ref_transaction_free(struct ref_transaction *transaction) int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction, size_t update_idx, - enum ref_transaction_error err) + enum ref_transaction_error err, + struct strbuf *details) { if (update_idx >= transaction->nr) BUG("trying to set rejection on invalid update index"); @@ -1307,6 +1309,7 @@ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction, transaction->updates[update_idx]->refname, 0); transaction->updates[update_idx]->rejection_err = err; + transaction->updates[update_idx]->rejection_details = strbuf_detach(details, NULL); ALLOC_GROW(transaction->rejections->update_indices, transaction->rejections->nr + 1, transaction->rejections->alloc); @@ -2698,30 +2701,33 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs if (!initial_transaction && (strset_contains(&conflicting_dirnames, dirname.buf) || !refs_read_raw_ref(refs, dirname.buf, &oid, &referent, - &type, &ignore_errno))) { - if (transaction && ref_transaction_maybe_set_rejected( - transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) { - strset_remove(&dirnames, dirname.buf); - strset_add(&conflicting_dirnames, dirname.buf); - continue; - } + &type, &ignore_errno))) { strbuf_addf(err, _("'%s' exists; cannot create '%s'"), dirname.buf, refname); + + if (transaction && ref_transaction_maybe_set_rejected( + transaction, *update_idx, + REF_TRANSACTION_ERROR_NAME_CONFLICT, err)) { + strset_remove(&dirnames, dirname.buf); + strset_add(&conflicting_dirnames, dirname.buf); + goto next_ref; + } + goto cleanup; } if (extras && string_list_has_string(extras, dirname.buf)) { - if (transaction && ref_transaction_maybe_set_rejected( - transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) { - strset_remove(&dirnames, dirname.buf); - continue; - } - strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), refname, dirname.buf); + + if (transaction && ref_transaction_maybe_set_rejected( + transaction, *update_idx, + REF_TRANSACTION_ERROR_NAME_CONFLICT, err)) { + strset_remove(&dirnames, dirname.buf); + goto next_ref; + } + goto cleanup; } } @@ -2751,14 +2757,14 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs if (skip && string_list_has_string(skip, iter->ref.name)) continue; + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + iter->ref.name, refname); if (transaction && ref_transaction_maybe_set_rejected( transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) - continue; + REF_TRANSACTION_ERROR_NAME_CONFLICT, err)) + goto next_ref; - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - iter->ref.name, refname); goto cleanup; } @@ -2768,15 +2774,17 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs extra_refname = find_descendant_ref(dirname.buf, extras, skip); if (extra_refname) { - if (transaction && ref_transaction_maybe_set_rejected( - transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) - continue; - strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), refname, extra_refname); + + if (transaction && ref_transaction_maybe_set_rejected( + transaction, *update_idx, + REF_TRANSACTION_ERROR_NAME_CONFLICT, err)) + goto next_ref; + goto cleanup; } +next_ref:; } ret = 0; @@ -2905,7 +2913,7 @@ void ref_transaction_for_each_rejected_update(struct ref_transaction *transactio (update->flags & REF_HAVE_OLD) ? &update->old_oid : NULL, (update->flags & REF_HAVE_NEW) ? &update->new_oid : NULL, update->old_target, update->new_target, - update->rejection_err, cb_data); + update->rejection_err, update->rejection_details, cb_data); } } diff --git a/refs.h b/refs.h index f0abfa1d93..f16b1b697b 100644 --- a/refs.h +++ b/refs.h @@ -993,6 +993,7 @@ typedef void ref_transaction_for_each_rejected_update_fn(const char *refname, const char *old_target, const char *new_target, enum ref_transaction_error err, + const char *details, void *cb_data); void ref_transaction_for_each_rejected_update(struct ref_transaction *transaction, ref_transaction_for_each_rejected_update_fn cb, diff --git a/refs/files-backend.c b/refs/files-backend.c index 240d3c3b26..b1b13b41f6 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2978,10 +2978,9 @@ static int files_transaction_prepare(struct ref_store *ref_store, head_ref, &refnames_to_check, err); if (ret) { - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { - strbuf_reset(err); + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err)) { ret = 0; - continue; } goto cleanup; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 4ea0c12299..59b3ecb9d6 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1437,8 +1437,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re update->refname); ret = REF_TRANSACTION_ERROR_CREATE_EXISTS; - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { - strbuf_reset(err); + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err)) { ret = 0; continue; } @@ -1452,8 +1452,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re oid_to_hex(&update->old_oid)); ret = REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE; - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { - strbuf_reset(err); + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err)) { ret = 0; continue; } @@ -1496,8 +1496,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re oid_to_hex(&update->old_oid)); ret = REF_TRANSACTION_ERROR_NONEXISTENT_REF; - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { - strbuf_reset(err); + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err)) { ret = 0; continue; } diff --git a/refs/refs-internal.h b/refs/refs-internal.h index c7d2a6e50b..191a25683f 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -128,6 +128,7 @@ struct ref_update { * was rejected. */ enum ref_transaction_error rejection_err; + const char *rejection_details; /* * If this ref_update was split off of a symref update via @@ -153,7 +154,8 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname, */ int ref_transaction_maybe_set_rejected(struct ref_transaction *transaction, size_t update_idx, - enum ref_transaction_error err); + enum ref_transaction_error err, + struct strbuf *details); /* * Add a ref_update with the specified properties to transaction, and diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index fe74af73af..5611808ad7 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1418,10 +1418,9 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, &refnames_to_check, head_type, &head_referent, &referent, err); if (ret) { - if (ref_transaction_maybe_set_rejected(transaction, i, ret)) { - strbuf_reset(err); + if (ref_transaction_maybe_set_rejected(transaction, i, + ret, err)) { ret = 0; - continue; } goto done; diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index db7f5444da..db6585b8d8 100755 --- 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 stdout && + git update-ref $type --stdin --batch-updates 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 stdout && + git update-ref $type --stdin --batch-updates 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 stdout && + git update-ref $type --stdin --batch-updates 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 stdout && + git update-ref $type --no-deref --stdin --batch-updates 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 stdout && + git update-ref $type --no-deref --stdin --batch-updates 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 stdout && + git update-ref $type --stdin --batch-updates 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 stdout && + git update-ref $type --stdin --batch-updates 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 stdout && + format_command $type "update refs/heads/ref" "$old_head" "$ZERO_OID" >>stdin && + git update-ref $type --stdin --batch-updates 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 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 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 stdout && + git update-ref $type --stdin --batch-updates 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 stdout && - test_grep "reference does not exist" stdout + git update-ref $type --stdin --batch-updates 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 stdout && - test_grep "incorrect old value provided" stdout + git update-ref $type --stdin --batch-updates 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 stdout && - test_grep "reference does not exist" stdout + git update-ref $type --stdin --batch-updates 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 diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ce1c23684e..c69afb5a60 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -1516,7 +1516,7 @@ test_expect_success REFFILES 'existing reference lock in repo' ' git remote add origin ../base && touch refs/heads/foo.lock && test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err && - test_grep "error: fetching ref refs/heads/foo failed: reference already exists" err && + test_grep -e "error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create" -e "refs/heads/foo.lock${SQ}: File exists." err && git rev-parse refs/heads/main >expect && git rev-parse refs/heads/branch >actual && test_cmp expect actual @@ -1530,7 +1530,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'F/D conflict on case insensiti cd case_insensitive && git remote add origin -- ../case_sensitive_fd && test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err && - test_grep "failed: refname conflict" err && + test_grep "cannot process ${SQ}refs/remotes/origin/foo${SQ} and ${SQ}refs/remotes/origin/foo/bar${SQ} at the same time" err && git rev-parse refs/heads/main >expect && git rev-parse refs/heads/foo/bar >actual && test_cmp expect actual @@ -1544,7 +1544,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'D/F conflict on case insensiti cd case_insensitive && git remote add origin -- ../case_sensitive_df && test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err && - test_grep "failed: refname conflict" err && + test_grep "cannot lock ref ${SQ}refs/remotes/origin/foo${SQ}: there is a non-empty directory ${SQ}./refs/remotes/origin/foo${SQ} blocking reference ${SQ}refs/remotes/origin/foo${SQ}" err && git rev-parse refs/heads/main >expect && git rev-parse refs/heads/Foo/bar >actual && test_cmp expect actual @@ -1658,7 +1658,7 @@ test_expect_success REFFILES "FETCH_HEAD is updated even if ref updates fail" ' git remote add origin ../base && >refs/heads/foo.lock && test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err && - test_grep "error: fetching ref refs/heads/foo failed: reference already exists" err && + test_grep -e "error: cannot lock ref ${SQ}refs/heads/foo${SQ}: Unable to create" -e "refs/heads/foo.lock${SQ}: File exists." err && test_grep "branch ${SQ}branch${SQ} of ../base" FETCH_HEAD && test_grep "branch ${SQ}foo${SQ} of ../base" FETCH_HEAD ) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 46926e7bbd..29e2f17608 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1882,4 +1882,20 @@ 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 "! \[remote rejected\] $tagsha -> branch (invalid new value provided)" err && + test_grep "trying to write non-commit object $tagsha to branch ${SQ}refs/heads/branch${SQ}" err + ) +' + test_done