[PATCH] checkout: add a test for creating a new branch with regexp as a starting point

[PATCH] checkout: add a test for creating a new branch with regexp as a starting point

[Dmitry V. Levin]

Reported-by: Ivan Zakharyaschev <imz@altlinux.org>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---

This is just a regression test for the bug.

 t/t2018-checkout-new-branch-by-regexp.sh |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)
 create mode 100755 t/t2018-checkout-new-branch-by-regexp.sh

diff --git a/t/t2018-checkout-new-branch-by-regexp.sh b/t/t2018-checkout-new-branch-by-regexp.sh
new file mode 100755
index 0000000..78e54c5
--- /dev/null
+++ b/t/t2018-checkout-new-branch-by-regexp.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+test_description='checkout -b new_branch :/regexp'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo a > a &&
+	git add a &&
+	test_tick &&
+	git commit -m first &&
+	echo b > b &&
+	git add b &&
+	test_tick &&
+	git commit -m second
+'
+
+test_expect_success checkout '
+	git checkout -b new_branch :/first
+'
+
+test_done

-- 
ldv

Re: [PATCH] checkout: add a test for creating a new branch with regexp as a starting point

[Ævar Arnfjörð Bjarmason]

On Thu, Jul 29, 2010 at 22:01, Dmitry V. Levin <ldv@altlinux.org> wrote:
> Reported-by: Ivan Zakharyaschev <imz@altlinux.org>
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>
> This is just a regression test for the bug.

Thanks, always good to have more tests.

>  t/t2018-checkout-new-branch-by-regexp.sh |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
>  create mode 100755 t/t2018-checkout-new-branch-by-regexp.sh
>
> diff --git a/t/t2018-checkout-new-branch-by-regexp.sh b/t/t2018-checkout-new-branch-by-regexp.sh
> new file mode 100755
> index 0000000..78e54c5
> --- /dev/null
> +++ b/t/t2018-checkout-new-branch-by-regexp.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +test_description='checkout -b new_branch :/regexp'
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +       echo a > a &&
> +       git add a &&
> +       test_tick &&
> +       git commit -m first &&
> +       echo b > b &&
> +       git add b &&
> +       test_tick &&
> +       git commit -m second
> +'

This should use test_commit (see t/README), but...

> +test_expect_success checkout '
> +       git checkout -b new_branch :/first
> +'
> +
> +test_done

...it looks like this can just be added to the end of
t2018-checkout-branch.sh instead of creating a new test. Creating a
new file just for a single test for such a simple feature is a bit of
an overkill.

Re: [PATCH] checkout: add a test for creating a new branch with regexp as a starting point

[Thomas Rast]

Ævar Arnfjörð Bjarmason wrote:
> On Thu, Jul 29, 2010 at 22:01, Dmitry V. Levin <ldv@altlinux.org> wrote:
> > +test_expect_success checkout '
> > +       git checkout -b new_branch :/first
> > +'
> > +
> > +test_done
> 
> ...it looks like this can just be added to the end of
> t2018-checkout-branch.sh instead of creating a new test. Creating a
> new file just for a single test for such a simple feature is a bit of
> an overkill.

It should also use test_expect_failure unless you expect to have a fix
soon, otherwise it would stop the test suite from running through.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] checkout: add a test for creating a new branch with regexp as a starting point

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

On Thu, Jul 29, 2010 at 11:07:20PM +0000, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Jul 29, 2010 at 22:01, Dmitry V. Levin wrote:
[...]
> > +test_expect_success setup '
> > +       echo a > a &&
> > +       git add a &&
> > +       test_tick &&
> > +       git commit -m first &&
> > +       echo b > b &&
> > +       git add b &&
> > +       test_tick &&
> > +       git commit -m second
> > +'
> 
> This should use test_commit (see t/README), but...

The peculiarity of this bug makes it impossible, because test_commit()
also creates a tag which spoils test conditions.

