All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: git@vger.kernel.org, Thomas Gummerer <t.gummerer@gmail.com>
Cc: peff@peff.net, Johannes.Schindelin@gmx.de, gitster@pobox.com
Subject: [PATCH v2 3/4] remote: actually check if remote exits
Date: Mon, 15 Feb 2016 23:39:43 +0100	[thread overview]
Message-ID: <1455575984-24348-4-git-send-email-t.gummerer@gmail.com> (raw)
In-Reply-To: <1455575984-24348-1-git-send-email-t.gummerer@gmail.com>

When converting the git remote command to a builtin in 211c89 ("Make
git-remote a builtin"), a few calls to check if a remote exists were
converted from:
       if (!exists $remote->{$name}) {
       	  [...]
to:
       remote = remote_get(argv[1]);
       if (!remote)
          [...]

The new check is not quite correct, because remote_get() never returns
NULL if a name is given.  This leaves us with the somewhat cryptic error
message "error: Could not remove config section 'remote.test'", if we
are trying to remove a remote that does not exist, or a similar error if
we try to rename a remote.

Use the remote_is_configured() function to check whether the remote
actually exists, and die with a more sensible error message ("No such
remote: $remotename") instead if it doesn't.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/remote.c  |  4 ++--
 t/t5505-remote.sh | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index d966262..981c487 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -634,7 +634,7 @@ static int mv(int argc, const char **argv)
 	rename.remote_branches = &remote_branches;
 
 	oldremote = remote_get(rename.old);
-	if (!oldremote)
+	if (!remote_is_configured(oldremote))
 		die(_("No such remote: %s"), rename.old);
 
 	if (!strcmp(rename.old, rename.new) && oldremote->origin != REMOTE_CONFIG)
@@ -773,7 +773,7 @@ static int rm(int argc, const char **argv)
 		usage_with_options(builtin_remote_rm_usage, options);
 
 	remote = remote_get(argv[1]);
-	if (!remote)
+	if (!remote_is_configured(remote))
 		die(_("No such remote: %s"), argv[1]);
 
 	known_remotes.to_delete = remote;
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 1a8e3b8..f1d073f 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -139,6 +139,24 @@ test_expect_success 'remove remote protects local branches' '
 	)
 '
 
+test_expect_success 'remove errors out early when deleting non-existent branch' '
+	(
+		cd test &&
+		echo "fatal: No such remote: foo" >expect &&
+		test_must_fail git remote rm foo 2>actual &&
+		test_i18ncmp expect actual
+	)
+'
+
+test_expect_success 'rename errors out early when deleting non-existent branch' '
+	(
+		cd test &&
+		echo "fatal: No such remote: foo" >expect &&
+		test_must_fail git remote rename foo bar 2>actual &&
+		test_i18ncmp expect actual
+	)
+'
+
 cat >test/expect <<EOF
 * remote origin
   Fetch URL: $(pwd)/one
-- 
2.7.1.410.g6faf27b

  parent reply	other threads:[~2016-02-15 22:40 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-15 22:39 [PATCH v2 0/4] git remote improvements Thomas Gummerer
2016-02-15 22:39 ` [PATCH v2 1/4] remote: use parse_config_key Thomas Gummerer
2016-02-15 23:04   ` Jeff King
2016-02-16  0:13     ` Thomas Gummerer
2016-02-15 22:39 ` [PATCH v2 2/4] remote: simplify remote_is_configured() Thomas Gummerer
2016-02-15 22:39 ` Thomas Gummerer [this message]
2016-02-15 22:39 ` [PATCH v2 4/4] remote: use remote_is_configured() for add and rename Thomas Gummerer
2016-02-15 22:52   ` Eric Sunshine
2016-02-15 23:09     ` Jeff King
2016-02-16  0:16       ` Thomas Gummerer
2016-02-16  9:47 ` [PATCH v3 0/4] git remote improvements Thomas Gummerer
2016-02-16  9:47   ` [PATCH v3 1/4] remote: use parse_config_key Thomas Gummerer
2016-02-16  9:47   ` [PATCH v3 2/4] remote: simplify remote_is_configured() Thomas Gummerer
2016-02-16  9:47   ` [PATCH v3 3/4] remote: actually check if remote exits Thomas Gummerer
2016-02-16  9:47   ` [PATCH v3 4/4] remote: use remote_is_configured() for add and rename Thomas Gummerer
2016-02-16 15:52   ` [PATCH v3 0/4] git remote improvements Jeff King
2016-02-16 21:36   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1455575984-24348-4-git-send-email-t.gummerer@gmail.com \
    --to=t.gummerer@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.