Re: [PATCH 4/4] builtin-remote: add set-head verb

From: Jeff King <peff@peff.net>
Date: Feb 14, 2009, 02:18 UTC
Message-ID: <20090214021831.GB9907@coredump.intra.peff.net>
In-Reply-To: <7vab8p4w1u.fsf@gitster.siamese.dyndns.org>

On Fri, Feb 13, 2009 at 06:00:29PM -0800, Junio C Hamano wrote:

Show 5 quoted lines

> > It was immediately obvious to me as "auto" (I think I even suggested
> > "-a" in another thread, so maybe that is why it seems so sensible to
> > me).
> 
> Yeah, latest round has --auto in it.  Thanks, Jay.

Oops. I even went back and double-checked to make sure that it was not there, but it would have helped if I actually checked the latest version. :(

Show 5 quoted lines

> I do not care too deeply if an explicit request to "set-head --auto"
> screws up and sets a HEAD that was pointing at the right branch to another
> branch because the command is not taught to give preference to the branch
> HEAD originally points at, so I do not think I have any more issues with
> the series for now, even though I may notice things later.

I think that is reasonable; that is a separate enhancement which can come later and is no reason to block the existing patches.

> I have this series queued to private topic branch but it still does not
> pass tests (breaks #8 and #18 of t5505 at least) by itself; the previous
> round was no better.  I think it is just the matter of updating the
> expected output in the tests, but I didn't look further.

Test #8 is just a matter of updating output. But #18 is explicitly about checking that "remote show" does not show symbolic refs. But Jay's patch is about explicitly showing symbolic refs (just doing so as a ref-name instead of a sha1):

  * FAIL 18: "remote show" does not show symbolic refs
                git clone one three &&
                (cd three &&
                 git remote show origin > output &&
                 ! grep HEAD < output &&
                 ! grep -i stale < output)

I guess we could tighten the grep to

  ! egrep "HEAD: [0-9a-f]{40}" < output

but it may just make sense to get rid of the test; the exact output is already covered by test #8. Squashable patch is below.

---

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index eb63718..8808580 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -136,6 +136,7 @@ EOF
 cat > test/expect << EOF
 * remote origin
   URL: $(pwd)/one
+  HEAD: master
   Remote branch merged with 'git pull' while on branch master
     master
   New remote branch (next fetch will store in remotes/origin)
@@ -338,16 +339,6 @@ test_expect_success 'update default (overridden, with funny whitespace)' '
 
 '
 
-test_expect_success '"remote show" does not show symbolic refs' '
-
-	git clone one three &&
-	(cd three &&
-	 git remote show origin > output &&
-	 ! grep HEAD < output &&
-	 ! grep -i stale < output)
-
-'
-
 test_expect_success 'reject adding remote with an invalid name' '
 
 	test_must_fail git remote add some:url desired-name