> > +test_expect_success checkout '
> > +       git checkout -b new_branch :/first
> > +'
> > +
> > +test_done
> 
> ...it looks like this can just be added to the end of
> t2018-checkout-branch.sh instead of creating a new test. Creating a
> new file just for a single test for such a simple feature is a bit of
> an overkill.

Well, I see no t2018-checkout-branch.sh yet.  What file do you suggest
appending?

On Fri, Jul 30, 2010 at 01:36:13AM +0200, Thomas Rast wrote:
> It should also use test_expect_failure unless you expect to have a fix
> soon, otherwise it would stop the test suite from running through.

Of course I'd like to have this bug fixed, but OK, let it be
test_expect_failure() for now.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] checkout: add a test for creating a new branch with regexp as a starting point

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 30, 2010 at 08:44, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Thu, Jul 29, 2010 at 11:07:20PM +0000, Ævar Arnfjörð Bjarmason wrote:
>> On Thu, Jul 29, 2010 at 22:01, Dmitry V. Levin wrote:
> [...]
>> > +test_expect_success setup '
>> > +       echo a > a &&
>> > +       git add a &&
>> > +       test_tick &&
>> > +       git commit -m first &&
>> > +       echo b > b &&
>> > +       git add b &&
>> > +       test_tick &&
>> > +       git commit -m second
>> > +'
>>
>> This should use test_commit (see t/README), but...
>
> The peculiarity of this bug makes it impossible, because test_commit()
> also creates a tag which spoils test conditions.

I didn't know that. It'd be good if the commit message or a comment
indicated that. And actually, we should probably have a
test_commit_notag() then.

>> > +test_expect_success checkout '
>> > +       git checkout -b new_branch :/first
>> > +'
>> > +
>> > +test_done
>>
>> ...it looks like this can just be added to the end of
>> t2018-checkout-branch.sh instead of creating a new test. Creating a
>> new file just for a single test for such a simple feature is a bit of
>> an overkill.
>
> Well, I see no t2018-checkout-branch.sh yet.  What file do you suggest
> appending?

Ah, t2018-checkout-branch.sh only exists on the pu branch, not
master/next. It's probably worthwhile to patch it anyway rather than
adding a new one.

When to use test_commit (Re: [PATCH] checkout: add a test for creating a new branch with regexp as a starting point)

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> And actually, we should probably have a
> test_commit_notag() then.

I don’t know.  What’s so bad about using "git commit" directly?

I often find myself using "git commit" in tests because test_commit
imposes all the usual restrictions for a ref name on the commit
message.  I would happily use an abbreviation for

 test_tick &&
 git commit -m "something" &&
 git tag something-else

if available because I don’t like typing, but would that help the
reader and test runner any?

(That’s not a rhetorical question.  A patch that answers in the
positive would be fine by me. ;-))

Re: When to use test_commit (Re: [PATCH] checkout: add a test for creating a new branch with regexp as a starting point)

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 30, 2010 at 19:13, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> And actually, we should probably have a
>> test_commit_notag() then.
>
> I don’t know.  What’s so bad about using "git commit" directly?

Nothing, it's just easier to write and read the tests when we have and
use functions for these common operations.

> I often find myself using "git commit" in tests because test_commit
> imposes all the usual restrictions for a ref name on the commit
> message.  I would happily use an abbreviation for
>
>  test_tick &&
>  git commit -m "something" &&
>  git tag something-else
>
> if available because I don’t like typing, but would that help the
> reader and test runner any?

