builtin/clone.c: don't ignore transport_fetch_refs() errors

If 'git clone' couldn't execute 'transport_fetch_refs()' (e.g., because of an error on the remote's side in 'git upload-pack'), then it will silently ignore it. Even though this has been the case at least since clone was ported to C (way back in 8434c2f1af (Build in clone, 2008-04-27)), 'git fetch' doesn't ignore these and reports any failures it sees. That suggests that ignoring the return value in 'git clone' is simply an oversight that should be corrected. That's exactly what this patch does. (Noticing and fixing this is no coincidence, we'll want it in the next patch in order to demonstrate a regression in 'git upload-pack' via a 'git clone'.) There's no additional logging here, but that matches how 'git fetch' handles the same case. An assumption there is that whichever part of transport_fetch_refs() fails will complain loudly, so any additional logging here is redundant. Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Dec 3, 2020 at 13:55 UTC aab179d937379e28c67dcab0a46fdba05c11d919
1 file changed +11 -4
builtin/clone.c
+11 -4
@@ -1293,8 +1293,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1293 break;
1294 }
1295
1296 - if (!is_local && !complete_refs_before_fetch)
1297 - transport_fetch_refs(transport, mapped_refs);
1296 + if (!is_local && !complete_refs_before_fetch) {
1297 + err = transport_fetch_refs(transport, mapped_refs);
1298 + if (err)
1299 + goto cleanup;
1300 + }
1301
1302 remote_head = find_ref_by_name(refs, "HEAD");
1303 remote_head_points_at =
@@ -1339,8 +1342,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1342
1343 if (is_local)
1344 clone_local(path, git_dir);
1342 - else if (refs && complete_refs_before_fetch)
1343 - transport_fetch_refs(transport, mapped_refs);
1345 + else if (refs && complete_refs_before_fetch) {
1346 + err = transport_fetch_refs(transport, mapped_refs);
1347 + if (err)
1348 + goto cleanup;
1349 + }
1350
1351 update_remote_refs(refs, mapped_refs, remote_head_points_at,
1352 branch_top.buf, reflog_msg.buf, transport,
@@ -1367,6 +1373,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1373 junk_mode = JUNK_LEAVE_REPO;
1374 err = checkout(submodule_progress);
1375
1376 +cleanup:
1377 free(remote_name);
1378 strbuf_release(&reflog_msg);
1379 strbuf_release(&branch_top);