From aab179d937379e28c67dcab0a46fdba05c11d919 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Thu, 3 Dec 2020 13:55:13 -0500 Subject: 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 Signed-off-by: Jeff King Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- builtin/clone.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index a0841923cf..a5630337e4 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1293,8 +1293,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix) break; } - if (!is_local && !complete_refs_before_fetch) - transport_fetch_refs(transport, mapped_refs); + if (!is_local && !complete_refs_before_fetch) { + err = transport_fetch_refs(transport, mapped_refs); + if (err) + goto cleanup; + } remote_head = find_ref_by_name(refs, "HEAD"); remote_head_points_at = @@ -1339,8 +1342,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (is_local) clone_local(path, git_dir); - else if (refs && complete_refs_before_fetch) - transport_fetch_refs(transport, mapped_refs); + else if (refs && complete_refs_before_fetch) { + err = transport_fetch_refs(transport, mapped_refs); + if (err) + goto cleanup; + } update_remote_refs(refs, mapped_refs, remote_head_points_at, branch_top.buf, reflog_msg.buf, transport, @@ -1367,6 +1373,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) junk_mode = JUNK_LEAVE_REPO; err = checkout(submodule_progress); +cleanup: free(remote_name); strbuf_release(&reflog_msg); strbuf_release(&branch_top); -- cgit v1.2.3 From cfaff3aac8063ec72f03c6761328c7fa44a15b34 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 11 Dec 2020 11:36:55 +0000 Subject: branch -m: allow renaming a yet-unborn branch In one of the next commits, we would like to give users some advice regarding the initial branch name, and how to modify it. To that end, it would be good if `git branch -m ` worked in a freshly initialized repository without any commits. Let's make it so. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/branch.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/branch.c b/builtin/branch.c index efb30b8820..200da319f1 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -538,7 +538,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int strbuf_addf(&logmsg, "Branch: renamed %s to %s", oldref.buf, newref.buf); - if (!copy && rename_ref(oldref.buf, newref.buf, logmsg.buf)) + if (!copy && + (!head || strcmp(oldname, head) || !is_null_oid(&head_oid)) && + rename_ref(oldref.buf, newref.buf, logmsg.buf)) die(_("Branch rename failed")); if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf)) die(_("Branch copy failed")); -- cgit v1.2.3 From cc0f13c57dedaf62c9f852b6bf363aee7e3392f1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 11 Dec 2020 11:36:56 +0000 Subject: get_default_branch_name(): prepare for showing some advice We are about to introduce a message giving users running `git init` some advice about `init.defaultBranch`. This will necessarily be done in `repo_default_branch_name()`. Not all code paths want to show that advice, though. In particular, the `git clone` codepath _specifically_ asks for `init_db()` to be quiet, via the `INIT_DB_QUIET` flag. In preparation for showing users above-mentioned advice, let's change the function signature of `get_default_branch_name()` to accept the parameter `quiet`. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/clone.c | 2 +- builtin/init-db.c | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index a0841923cf..64b1784011 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1323,7 +1323,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) remote_head = NULL; option_no_checkout = 1; if (!option_bare) { - const char *branch = git_default_branch_name(); + const char *branch = git_default_branch_name(0); char *ref = xstrfmt("refs/heads/%s", branch); install_branch_config(0, branch, remote_name, ref); diff --git a/builtin/init-db.c b/builtin/init-db.c index 01bc648d41..dcc45bef51 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -202,7 +202,8 @@ void initialize_repository_version(int hash_algo, int reinit) static int create_default_files(const char *template_path, const char *original_git_dir, const char *initial_branch, - const struct repository_format *fmt) + const struct repository_format *fmt, + int quiet) { struct stat st1; struct strbuf buf = STRBUF_INIT; @@ -267,7 +268,7 @@ static int create_default_files(const char *template_path, char *ref; if (!initial_branch) - initial_branch = git_default_branch_name(); + initial_branch = git_default_branch_name(quiet); ref = xstrfmt("refs/heads/%s", initial_branch); if (check_refname_format(ref, 0) < 0) @@ -438,7 +439,8 @@ int init_db(const char *git_dir, const char *real_git_dir, validate_hash_algorithm(&repo_fmt, hash); reinit = create_default_files(template_dir, original_git_dir, - initial_branch, &repo_fmt); + initial_branch, &repo_fmt, + flags & INIT_DB_QUIET); if (reinit && initial_branch) warning(_("re-init: ignored --initial-branch=%s"), initial_branch); -- cgit v1.2.3 From 56f56ac50b932310c629832fad256e624ca451e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 16 Dec 2020 00:50:27 +0100 Subject: style: do not "break" in switch() after "return" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove this unreachable code. It was found by SunCC, it's found by a non-fatal warning emitted by SunCC. It's one of the things it's more vehement about than GCC & Clang. It complains about a lot of other similarly unreachable code, e.g. a BUG(...) without a "return", and a "return 0" after a long if/else, both of whom have "return" statements. Those are also genuine redundancies to a compiler, but arguably make the code a bit easier to read & less fragile to maintain. These return/break cases are just unnecessary however, and as seen here the surrounding code just did a plain "return" without a "break" already. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/fast-export.c | 1 - 1 file changed, 1 deletion(-) (limited to 'builtin') diff --git a/builtin/fast-export.c b/builtin/fast-export.c index d2e33f5005..0a60356b06 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -923,7 +923,6 @@ static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name) if (!tag) die("Tag %s points nowhere?", e->name); return (struct commit *)tag; - break; } default: return NULL; -- cgit v1.2.3