Maybe just introduce a fourth argument to test_commit, to make it
<message> [<file> [<contents> [<tagname>]] instead of <message>
[<file> [<contents>]] ?

[PATCH 1/2] test-lib.sh: introduce 4th argument to test_commit() specifying a tag name

[Dmitry V. Levin]

When fourth argument to test_commit() is specified, use it as a tag
name, unless it equals to empty string.  In the latter case, skip tag
creation.

Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
 t/test-lib.sh |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 367f053..a203383 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,10 +277,12 @@ test_tick () {
 	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
 }
 
-# Call test_commit with the arguments "<message> [<file> [<contents>]]"
+# Call test_commit with the arguments
+# "<message> [<file> [<contents> [<tagname>]]]"
 #
 # This will commit a file with the given contents and the given commit
-# message.  It will also add a tag with <message> as name.
+# message.  It will also add a tag with the given name unless the latter
+# is the empty string.
 #
 # Both <file> and <contents> default to <message>.
 
@@ -290,7 +292,7 @@ test_commit () {
 	git add "$file" &&
 	test_tick &&
 	git commit -m "$1" &&
-	git tag "$1"
+	if test -n "${4-$1}"; then git tag "${4-$1}"; fi
 }
 
 # Call test_merge with the arguments "<message> <commit>", where <commit>


-- 
ldv

Re: [PATCH 1/2] test-lib.sh: introduce 4th argument to test_commit() specifying a tag name

[Ævar Arnfjörð Bjarmason]

On Sat, Jul 31, 2010 at 00:18, Dmitry V. Levin <ldv@altlinux.org> wrote:
> When fourth argument to test_commit() is specified, use it as a tag
> name, unless it equals to empty string.  In the latter case, skip tag
> creation.

Looks good, mostly.

> Suggested-by: Ęvar Arnfjörš Bjarmason <avarab@gmail.com>

Seems like you sent a ISO-8859-1 E-Mail with UTF-8 content.

My name also doubles as a UUID and a UTF-8 canary, you see.

> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>  t/test-lib.sh |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 367f053..a203383 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -277,10 +277,12 @@ test_tick () {
>        export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
>  }
>
> -# Call test_commit with the arguments "<message> [<file> [<contents>]]"
> +# Call test_commit with the arguments
> +# "<message> [<file> [<contents> [<tagname>]]]"
>  #
>  # This will commit a file with the given contents and the given commit
> -# message.  It will also add a tag with <message> as name.
> +# message.  It will also add a tag with the given name unless the latter
> +# is the empty string.

You should also patch t/README, which documents this function.

>  # Both <file> and <contents> default to <message>.
>
> @@ -290,7 +292,7 @@ test_commit () {
>        git add "$file" &&
>        test_tick &&
>        git commit -m "$1" &&
> -       git tag "$1"
> +       if test -n "${4-$1}"; then git tag "${4-$1}"; fi

This is just a nit, but I'd write this on 4 lines:

if test..
then
    git tag
fi

[PATCH 1/2 v2] test-lib.sh: introduce 4th argument to test_commit() specifying a tag name

[Dmitry V. Levin]

When fourth argument to test_commit() is specified, use it as a tag
name, unless it equals to empty string.  In the latter case, skip tag
creation.

Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
 t/README      |    8 ++++----
 t/test-lib.sh |   11 ++++++++---
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/t/README b/t/README
index 0d1183c..18f5810 100644
--- a/t/README
+++ b/t/README
@@ -388,13 +388,13 @@ library for your script to use.
    committer times to defined stated.  Subsequent calls will
    advance the times by a fixed amount.
 
- - test_commit <message> [<filename> [<contents>]]
+ - test_commit <message> [<filename> [<contents> [<tagname>]]]
 
    Creates a commit with the given message, committing the given
    file with the given contents (default for both is to reuse the
-   message string), and adds a tag (again reusing the message
-   string as name).  Calls test_tick to make the SHA-1s
-   reproducible.
+   message string), and adds a tag with the given name (default again
+   is to reuse the message string as name) unless this name is the
+   empty string.  Calls test_tick to make the SHA-1s reproducible.
 
  - test_merge <message> <commit-or-tag>
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e5523dd..ebb6215 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -277,10 +277,12 @@ test_tick () {
 	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
 }
 
-# Call test_commit with the arguments "<message> [<file> [<contents>]]"
+# Call test_commit with the arguments
+# "<message> [<file> [<contents> [<tagname>]]]"
 #
 # This will commit a file with the given contents and the given commit
-# message.  It will also add a tag with <message> as name.
+# message.  It will also add a tag with the given name unless the latter
+# is the empty string.
 #
 # Both <file> and <contents> default to <message>.
 
@@ -290,7 +292,10 @@ test_commit () {
 	git add "$file" &&
 	test_tick &&
 	git commit -m "$1" &&
-	git tag "$1"
+	if test -n "${4-$1}"
+	then
+		git tag "${4-$1}"
+	fi
 }
 
 # Call test_merge with the arguments "<message> <commit>", where <commit>


-- 
ldv

Re: [PATCH 1/2 v2] test-lib.sh: introduce 4th argument to test_commit() specifying a tag name

[Ævar Arnfjörð Bjarmason]

On Sat, Jul 31, 2010 at 01:56, Dmitry V. Levin <ldv@altlinux.org> wrote:
> When fourth argument to test_commit() is specified, use it as a tag
> name, unless it equals to empty string.  In the latter case, skip tag
> creation.
>
> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>  t/README      |    8 ++++----
>  t/test-lib.sh |   11 ++++++++---
>  2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/t/README b/t/README
> index 0d1183c..18f5810 100644
> --- a/t/README
> +++ b/t/README
> @@ -388,13 +388,13 @@ library for your script to use.
>    committer times to defined stated.  Subsequent calls will
>    advance the times by a fixed amount.
>
> - - test_commit <message> [<filename> [<contents>]]
> + - test_commit <message> [<filename> [<contents> [<tagname>]]]
>
>    Creates a commit with the given message, committing the given
>    file with the given contents (default for both is to reuse the
> -   message string), and adds a tag (again reusing the message
> -   string as name).  Calls test_tick to make the SHA-1s
> -   reproducible.
> +   message string), and adds a tag with the given name (default again
> +   is to reuse the message string as name) unless this name is the
> +   empty string.  Calls test_tick to make the SHA-1s reproducible.
>
>  - test_merge <message> <commit-or-tag>
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index e5523dd..ebb6215 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -277,10 +277,12 @@ test_tick () {
>        export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
>  }
>
> -# Call test_commit with the arguments "<message> [<file> [<contents>]]"
> +# Call test_commit with the arguments
> +# "<message> [<file> [<contents> [<tagname>]]]"
>  #
>  # This will commit a file with the given contents and the given commit
> -# message.  It will also add a tag with <message> as name.
> +# message.  It will also add a tag with the given name unless the latter
> +# is the empty string.
>  #
>  # Both <file> and <contents> default to <message>.
>
> @@ -290,7 +292,10 @@ test_commit () {
>        git add "$file" &&
>        test_tick &&
>        git commit -m "$1" &&
> -       git tag "$1"
> +       if test -n "${4-$1}"
> +       then
> +               git tag "${4-$1}"
> +       fi
>  }
>
>  # Call test_merge with the arguments "<message> <commit>", where <commit>

