patch, 2 partst6300: test sort with multiple keys

> The linked list describing sort options was not correctly set up in
> opt_parse_sort. In the result, contrary to the documentation. only the
> last of multiple --sort options to git-for-each-ref was taken into
> account. This commit fixes it.
>
> Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
> ---
>  builtin/for-each-ref.c  | 4 +++-
>  t/t6300-for-each-ref.sh | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index b01d76a..0c5294e 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -962,7 +962,9 @@ static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
>  	if (!arg) /* should --no-sort void the list ? */
>  		return -1;
>  
> -	*sort_tail = s = xcalloc(1, sizeof(*s));
> +	s = xcalloc(1, sizeof(*s));
> +	s->next = *sort_tail;
> +	*sort_tail = s;

> Documentation of git-for-each-ref says that --sort=<key> option can be
> used multiple times, in which case the last key becomes the primary key.
> However this functionality was never checked in test suite and is
> currently broken. This commit adds appropriate test in preparation for fix.
>
> Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
> ---

> +test_expect_success 'Create branches to test sort with multiple keys' '
> +	git checkout -b Branch1 &&
> +	echo foo >> one &&
> +	git commit -a -m "Branch1 commit" &&
> +	git checkout -b Branch2 &&
> +	echo foo >> one &&
> +	git commit -a -m "Branch2 commit"
> +'
> +
> +test_atom refs/heads/Branch1 objectname 32fca05e9f638021a123a84226acf17756acc18b
> +test_atom refs/heads/Branch2 objectname 194a5b89ac661a114566ba4374bc06c2797539f3

> +
>  cat >expected <<\EOF
> +67a36f10722846e891fbada1ba48ed035de75581 commit	refs/heads/master
> +194a5b89ac661a114566ba4374bc06c2797539f3 commit	refs/heads/Branch2
> +32fca05e9f638021a123a84226acf17756acc18b commit	refs/heads/Branch1
> +EOF
> +
> +test_expect_failure 'Verify sort with multiple keys' '
> +	git for-each-ref --sort=objectname --sort=committerdate refs/heads > actual &&
> +	test_cmp expected actual
> +'
> +
> +cat >expected <<\EOF
> +'refs/heads/Branch1'
> +'refs/heads/Branch2'
>  'refs/heads/master'
>  'refs/remotes/origin/master'
>  'refs/tags/testtag'
> @@ -264,6 +289,8 @@ test_expect_success 'Quoting style: python' '
>  '
>  
>  cat >expected <<\EOF
> +"refs/heads/Branch1"
> +"refs/heads/Branch2"
>  "refs/heads/master"
>  "refs/remotes/origin/master"
>  "refs/tags/testtag"
> @@ -285,6 +312,8 @@ for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do
>  done
>  
>  cat >expected <<\EOF
> +Branch1
> +Branch2
>  master
>  testtag
>  EOF
> @@ -296,6 +325,8 @@ test_expect_success 'Check short refname format' '
>  '
>  
>  cat >expected <<EOF
> +
> +
>  origin/master

>  EOF
>  
> @@ -309,7 +340,7 @@ cat >expected <<EOF
>  EOF
>  
>  test_expect_success 'Check short objectname format' '
> -	git for-each-ref --format="%(objectname:short)" refs/heads >actual &&
> +	git for-each-ref --format="%(objectname:short)" refs/heads/master >actual &&
>  	test_cmp expected actual
>  '

> > +test_expect_success 'Create branches to test sort with multiple keys' '
> > +	git checkout -b Branch1 &&
> > +	echo foo >> one &&
> > +	git commit -a -m "Branch1 commit" &&
> > +	git checkout -b Branch2 &&
> > +	echo foo >> one &&
> > +	git commit -a -m "Branch2 commit"
> > +'
> > +
> > +test_atom refs/heads/Branch1 objectname 32fca05e9f638021a123a84226acf17756acc18b
> > +test_atom refs/heads/Branch2 objectname 194a5b89ac661a114566ba4374bc06c2797539f3

> Do these need to be "Branch[12]", not "branch[12]" for the code to
> exhibit the bug?  If not, please don't be creative in names like
> these.  On case corrupting filesystems you may write Branch1 and
> they may come back as branch1, but that is not what we are testing
> here.

> >  test_expect_success 'Check short objectname format' '
> > -	git for-each-ref --format="%(objectname:short)" refs/heads >actual &&
> > +	git for-each-ref --format="%(objectname:short)" refs/heads/master >actual &&
> >  	test_cmp expected actual
> >  '

> All in all, I have to wonder if you can limit the updates to other
> unrelated tests if you added a new test near the end.  Also doesn't
> the existing test already create enough refs to let you sort with
> multiple keys and demonstrate the breakage already, without adding new
> refs and objects?

> Documentation of git-for-each-ref says that --sort=<key> option can be
> used multiple times, in which case the last key becomes the primary key.
> However this functionality was never checked in test suite and is
> currently broken. This commit adds appropriate test in preparation for fix.
>
> Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
> ---
>  t/t6300-for-each-ref.sh | 10 ++++++++++
>  1 file changed, 10 insertions(+)

> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 1721784..a0d82d4 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -456,4 +456,14 @@ test_atom refs/tags/signed-long contents "subject line
>  body contents
>  $sig"
>  
> +cat >expected <<\EOF
> +408fe76d02a785a006c2e9c669b7be5589ede96d <committer@example.com> refs/tags/master
> +90b5ebede4899eda64893bc2a4c8f1d6fb6dfc40 <committer@example.com> refs/tags/bogo
> +EOF
> +
> +test_expect_failure 'Verify sort with multiple keys' '
> +	git for-each-ref --format="%(objectname) %(taggeremail) %(refname)" --sort=objectname --sort=taggeremail \
> +		refs/tags/bogo refs/tags/master > actual &&
> +	test_cmp expected actual
> +'
>  test_done
> -- 
> 1.7.12.rc3