Re: [PATCH] sha1-name.c: for ":/", find detached HEAD commits
- Date: Tue, 10 Jul 2018 13:54:47 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH] sha1-name.c: for ":/", find detached HEAD commits
On Tue, Jul 10, 2018 at 08:41:06AM -0700, William Chargin wrote:
> This patch broadens the set of commits matched by ":/" pathspecs to
> include commits reachable from HEAD but not any named ref. This avoids
> surprising behavior when working with a detached HEAD and trying to
> refer to a commit that was recently created and only exists within the
> detached state.
> Signed-off-by: William Chargin <wchargin@xxxxxxxxx>
> Based-on-patch-by: Jeff King <peff@xxxxxxxx>
Thanks, this looks perfect!
Just to be clear on licensing, I'll add my:
Signed-off-by: Jeff King <peff@xxxxxxxx>
> diff --git a/t/t4208-log-magic-pathspec.sh b/t/t4208-log-magic-pathspec.sh
> index 62f335b2d..41b9f3eba 100755
> --- a/t/t4208-log-magic-pathspec.sh
> +++ b/t/t4208-log-magic-pathspec.sh
> @@ -25,6 +25,20 @@ test_expect_success '"git log :/a -- " should not be ambiguous' '
> git log :/a --
> +test_expect_success '"git log :/detached -- " should find a commit only in HEAD' '
> + test_when_finished "git checkout master" &&
> + git checkout --detach &&
> + test_tick &&
> + git commit --allow-empty -m detached &&
> + test_tick &&
> + git commit --allow-empty -m something-else &&
> + git log :/detached --
At first I wondered if you could just use test_commit here (instead of a
manual test_tick and commit). But we care about reachability only from
the detached HEAD here, so this must _not_ use test_commit, which
creates an extra tag.
It could be worth mentioning that in a comment (since somebody who later
refactors to make that change would not realize the problem, as it would
simply cause a broken test to return a false success). But...
> +test_expect_success '"git log :/detached -- " should not find an orphaned commit' '
> + test_must_fail git log :/detached --
This follow-up test _would_ notice such a breakage. Very nicely done.