This one looks good, thanks.

Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

[PATCH 2/2] checkout: add a test for creating a new branch with regexp as a starting point

[Dmitry V. Levin]

Reported-by: Ivan Zakharyaschev <imz@altlinux.org>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
 t/t2018-checkout-branch.sh |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 1caffea..4d26b2a 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -118,7 +118,20 @@ test_expect_success 'checkout -b to an existing branch fails' '
 	test_must_fail do_checkout branch2 $HEAD2
 '
 
+test_expect_failure 'checkout -b new_branch :/regexp' '
+	# clean up from previous test
+	git reset --hard &&
+
+	do_checkout old_regexp_branch branch1 &&
+	test_commit first '' '' '' &&
+	test_commit second '' '' '' &&
+
+	do_checkout new_regexp_branch :/first
+'
+
 test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
+	# clean up from previous test
+	git reset --hard &&
 	git checkout branch1 &&
 
 	do_checkout branch2 "" -B


-- 
ldv

Re: [PATCH 2/2] checkout: add a test for creating a new branch with regexp as a starting point

[Ævar Arnfjörð Bjarmason]

On Sat, Jul 31, 2010 at 00:19, Dmitry V. Levin <ldv@altlinux.org> wrote:
> Reported-by: Ivan Zakharyaschev <imz@altlinux.org>
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>  t/t2018-checkout-branch.sh |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> index 1caffea..4d26b2a 100755
> --- a/t/t2018-checkout-branch.sh
> +++ b/t/t2018-checkout-branch.sh
> @@ -118,7 +118,20 @@ test_expect_success 'checkout -b to an existing branch fails' '
>        test_must_fail do_checkout branch2 $HEAD2
>  '
>
> +test_expect_failure 'checkout -b new_branch :/regexp' '
> +       # clean up from previous test
> +       git reset --hard &&
> +
> +       do_checkout old_regexp_branch branch1 &&
> +       test_commit first '' '' '' &&
> +       test_commit second '' '' '' &&

