From: Eric Sunshine Date: Mon, 15 Feb 2016 22:52:14 GMT Subject: Re: [PATCH v2 4/4] remote: use remote_is_configured() for add and rename Message-ID: In-Reply-To: <1455575984-24348-5-git-send-email-t.gummerer@gmail.com> On Mon, Feb 15, 2016 at 5:39 PM, Thomas Gummerer wrote: > Both remote add and remote rename use a slightly different hand-rolled > check if the remote exits. The hand-rolled check may have some subtle > cases in which it might fail to detect when a remote already exists. > One such case was fixed in fb86e32 ("git remote: allow adding remotes > agreeing with url.<...>.insteadOf"). Another case is when a remote is > configured as follows: > [...] > Signed-off-by: Thomas Gummerer > --- > diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh > @@ -157,6 +157,24 @@ test_expect_success 'rename errors out early when deleting non-existent branch' > +test_expect_success 'add existing foreign_vcs remote' ' > + git config --add remote.foo.vcs "bar" && > + git config --add remote.bar.vcs "bar" && > + test_when_finished git remote rm foo && > + test_when_finished git remote rm bar && Nit: If the second git-config fails, then none of the cleanup will happen. You'd either want to re-order them like this: git config --add remote.foo.vcs "bar" && test_when_finished git remote rm foo && git config --add remote.bar.vcs "bar" && test_when_finished git remote rm bar && or this: test_when_finished git remote rm foo && git config --add remote.foo.vcs "bar" && test_when_finished git remote rm bar && git config --add remote.bar.vcs "bar" && or this: test_when_finished git remote rm foo && test_when_finished git remote rm bar && git config --add remote.foo.vcs "bar" && git config --add remote.bar.vcs "bar" && Probably not worth a re-roll, though. > + echo "fatal: remote bar already exists." >expect && > + test_must_fail git remote rename foo bar 2>actual && > + test_i18ncmp expect actual > +'