Re: [PATCH 3/4] git-remote rename: support branches->config migration
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 12, 2008, 04:22 UTC
- Message-ID
- <7viqqtshdd.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <20081112020158.GK24201@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
Show 14 quoted lines
> On Tue, Nov 11, 2008 at 04:49:14PM -0800, Junio C Hamano <gitster@pobox.com> wrote: >> There is something fishy going on between 2/4 and 3/4. 2/4 was advertised >> to migrate remotes to config and had a call to migrate_file() for that >> purpose. Here this one now allows to convert branches but there is no >> change to the callsite of migrate_file(). >> >> Which would mean that 2/4 would convert branches/foo too. And this one is >> only to remove the leftover branches/foo file. >> >> Or am I utterly confused? > > The trick is that 2/4 already added support for remotes/foo as it uses > remote_get() and that detects remotes/foo as well, but that is > completely unintentional.
That is not a trick; it merely is a broken code.
The function migrate_file() introduced by [2/4] is called for any remote definition that did not come from config (by definition, it either came from remotes/foo or branches/foo). The function adds the entries for the given remote definition to the config file, and then removes remotes/foo file if the remote definition came from it. So it is a logically consistent change if you only called this function only for remote definitions that came from remotes/foo.
But the function is called for a remote definition that originally came from branches/foo as well. It happily adds the definition to the config, even though it *fails to remove* branches/foo file.
Do you still think 2/4 is a logically contained good change?
If you apply this to 5505 (taken from 3/4, but removing the check for branches/origin file), and look at resulting t/trash*/six/.git/config file, you will see you have already migrated the remote definition to the config.
diff --git i/t/t5505-remote.sh w/t/t5505-remote.sh index 1567631..60bb9e5 100755 --- i/t/t5505-remote.sh +++ w/t/t5505-remote.sh @@ -364,4 +364,15 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' ' test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin") ' -test_done +test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' ' + git clone one six && + origin_url=$(pwd)/one && + (cd six && + git remote rm origin && + echo "$origin_url" > .git/branches/origin && + git remote rename origin origin && + test "$(git config remote.origin.url)" = "$origin_url" && + test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin") +' + +: test_done