Should note in the commit message or a comment that this test needs to
not create a tag, in case someone wonders down the line what the '' ''
'' ASCII art is all about :)

Looks good otherwise, thanks for patching test_commit.

[PATCH 2/2 v2] checkout: add a test for creating a new branch with regexp as a starting point

[Dmitry V. Levin]

Reported-by: Ivan Zakharyaschev <imz@altlinux.org>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
 t/t2018-checkout-branch.sh |   25 +++++++++++++++++++++----
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 1caffea..6ada870 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -4,12 +4,13 @@ test_description='checkout '
 
 . ./test-lib.sh
 
-# Arguments: <branch> <sha> [<checkout options>]
+# Arguments: <branch> <start_point> [<checkout options>]
 #
 # Runs "git checkout" to switch to <branch>, testing that
 #
 #   1) we are on the specified branch, <branch>;
-#   2) HEAD is <sha>; if <sha> is not specified, the old HEAD is used.
+#   2) HEAD points to the same commit as <start_point> does;
+#      if <start_point> is not specified, the old HEAD is used.
 #
 # If <checkout options> is not specified, "git checkout" is run with -b.
 do_checkout() {
@@ -17,7 +18,8 @@ do_checkout() {
 	exp_ref="refs/heads/$exp_branch" &&
 
 	# if <sha> is not specified, use HEAD.
-	exp_sha=${2:-$(git rev-parse --verify HEAD)} &&
+	start_point=${2:-HEAD}
+	exp_sha=$(git rev-parse --verify $start_point) &&
 
 	# default options for git checkout: -b
 	if [ -z "$3" ]; then
@@ -26,7 +28,7 @@ do_checkout() {
 		opts="$3"
 	fi
 
-	git checkout $opts $exp_branch $exp_sha &&
+	git checkout $opts $exp_branch $start_point &&
 
 	test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) &&
 	test $exp_sha = $(git rev-parse --verify HEAD)
@@ -118,7 +120,22 @@ test_expect_success 'checkout -b to an existing branch fails' '
 	test_must_fail do_checkout branch2 $HEAD2
 '
 
+test_expect_failure 'checkout -b new_branch :/regexp' '
+	# clean up from previous test
+	git reset --hard &&
+
+	do_checkout old_regexp_branch branch1 &&
+	# The first commit in this test should not be referenced by
+	# other branches or tags.
+	test_commit first a a "" &&
+	test_commit second &&
+
+	do_checkout new_regexp_branch :/first
+'
+
 test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
+	# clean up from previous test
+	git reset --hard &&
 	git checkout branch1 &&
 
 	do_checkout branch2 "" -B
-- 
1.7.2.1

Re: [PATCH 2/2 v2] checkout: add a test for creating a new branch with regexp as a starting point

[Ævar Arnfjörð Bjarmason]

On Sat, Jul 31, 2010 at 01:44, Dmitry V. Levin <ldv@altlinux.org> wrote:
> Reported-by: Ivan Zakharyaschev <imz@altlinux.org>
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>  t/t2018-checkout-branch.sh |   25 +++++++++++++++++++++----
>  1 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> index 1caffea..6ada870 100755
> --- a/t/t2018-checkout-branch.sh
> +++ b/t/t2018-checkout-branch.sh
> @@ -4,12 +4,13 @@ test_description='checkout '
>
>  . ./test-lib.sh
>
> -# Arguments: <branch> <sha> [<checkout options>]
> +# Arguments: <branch> <start_point> [<checkout options>]
>  #
>  # Runs "git checkout" to switch to <branch>, testing that
>  #
>  #   1) we are on the specified branch, <branch>;
> -#   2) HEAD is <sha>; if <sha> is not specified, the old HEAD is used.
> +#   2) HEAD points to the same commit as <start_point> does;
> +#      if <start_point> is not specified, the old HEAD is used.
>  #
>  # If <checkout options> is not specified, "git checkout" is run with -b.
>  do_checkout() {
> @@ -17,7 +18,8 @@ do_checkout() {
>        exp_ref="refs/heads/$exp_branch" &&
>
>        # if <sha> is not specified, use HEAD.
> -       exp_sha=${2:-$(git rev-parse --verify HEAD)} &&
> +       start_point=${2:-HEAD}
> +       exp_sha=$(git rev-parse --verify $start_point) &&
>
>        # default options for git checkout: -b
>        if [ -z "$3" ]; then
> @@ -26,7 +28,7 @@ do_checkout() {
>                opts="$3"
>        fi
>
> -       git checkout $opts $exp_branch $exp_sha &&
> +       git checkout $opts $exp_branch $start_point &&
>
>        test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) &&
>        test $exp_sha = $(git rev-parse --verify HEAD)
> @@ -118,7 +120,22 @@ test_expect_success 'checkout -b to an existing branch fails' '
>        test_must_fail do_checkout branch2 $HEAD2
>  '
>
> +test_expect_failure 'checkout -b new_branch :/regexp' '
> +       # clean up from previous test
> +       git reset --hard &&
> +
> +       do_checkout old_regexp_branch branch1 &&
> +       # The first commit in this test should not be referenced by
> +       # other branches or tags.
> +       test_commit first a a "" &&
> +       test_commit second &&
> +
> +       do_checkout new_regexp_branch :/first
> +'
> +
>  test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
> +       # clean up from previous test
> +       git reset --hard &&
>        git checkout branch1 &&
>
>        do_checkout branch2 "" -B
> --
> 1.7.2.1

Looks good, thanks.

Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Re: [PATCH 2/2 v3] checkout: add a test for creating a new branch with regexp as a starting point

[Dmitry V. Levin]

Reported-by: Ivan Zakharyaschev <imz@altlinux.org>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

The only difference between v2 and v3 editions of this patch is that
test_expect_failure has been replaced with test_expect_success, because
the fix (jc/sha1-name-find-fix) is already available.

 t/t2018-checkout-branch.sh |   25 +++++++++++++++++++++----
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 1caffea..13f7194 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -4,12 +4,13 @@ test_description='checkout '
 
 . ./test-lib.sh
 
-# Arguments: <branch> <sha> [<checkout options>]
+# Arguments: <branch> <start_point> [<checkout options>]
 #
 # Runs "git checkout" to switch to <branch>, testing that
 #
 #   1) we are on the specified branch, <branch>;
-#   2) HEAD is <sha>; if <sha> is not specified, the old HEAD is used.
+#   2) HEAD points to the same commit as <start_point> does;
+#      if <start_point> is not specified, the old HEAD is used.
 #
 # If <checkout options> is not specified, "git checkout" is run with -b.
 do_checkout() {
@@ -17,7 +18,8 @@ do_checkout() {
 	exp_ref="refs/heads/$exp_branch" &&
 
 	# if <sha> is not specified, use HEAD.
-	exp_sha=${2:-$(git rev-parse --verify HEAD)} &&
+	start_point=${2:-HEAD}
+	exp_sha=$(git rev-parse --verify $start_point) &&
 
 	# default options for git checkout: -b
 	if [ -z "$3" ]; then
@@ -26,7 +28,7 @@ do_checkout() {
 		opts="$3"
 	fi
 
-	git checkout $opts $exp_branch $exp_sha &&
+	git checkout $opts $exp_branch $start_point &&
 
 	test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) &&
 	test $exp_sha = $(git rev-parse --verify HEAD)
@@ -118,7 +120,22 @@ test_expect_success 'checkout -b to an existing branch fails' '
 	test_must_fail do_checkout branch2 $HEAD2
 '
 
+test_expect_success 'checkout -b new_branch :/regexp' '
+	# clean up from previous test
+	git reset --hard &&
+
+	do_checkout old_regexp_branch branch1 &&
+	# The first commit in this test should not be referenced by
+	# other branches or tags.
+	test_commit first a a "" &&
+	test_commit second &&
+
+	do_checkout new_regexp_branch :/first
+'
+
 test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
+	# clean up from previous test
+	git reset --hard &&
 	git checkout branch1 &&
 
 	do_checkout branch2 "" -B

-- 
ldv

Re: [PATCH] checkout: add a test for creating a new branch with regexp as a starting point

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

>> > +test_expect_success checkout '
>> > +       git checkout -b new_branch :/first
>> > +'
>> > +
>> > +test_done
>> 
>> ...it looks like this can just be added to the end of
>> t2018-checkout-branch.sh instead of creating a new test. Creating a
>> new file just for a single test for such a simple feature is a bit of
>> an overkill.
>
> It should also use test_expect_failure unless you expect to have a fix
> soon, otherwise it would stop the test suite from running through.

I think this is an ancient bug from the very beginning of the ":/"
notation.  Because it temporarily uses one bit in the object flags, but
the get_sha1_oneline() parser can be called multiple times, it tries to
clear the flag bits it smudged at the end, but fails.

The basic pattern for traversing the ancestry while avoiding duplication
is:

    prepare commits you traverse from in "list";
    while (list) {
        commit = pop_most_recent_commit(&list, MARK);
        try to use commit;
    }

pop_most_recent_commit() removes a commit from the list, pushes parents of
the commit that are _not_ marked with any of the bits in the MARK to the
given list, and returns that commit.

While introducing the feature ':/', however, 28a4d94 (object name:
introduce ':/<oneline prefix>' notation, 2007-02-24) wanted to avoid
leaving the mark used in the loop before leaving the function (which is a
right thing to do), by doing this:

    prepare commits you traverse from in "list";
+   keep a copy of "list" in "backup";
    while (list) {
        commit = pop_most_recent_commit(&list, MARK);
        try to use commit;
    }
+   for (commit in backup)
+       clear_commit_marks(commit, MARK);

which is wrong, as clear_commit_marks() will ignore if the starting commit
does not have the MARK.

I think the fix should be just as simple as this.  Essentially, X_SEEN
mark means "we have marked and placed this item on the list (so do not put
it again back on the list when it appears as a parent of some other item)"
so marking commits we initially put on the list with the bit should be the
right thing to do regardless of this clean-up business.

 sha1_name.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 4f2af8d..b935688 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -704,8 +704,10 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 		die("Invalid search pattern: %s", prefix);
 
 	for_each_ref(handle_one_ref, &list);
-	for (l = list; l; l = l->next)
+	for (l = list; l; l = l->next) {
 		commit_list_insert(l->item, &backup);
+		l->item->object.flags |= ONELINE_SEEN;
+	}
 	while (list) {
 		char *p;
 		struct commit *commit;