Re: issue upgrading git from 1.8.2.1 to 2.8.0

2016-04-22 Thread Jeff King
On Sat, Apr 23, 2016 at 01:18:50AM -0400, Gennaro Torre wrote:

> This is what I see:
> 
> 01:16:15.789969 pkt-line.c:80   packet:fetch>
> git-upload-pack /tumblr.git\0host=\0
> 01:16:15.851231 pkt-line.c:80   packet:fetch<
> a210029455476c3ae0d34748aeaeccf71864fd81 HEAD\0multi_ack thin-pack
> side-band side-band-64k ofs-delta shallow no-progress include-tag
> multi_ack_detailed symref=HEAD:refs/heads/master agent=git/2.8.0
> 01:16:15.851433 pkt-line.c:80   packet:fetch<
> 511b1feab8d10b81777a75c02dabc94c7da9e0d4 refs/heads/263-fix
> .
> 01:17:59.497259 pkt-line.c:80   packet:fetch<
> cd63ca15d47b469e692faea2e04f32a241a6637a refs/tags/hotposts_v0.1
> 01:17:59.497265 pkt-line.c:80   packet:fetch<
> 4749ee8a11ca093928c8a43fb0147d22177f1d71 refs/tags/hotposts_v0.1^{}
> 01:17:59.497282 pkt-line.c:80   packet:fetch< 
> Server supports multi_ack_detailed
> Server supports side-band-64k
> Server supports ofs-delta
> Server version is git/2.8.0

So that's weird. The client gets the initial ref advertisement and
then...does nothing? It should drop into find_common() and start sending
want/have lines to the server (or decide it doesn't need any objects and
immediately send a flush packet).

Is the client chewing any CPU, or otherwise doing anything according to
"strace"? Is it possible to connect to it with gdb and get a backtrace?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: issue upgrading git from 1.8.2.1 to 2.8.0

2016-04-22 Thread Jeff King
On Sat, Apr 23, 2016 at 01:01:06AM -0400, Gennaro Torre wrote:

> @Jeff King nope nothing interesting, it just hangs:
> 
> [gtorre@host1 A:DEVEL tumblr.old]$ git fetch -vv
> Looking up  ... done.
> Connecting to  (port 9418) ...  done.
> Server supports multi_ack_detailed
> Server supports side-band-64k
> Server supports ofs-delta
> Server version is git/2.8.0

Hmm.  If you run with GIT_TRACE_PACKET=1, that may give an indication of
why it is hanging. Or perhaps tracing via gdb to see where we are when
it hangs.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: possible bug of git stash deleting uncommitted files in corner case

2016-04-22 Thread Jeff King
On Fri, Apr 22, 2016 at 09:04:25PM +0200, Remi Galan Alfonso wrote:

> For this bug it doesn't seem to be specifically linked to git stash,
> since 'git status' doesn't display correct informations in the first
> place (it doesn't show foo/bar as an untracked file).
> 
> I tried something quickly, based on Daniele's case:
> git init
> echo 'X' >foo
> git add foo
> git commit -m "Added foo"
> rm foo
> mkdir foo
> echo 'B' >foo/bar
> 
> git status # foo/bar not shown in Untracked files
> 
> git add foo/bar
> 
> git status then shows as expected:
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD ..." to unstage)
> # 
> # deleted:foo
> # new file:   foo/bar

Before you "git add foo/bar", try "git status -uall", which asks git to
descend into directories when looking for untracked files. It _does_
show foo/bar as untracked.

So I think what is happening is that in the case without "-uall", we see
"foo" as an untracked file, but then check that with the index to say
"ah, it is not untracked, there is an entry in the index".  But of
course the earlier mention in "not staged for commit" will not say
anything about the new directory "foo", because we only diff actual
files there.

Likewise, if you ask "ls-files -o", it mentions "foo/bar".

So I think the bug is in the way dir.c handles
DIR_SHOW_OTHER_DIRECTORIES, or possibly in the way that
wt_status_collect_untracked handles its results.

It may be that the latter needs to special-case its
cache_name_is_other() check, and make sure we mention an "other" file we
found that is a directory, even if it has an index entry.

> However git stash fails this time:
> # error: foo: is a directory - add individual files instead
> # fatal: Unable to process path foo
> # Cannot save the current worktree state

I suspect the actual stash bug is separate from the bug above. It just
looks like update-index is not smart enough to realize that a file being
replaced with a directory is effectively an index deletion of that
entry. That's just a guess, though.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1] travis-ci: build documentation

2016-04-22 Thread Jeff King
On Fri, Apr 22, 2016 at 10:34:02AM +0200, larsxschnei...@gmail.com wrote:

> From: Lars Schneider 
> 
> Run "make doc" to check if all documentation can be build without errors.
> Since the documentation is the same on every platform/compiler, the check
> is only performed as part of the Linux/GCC build job to maintain a fast
> CI process.

This does slow down the normal test results for linux/gcc, though. I
don't know very much about Travis, but is it possible to break out the
documentation build into its own test, with a separate build status from
the other runs?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] fast-import: implement --min-pack-size parameter

2016-04-22 Thread Jeff King
On Sat, Apr 23, 2016 at 02:42:25AM +, Eric Wong wrote:

> With many incremental imports, small packs become highly
> inefficient due to the need to readdir scan and load many
> indices to locate even a single object.  Frequent repacking and
> consolidation may be prohibitively expensive in terms of disk
> I/O, especially in large repositories where the initial packs
> were aggressively optimized and marked with .keep files.
> 
> In those cases, users may be better served with loose objects
> and relying on "git gc --auto".
> 
> Signed-off-by: Eric Wong 
> ---
>   There should be a matching config file directive, but I'm
>   not sure how/if it should affect other commands.  So I'm
>   not sure if it should be "pack.packSizeMin" or
>   "fastimport.packSizeMin" or something else.

This same concept exists for pushing/fetching, but there we measure it
not in bytes but by the number of objects. Which is probably a better
measure. A single 10MB blob is better as a loose object than as a pack,
but a thousand 10KB blobs should be a pack.

There we have fetch.unpackLimit and receive.unpackLimit for the two
operations, plus transfer.unpackLimit to control both of them. This
doesn't necessarily need to be tied to that config, but you could
certainly consider it in the same boat. It's a way of transferring a
load of objects into the repository.

So it would make some sense to me to have fastimport.unpackLimit,
falling back to transfer.unpackLimit.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] fast-import: implement --min-pack-size parameter

2016-04-22 Thread Eric Wong
With many incremental imports, small packs become highly
inefficient due to the need to readdir scan and load many
indices to locate even a single object.  Frequent repacking and
consolidation may be prohibitively expensive in terms of disk
I/O, especially in large repositories where the initial packs
were aggressively optimized and marked with .keep files.

In those cases, users may be better served with loose objects
and relying on "git gc --auto".

Signed-off-by: Eric Wong 
---
  There should be a matching config file directive, but I'm
  not sure how/if it should affect other commands.  So I'm
  not sure if it should be "pack.packSizeMin" or
  "fastimport.packSizeMin" or something else.

  To further reduce disk I/O, the fsync_or_die call in
  fixup_pack_header_footer could probably be moved out of that
  function and become the fphf caller's responsibility.

 Documentation/git-fast-import.txt   |  9 
 fast-import.c   | 30 ++
 t/t9302-fast-import-min-packsize.sh | 42 +
 3 files changed, 81 insertions(+)
 create mode 100755 t/t9302-fast-import-min-packsize.sh

diff --git a/Documentation/git-fast-import.txt 
b/Documentation/git-fast-import.txt
index 66910aa..8c0ac94 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -136,6 +136,15 @@ Performance and Compression Tuning
Maximum size of each output packfile.
The default is unlimited.
 
+--min-pack-size=::
+   Mininum size of an output packfile, packfiles smaller
+   than this threshold are unpacked into loose objects and
+   the pack is discarded.  This is useful when performing
+   small, incremental imports as loose objects and relying
+   on `git gc --auto` may be more efficient than generating
+   many tiny packs.
+   The default is to always preserve the pack and never
+   generate loose objects.
 
 Performance
 ---
diff --git a/fast-import.c b/fast-import.c
index 9fc7093..a00bee5 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -166,6 +166,7 @@ Format of STDIN stream:
 #include "quote.h"
 #include "exec_cmd.h"
 #include "dir.h"
+#include "run-command.h"
 
 #define PACK_ID_BITS 16
 #define MAX_PACK_ID ((1<pack_fd, 0, SEEK_SET) < 0)
+   die_errno("Failed seeking to start of '%s'", p->pack_name);
+
+   unpack.in = p->pack_fd;
+   unpack.git_cmd = 1;
+   unpack.stdout_to_stderr = 1;
+   argv_array_push(, "unpack-objects");
+   argv_array_push(, "-q");
+
+   return run_command();
+}
+
 static void end_packfile(void)
 {
static int running;
@@ -972,6 +990,12 @@ static void end_packfile(void)
fixup_pack_header_footer(pack_data->pack_fd, pack_data->sha1,
pack_data->pack_name, object_count,
cur_pack_sha1, pack_size);
+
+   if (pack_size < min_packsize) {
+   if (loosen_small_pack(pack_data) == 0)
+   goto discard_pack;
+   }
+
close(pack_data->pack_fd);
idx_name = keep_pack(create_index());
 
@@ -1002,6 +1026,7 @@ static void end_packfile(void)
pack_id++;
}
else {
+discard_pack:
close(pack_data->pack_fd);
unlink_or_warn(pack_data->pack_name);
}
@@ -3237,6 +3262,11 @@ static int parse_one_option(const char *option)
v = 1024 * 1024;
}
max_packsize = v;
+   } else if (skip_prefix(option, "min-pack-size=", )) {
+   unsigned long v;
+   if (!git_parse_ulong(option, ))
+   return 0;
+   min_packsize = v;
} else if (skip_prefix(option, "big-file-threshold=", )) {
unsigned long v;
if (!git_parse_ulong(option, ))
diff --git a/t/t9302-fast-import-min-packsize.sh 
b/t/t9302-fast-import-min-packsize.sh
new file mode 100755
index 000..7dcdccc
--- /dev/null
+++ b/t/t9302-fast-import-min-packsize.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+test_description='test git fast-import min-packsize'
+. ./test-lib.sh
+
+test_expect_success 'create loose objects on import' '
+   test_tick &&
+   cat >input <<-INPUT_END &&
+   commit refs/heads/master
+   committer $GIT_COMMITTER_NAME 

Re: [PATCH] hooks: Add ability to specify where the hook directory is

2016-04-22 Thread Ævar Arnfjörð Bjarmason
On Sat, Apr 23, 2016 at 2:13 AM, SZEDER Gábor  wrote:
>
>> Change the hardcoded lookup for .git/hooks/* to optionally lookup in
>> $(git config core.hooksDirectory)/* instead if that config key is set.
>>
>> This is essentially a more intrusive version of the git-init ability to
>> specify hooks on init time via init templates.
>>
>> The difference between that facility and this feature is that this can
>> be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
>> apply for all your personal repositories, or all repositories on the
>> system.
>>
>> I plan on using this on a centralized Git server where users can create
>> arbitrary repositories under /gitroot, but I'd like to manage all the
>> hooks that should be run centrally via a unified dispatch mechanism.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason 
>> ---
>>  Documentation/config.txt  | 10 ++
>>  Documentation/githooks.txt|  5 -
>>  cache.h   |  1 +
>>  config.c  |  3 +++
>>  environment.c |  1 +
>>  run-command.c |  5 -
>>  t/t1350-config-hooks-directory.sh | 35 +++
>>  7 files changed, 58 insertions(+), 2 deletions(-)
>>  create mode 100755 t/t1350-config-hooks-directory.sh
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 42d2b50..2faf3c0 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -618,6 +618,16 @@ core.attributesFile::
>>   $XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
>>   set or empty, $HOME/.config/git/attributes is used instead.
>>
>> +core.hooksDirectory::
>> + By default Git will look for your hooks in the '$GIT_DIR/hooks'
>> + directory. Set this to different absolute directory name,

Thanks for the review. I'll submit a new patch with fixes pending any
more comments from others to reduce list churn.

> Mental note: here you say that it should be an absolute directory.
>
>> + e.g. '/etc/git/hooks', and Git will try to find your hooks that
>
> s/hooks that/hooks in that/

Will fix.

>> + directory, e.g. '/etc/git/hooks/pre-receive' instead of in
>> + '$GIT_DIR/hooks'.
>> ++
>> +This is useful in cases where you'd like to centrally configure your
>> +Git hooks instead of configuring them on a per-repository basis.
>> +
>>  core.editor::
>>   Commands such as `commit` and `tag` that lets you edit
>>   messages by launching an editor uses the value of this
>
>
>> diff --git a/t/t1350-config-hooks-directory.sh 
>> b/t/t1350-config-hooks-directory.sh
>> new file mode 100755
>> index 000..556c1d3
>> --- /dev/null
>> +++ b/t/t1350-config-hooks-directory.sh
>> @@ -0,0 +1,35 @@
>> +#!/bin/sh
>> +
>> +test_description='Test the core.hooksDirectory configuration variable'
>> +
>> +. ./test-lib.sh
>> +
>> +test_expect_success 'set up a pre-commit hook in core.hooksDirectory' '
>> + mkdir -p .git/custom-hooks .git/hooks &&
>> + cat >.git/custom-hooks/pre-commit <> +#!$SHELL_PATH
>> +printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
>> +EOF
>> + cat >.git/hooks/pre-commit <> + chmod +x .git/hooks/pre-commit
>> +#!$SHELL_PATH
>> +printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
>> +EOF
>> + chmod +x .git/custom-hooks/pre-commit
>> +'
>
> Please use the 'write_script' helper for, well, writing scripts.

Will fix.

>> +
>> +test_expect_success 'Check that various forms of specifying 
>> core.hooksDirectory work' '
>> + test_commit no_custom_hook &&
>> + git config core.hooksDirectory .git/custom-hooks &&
>> + test_commit have_custom_hook &&
>> + git config core.hooksDirectory .git/custom-hooks/ &&
>> + test_commit have_custom_hook_trailing_slash &&
>
> These two cases ensure that it should work even when the configured
> hook directory is given as a relative path, though the docs say it
> should be an absolute path.

Yeah I didn't mention this in the docs or commit message, but I
figured not promising anything else in the docs made sense.

I.e. a relative path of e.g. .git/custom-hook/* won't work if you're
cd'd into a subdirectory of your repository, so telling users that it
has to be absolute is probably an OK white lie.

>> + git config core.hooksDirectory "$PWD/.git/custom-hooks" &&
>> + test_commit have_custom_hook_abs_path &&
>> + git config core.hooksDirectory "$PWD/.git/custom-hooks/" &&
>> + test_commit have_custom_hook_abs_path_trailing_slash &&
>> +printf "%s" "" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
>> +test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect 
>> .git/PRE-COMMIT-HOOK-WAS-CALLED
>
> Indentation with spaces.

Will fix.

>> +'
>> +
>> +test_done
>> --
>> 2.1.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH] hooks: Add ability to specify where the hook directory is

2016-04-22 Thread SZEDER Gábor

> Change the hardcoded lookup for .git/hooks/* to optionally lookup in
> $(git config core.hooksDirectory)/* instead if that config key is set.
> 
> This is essentially a more intrusive version of the git-init ability to
> specify hooks on init time via init templates.
> 
> The difference between that facility and this feature is that this can
> be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
> apply for all your personal repositories, or all repositories on the
> system.
> 
> I plan on using this on a centralized Git server where users can create
> arbitrary repositories under /gitroot, but I'd like to manage all the
> hooks that should be run centrally via a unified dispatch mechanism.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  Documentation/config.txt  | 10 ++
>  Documentation/githooks.txt|  5 -
>  cache.h   |  1 +
>  config.c  |  3 +++
>  environment.c |  1 +
>  run-command.c |  5 -
>  t/t1350-config-hooks-directory.sh | 35 +++
>  7 files changed, 58 insertions(+), 2 deletions(-)
>  create mode 100755 t/t1350-config-hooks-directory.sh
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 42d2b50..2faf3c0 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -618,6 +618,16 @@ core.attributesFile::
>   $XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
>   set or empty, $HOME/.config/git/attributes is used instead.
>  
> +core.hooksDirectory::
> + By default Git will look for your hooks in the '$GIT_DIR/hooks'
> + directory. Set this to different absolute directory name,

Mental note: here you say that it should be an absolute directory.

> + e.g. '/etc/git/hooks', and Git will try to find your hooks that

s/hooks that/hooks in that/

> + directory, e.g. '/etc/git/hooks/pre-receive' instead of in
> + '$GIT_DIR/hooks'.
> ++
> +This is useful in cases where you'd like to centrally configure your
> +Git hooks instead of configuring them on a per-repository basis.
> +
>  core.editor::
>   Commands such as `commit` and `tag` that lets you edit
>   messages by launching an editor uses the value of this


> diff --git a/t/t1350-config-hooks-directory.sh 
> b/t/t1350-config-hooks-directory.sh
> new file mode 100755
> index 000..556c1d3
> --- /dev/null
> +++ b/t/t1350-config-hooks-directory.sh
> @@ -0,0 +1,35 @@
> +#!/bin/sh
> +
> +test_description='Test the core.hooksDirectory configuration variable'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'set up a pre-commit hook in core.hooksDirectory' '
> + mkdir -p .git/custom-hooks .git/hooks &&
> + cat >.git/custom-hooks/pre-commit < +#!$SHELL_PATH
> +printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> + cat >.git/hooks/pre-commit < + chmod +x .git/hooks/pre-commit
> +#!$SHELL_PATH
> +printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> + chmod +x .git/custom-hooks/pre-commit
> +'

Please use the 'write_script' helper for, well, writing scripts.

> +
> +test_expect_success 'Check that various forms of specifying 
> core.hooksDirectory work' '
> + test_commit no_custom_hook &&
> + git config core.hooksDirectory .git/custom-hooks &&
> + test_commit have_custom_hook &&
> + git config core.hooksDirectory .git/custom-hooks/ &&
> + test_commit have_custom_hook_trailing_slash &&

These two cases ensure that it should work even when the configured
hook directory is given as a relative path, though the docs say it
should be an absolute path.

> + git config core.hooksDirectory "$PWD/.git/custom-hooks" &&
> + test_commit have_custom_hook_abs_path &&
> + git config core.hooksDirectory "$PWD/.git/custom-hooks/" &&
> + test_commit have_custom_hook_abs_path_trailing_slash &&
> +printf "%s" "" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
> +test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect 
> .git/PRE-COMMIT-HOOK-WAS-CALLED

Indentation with spaces.

> +'
> +
> +test_done
> -- 
> 2.1.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: make test Unexpected passes

2016-04-22 Thread Ben Woosley
Ramsay Jones  ramsayjones.plus.com> writes:

> 
> Hi Ben, Junio,
> 
> Tonight, the testsuite passed with a couple of 'unexpected passes', viz:
>
> In the first case, t3421-*.sh, git bisect fingered commit f32ec670
> ("git-rebase--merge: don't include absent parent as a base", 20-04-2016).
>
> ATB,
> Ramsay Jones
> 
 
Yep,

These know breakages:

ok 50 - rebase -m --onto --root
ok 54 - rebase -m without --onto --root with disjoint history

Have to do with rebasing a root/orphan branch with the -m flag,
which defaults to -- merge=recursive, which is the case the patch fixed.

Here are the necessary changes:

--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -253,7 +253,7 @@ test_run_rebase () {
"
 }
 test_run_rebase success ''
-test_run_rebase failure -m
+test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase success -p
 
@@ -268,7 +268,7 @@ test_run_rebase () {
"
 }
 test_run_rebase success ''
-test_run_rebase failure -m
+test_run_rebase success -m
 test_run_rebase success -i
 test_run_rebase failure -p
 
-- 
2.8.1.211.g8e54d77


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

2016-04-22 Thread Ævar Arnfjörð Bjarmason
On Sat, Apr 23, 2016 at 1:33 AM, Ævar Arnfjörð Bjarmason
 wrote:
> Change the hardcoded lookup for .git/hooks/* to optionally lookup in
> $(git config core.hooksDirectory)/* instead if that config key is set.

I think this'll do for my use-case, but I started with a rather more
ambitious patch that I could forsee not finishing today.

I wanted to support executing e.g. the pre-commit hook, in order, from any of:

.git/hooks/pre-commit
.git/hooks/pre-commit.d/*
 [We could add some ~-wide path here I guess...]
 /etc/git/hooks/pre-commit
 /etc/git/hooks/pre-commit.d/*

Where the * would be resolved in glob() order.

The motivation was solving the use-case I'm solving with
core.hooksDirectory, perhaps with a config variable to set whether you
wanted to skip per-repo or system-wide hooks, but also having
something more general.

The reason for supporting the *.d directories was that I spotted a lot
of hooks people had hacked up at work using the pee(1) command[1] to
run sequences of other unrelated hook commands.

Just symlinking stuff is simpler and more portable if we do the work
in git.git once. We'd run  The pee(1) command also doesn't quit on the
first command that returns nonzero, which would make sense e.g. for
pre-commit hooks.

I have it working for the hooks that use the simple run_hook_ve()
interface, but the ones that have to e.g. pass input on stdin just
find_hook() directly & do a custom run_command(), so all of those
callsites would have to be patched and/or I'd have to hack up some
custom callback mechanism.

1. http://manpages.ubuntu.com/manpages/xenial/en/man1/pee.1.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] hooks: Add ability to specify where the hook directory is

2016-04-22 Thread Ævar Arnfjörð Bjarmason
Change the hardcoded lookup for .git/hooks/* to optionally lookup in
$(git config core.hooksDirectory)/* instead if that config key is set.

This is essentially a more intrusive version of the git-init ability to
specify hooks on init time via init templates.

The difference between that facility and this feature is that this can
be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
apply for all your personal repositories, or all repositories on the
system.

I plan on using this on a centralized Git server where users can create
arbitrary repositories under /gitroot, but I'd like to manage all the
hooks that should be run centrally via a unified dispatch mechanism.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt  | 10 ++
 Documentation/githooks.txt|  5 -
 cache.h   |  1 +
 config.c  |  3 +++
 environment.c |  1 +
 run-command.c |  5 -
 t/t1350-config-hooks-directory.sh | 35 +++
 7 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100755 t/t1350-config-hooks-directory.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..2faf3c0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -618,6 +618,16 @@ core.attributesFile::
$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
set or empty, $HOME/.config/git/attributes is used instead.
 
+core.hooksDirectory::
+   By default Git will look for your hooks in the '$GIT_DIR/hooks'
+   directory. Set this to different absolute directory name,
+   e.g. '/etc/git/hooks', and Git will try to find your hooks that
+   directory, e.g. '/etc/git/hooks/pre-receive' instead of in
+   '$GIT_DIR/hooks'.
++
+This is useful in cases where you'd like to centrally configure your
+Git hooks instead of configuring them on a per-repository basis.
+
 core.editor::
Commands such as `commit` and `tag` that lets you edit
messages by launching an editor uses the value of this
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a2f59b1..e1fe66d 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -13,13 +13,16 @@ $GIT_DIR/hooks/*
 DESCRIPTION
 ---
 
-Hooks are little scripts you can place in `$GIT_DIR/hooks`
+Hooks are little scripts you can place in the `hooks`
 directory to trigger action at certain points.  When
 'git init' is run, a handful of example hooks are copied into the
 `hooks` directory of the new repository, but by default they are
 all disabled.  To enable a hook, rename it by removing its `.sample`
 suffix.
 
+By default the hooks directory is `$GIT_DIR/hooks`, but that can be
+changed via the `core.hooksDirectory` (see linkgit:git-config[1]).
+
 NOTE: It is also a requirement for a given hook to be executable.
 However - in a freshly initialized repository - the `.sample` files are
 executable by default.
diff --git a/cache.h b/cache.h
index 2711048..85ad594 100644
--- a/cache.h
+++ b/cache.h
@@ -654,6 +654,7 @@ extern int warn_on_object_refname_ambiguity;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
 extern const char *git_attributes_file;
+extern const char *git_hooks_directory;
 extern int zlib_compression_level;
 extern int core_compression_level;
 extern int core_compression_seen;
diff --git a/config.c b/config.c
index 10b5c95..543de4e 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,9 @@ static int git_default_core_config(const char *var, const 
char *value)
if (!strcmp(var, "core.attributesfile"))
return git_config_pathname(_attributes_file, var, value);
 
+   if (!strcmp(var, "core.hooksdirectory"))
+   return git_config_pathname(_hooks_directory, var, value);
+
if (!strcmp(var, "core.bare")) {
is_bare_repository_cfg = git_config_bool(var, value);
return 0;
diff --git a/environment.c b/environment.c
index 57acb2f..ffb5dec 100644
--- a/environment.c
+++ b/environment.c
@@ -31,6 +31,7 @@ const char *git_log_output_encoding;
 const char *apply_default_whitespace;
 const char *apply_default_ignorewhitespace;
 const char *git_attributes_file;
+const char *git_hooks_directory;
 int zlib_compression_level = Z_BEST_SPEED;
 int core_compression_level;
 int core_compression_seen;
diff --git a/run-command.c b/run-command.c
index 8c7115a..ae8e470 100644
--- a/run-command.c
+++ b/run-command.c
@@ -815,7 +815,10 @@ const char *find_hook(const char *name)
static struct strbuf path = STRBUF_INIT;
 
strbuf_reset();
-   strbuf_git_path(, "hooks/%s", name);
+   if (git_hooks_directory)
+   strbuf_addf(, "%s/%s", git_hooks_directory, name);
+   else
+   strbuf_git_path(, "hooks/%s", name);
if (access(path.buf, 

Re: issue upgrading git from 1.8.2.1 to 2.8.0

2016-04-22 Thread Jeff King
On Fri, Apr 22, 2016 at 06:44:02PM -0400, Gennaro Torre wrote:

> When we upgraded to 2.8.0, all nodes received the upgrade.
> 
> We observed that when we tried to deploy code to the nodes (they run
> `git fetch` to update the repository) this would fail.

Did `git fetch` say anything interesting on stderr?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: issue upgrading git from 1.8.2.1 to 2.8.0

2016-04-22 Thread Junio C Hamano
Stefan Beller  writes:

> Although 1.8..2.8 is a lot, Git ought to stay compatible there.

In principle that is true, but there were deliberate backward
incompatible changes at around 2.0 boundary, and those who followed
along the upgrade paths were given warnings and advice messages to
guide them to adjust their configurations during the incremental
upgrade path.  It is not all that surprising if somebody who jumped
between the two versions in one hop didn't benefit from any such
guidance, though.

Offhand, perhaps .git/config left by ancient versions and recent
versions may differ in such a way that it affects the particular
workflow deployed there?




--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: issue upgrading git from 1.8.2.1 to 2.8.0

2016-04-22 Thread Stefan Beller
On Fri, Apr 22, 2016 at 3:44 PM, Gennaro Torre  wrote:
>
> Hi,
>
> Last week we upgraded git from 1.8.2.1 to 2.8.0. We saw some very
> weird behavior where git clones and pushes were working, but git fetch
> was not.
>
> Here is our setup:
>
> We have 10+ nodes caching a few repositories, the repos were
> originally cloned via git 1.8.2.1 with `git clone --mirror `
>
> We have 3000+ nodes that take deploys using the git protocol, via `git
> fetch`, the existing repositories were also cloned via git 1.8.2.1
> from the caching nodes.
>
> When we upgraded to 2.8.0, all nodes received the upgrade.
>
> We observed that when we tried to deploy code to the nodes (they run
> `git fetch` to update the repository) this would fail.
>
> The fix: we deleted the repositories originally cloned via git
> 1.8.2.1, and did a `git clone ` dropping a fresh repository that
> was cloned using git 2.8.0. Everything started working correctly. Our
> running theory here is that there was some incompatibility with the
> repositories cloned with the old version of git, and trying to run
> `git fetch` with the newest version of git.

Do you still have an old repository?
You could compare a new and old repository with e.g. meld
to see if there are differences you don't expect.

Although 1.8..2.8 is a lot, Git ought to stay compatible there.

Thanks,
Stefan

>
>
> Thanks,
> Gennaro
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


issue upgrading git from 1.8.2.1 to 2.8.0

2016-04-22 Thread Gennaro Torre
Hi,

Last week we upgraded git from 1.8.2.1 to 2.8.0. We saw some very
weird behavior where git clones and pushes were working, but git fetch
was not.

Here is our setup:

We have 10+ nodes caching a few repositories, the repos were
originally cloned via git 1.8.2.1 with `git clone --mirror `

We have 3000+ nodes that take deploys using the git protocol, via `git
fetch`, the existing repositories were also cloned via git 1.8.2.1
from the caching nodes.

When we upgraded to 2.8.0, all nodes received the upgrade.

We observed that when we tried to deploy code to the nodes (they run
`git fetch` to update the repository) this would fail.

The fix: we deleted the repositories originally cloned via git
1.8.2.1, and did a `git clone ` dropping a fresh repository that
was cloned using git 2.8.0. Everything started working correctly. Our
running theory here is that there was some incompatibility with the
repositories cloned with the old version of git, and trying to run
`git fetch` with the newest version of git.

Thanks,
Gennaro
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: make test Unexpected passes

2016-04-22 Thread Elijah Newren
On Fri, Apr 22, 2016 at 1:05 PM, Ramsay Jones
 wrote:
> Hi Ben, Junio,
>
> In the second case, t6036-*.sh, git bisect fingered commit b61f9d6e
> ("ll-merge: use a longer conflict marker for internal merge", 14-04-2016).

Yeah, the t6036 testcase 'git detects conflict w/
criss-cross+contrived resolution' could be made to pass by tweaking
the conflict markers.  In fact, any tweak would make it appear to
pass, but the test could be updated to still fail by updating the
contrived resolution of a previous merge to use the newer conflict
marker style as well.

The test is kind of fragile in this way, and is really there just to
document this slightly weird and never seen in practice corner case.
I don't think we'll ever fix the underlying "problem"; not even sure
if it's possible without fundamentally re-thinking our merge strategy
altogether.  I just thought that when I was writing all those tests
that documenting this particular behavior as a testcase had some
value, but if the conflict markers are going to be updated
periodically, then the cost of the test may outweigh its value and we
should just toss this one test from that file.

Elijah
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

2016-04-22 Thread Junio C Hamano
Junio C Hamano  writes:

>>>  Is this going to be rerolled?
>>>  ($gmane/291382)
>>
>> The changes weren't that big enough and I had my end semester exams
>> coming so I decided not to re-roll it.
>> If you think contrary, I can do a re-roll with the changes suggested
>> by Eric Sunshine.
>
> I agree that this was pretty close to be done.  Let me see if I can
> find time to finish it up in the coming few days.

I looked at the reviews again and I take the above back--the end
result of applying all patches may not have to change a lot, but
the ordering and structure of the series may need cleanign up.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/6] Introduce a get_oid function.

2016-04-22 Thread brian m. carlson
The get_oid function is equivalent to the get_sha1 function, but uses a
struct object_id instead.

Signed-off-by: brian m. carlson 
---
 cache.h | 2 ++
 sha1_name.c | 9 +
 2 files changed, 11 insertions(+)

diff --git a/cache.h b/cache.h
index 2711048c..22b73646 100644
--- a/cache.h
+++ b/cache.h
@@ -1156,6 +1156,8 @@ extern int get_sha1_blob(const char *str, unsigned char 
*sha1);
 extern void maybe_die_on_misspelt_object_name(const char *name, const char 
*prefix);
 extern int get_sha1_with_context(const char *str, unsigned flags, unsigned 
char *sha1, struct object_context *orc);
 
+extern int get_oid(const char *str, struct object_id *oid);
+
 typedef int each_abbrev_fn(const unsigned char *sha1, void *);
 extern int for_each_abbrev(const char *prefix, each_abbrev_fn, void *);
 
diff --git a/sha1_name.c b/sha1_name.c
index 776101e8..ca7ddd6f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1215,6 +1215,15 @@ int get_sha1(const char *name, unsigned char *sha1)
 }
 
 /*
+ * This is like "get_sha1()", but for struct object_id.
+ */
+int get_oid(const char *name, struct object_id *oid)
+{
+   return get_sha1(name, oid->hash);
+}
+
+
+/*
  * Many callers know that the user meant to name a commit-ish by
  * syntactical positions where the object name appears.  Calling this
  * function allows the machinery to disambiguate shorter-than-unique
-- 
2.8.1.369.geae769a

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 6/6] match-trees: convert several leaf functions to struct object_id

2016-04-22 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 match-trees.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/match-trees.c b/match-trees.c
index 8ca7c68f..396b7338 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -48,17 +48,17 @@ static int score_matches(unsigned mode1, unsigned mode2, 
const char *path)
 }
 
 static void *fill_tree_desc_strict(struct tree_desc *desc,
-  const unsigned char *hash)
+  const struct object_id *hash)
 {
void *buffer;
enum object_type type;
unsigned long size;
 
-   buffer = read_sha1_file(hash, , );
+   buffer = read_sha1_file(hash->hash, , );
if (!buffer)
-   die("unable to read tree (%s)", sha1_to_hex(hash));
+   die("unable to read tree (%s)", oid_to_hex(hash));
if (type != OBJ_TREE)
-   die("%s is not a tree", sha1_to_hex(hash));
+   die("%s is not a tree", oid_to_hex(hash));
init_tree_desc(desc, buffer, size);
return buffer;
 }
@@ -73,7 +73,7 @@ static int base_name_entries_compare(const struct name_entry 
*a,
 /*
  * Inspect two trees, and give a score that tells how similar they are.
  */
-static int score_trees(const unsigned char *hash1, const unsigned char *hash2)
+static int score_trees(const struct object_id *hash1, const struct object_id 
*hash2)
 {
struct tree_desc one;
struct tree_desc two;
@@ -119,8 +119,8 @@ static int score_trees(const unsigned char *hash1, const 
unsigned char *hash2)
 /*
  * Match one itself and its subtrees with two and pick the best match.
  */
-static void match_trees(const unsigned char *hash1,
-   const unsigned char *hash2,
+static void match_trees(const struct object_id *hash1,
+   const struct object_id *hash2,
int *best_score,
char **best_match,
const char *base,
@@ -138,7 +138,7 @@ static void match_trees(const unsigned char *hash1,
elem = tree_entry_extract(, , );
if (!S_ISDIR(mode))
goto next;
-   score = score_trees(elem->hash, hash2);
+   score = score_trees(elem, hash2);
if (*best_score < score) {
free(*best_match);
*best_match = xstrfmt("%s%s", base, path);
@@ -146,7 +146,7 @@ static void match_trees(const unsigned char *hash1,
}
if (recurse_limit) {
char *newbase = xstrfmt("%s%s/", base, path);
-   match_trees(elem->hash, hash2, best_score, best_match,
+   match_trees(elem, hash2, best_score, best_match,
newbase, recurse_limit - 1);
free(newbase);
}
@@ -245,7 +245,7 @@ void shift_tree(const struct object_id *hash1,
if (!depth_limit)
depth_limit = 2;
 
-   add_score = del_score = score_trees(hash1->hash, hash2->hash);
+   add_score = del_score = score_trees(hash1, hash2);
add_prefix = xcalloc(1, 1);
del_prefix = xcalloc(1, 1);
 
@@ -253,13 +253,13 @@ void shift_tree(const struct object_id *hash1,
 * See if one's subtree resembles two; if so we need to prefix
 * two with a few fake trees to match the prefix.
 */
-   match_trees(hash1->hash, hash2->hash, _score, _prefix, "", 
depth_limit);
+   match_trees(hash1, hash2, _score, _prefix, "", depth_limit);
 
/*
 * See if two's subtree resembles one; if so we need to
 * pick only subtree of two.
 */
-   match_trees(hash2->hash, hash1->hash, _score, _prefix, "", 
depth_limit);
+   match_trees(hash2, hash1, _score, _prefix, "", depth_limit);
 
/* Assume we do not have to do any shifting */
oidcpy(shifted, hash2);
@@ -309,16 +309,16 @@ void shift_tree_by(const struct object_id *hash1,
 
if (candidate == 3) {
/* Both are plausible -- we need to evaluate the score */
-   int best_score = score_trees(hash1->hash, hash2->hash);
+   int best_score = score_trees(hash1, hash2);
int score;
 
candidate = 0;
-   score = score_trees(sub1.hash, hash2->hash);
+   score = score_trees(, hash2);
if (score > best_score) {
candidate = 1;
best_score = score;
}
-   score = score_trees(sub2.hash, hash1->hash);
+   score = score_trees(, hash1);
if (score > best_score)
candidate = 2;
}
-- 
2.8.1.369.geae769a

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More 

[PATCH v2 2/6] test-match-trees: convert to use struct object_id

2016-04-22 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 test-match-trees.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test-match-trees.c b/test-match-trees.c
index 4dad7095..41aff841 100644
--- a/test-match-trees.c
+++ b/test-match-trees.c
@@ -3,24 +3,24 @@
 
 int main(int ac, char **av)
 {
-   unsigned char hash1[20], hash2[20], shifted[20];
+   struct object_id hash1, hash2, shifted;
struct tree *one, *two;
 
setup_git_directory();
 
-   if (get_sha1(av[1], hash1))
+   if (get_oid(av[1], ))
die("cannot parse %s as an object name", av[1]);
-   if (get_sha1(av[2], hash2))
+   if (get_oid(av[2], ))
die("cannot parse %s as an object name", av[2]);
-   one = parse_tree_indirect(hash1);
+   one = parse_tree_indirect(hash1.hash);
if (!one)
die("not a tree-ish %s", av[1]);
-   two = parse_tree_indirect(hash2);
+   two = parse_tree_indirect(hash2.hash);
if (!two)
die("not a tree-ish %s", av[2]);
 
-   shift_tree(one->object.oid.hash, two->object.oid.hash, shifted, -1);
-   printf("shifted: %s\n", sha1_to_hex(shifted));
+   shift_tree(one->object.oid.hash, two->object.oid.hash, shifted.hash, 
-1);
+   printf("shifted: %s\n", oid_to_hex());
 
exit(0);
 }
-- 
2.8.1.369.geae769a

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/6] match-trees: convert shift_tree and shift_tree_by to object_id

2016-04-22 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 cache.h|  4 ++--
 match-trees.c  | 44 ++--
 merge-recursive.c  |  4 ++--
 test-match-trees.c |  2 +-
 4 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/cache.h b/cache.h
index 22b73646..70091e73 100644
--- a/cache.h
+++ b/cache.h
@@ -1768,8 +1768,8 @@ int add_files_to_cache(const char *prefix, const struct 
pathspec *pathspec, int
 extern int diff_auto_refresh_index;
 
 /* match-trees.c */
-void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, 
int);
-void shift_tree_by(const unsigned char *, const unsigned char *, unsigned char 
*, const char *);
+void shift_tree(const struct object_id *, const struct object_id *, struct 
object_id *, int);
+void shift_tree_by(const struct object_id *, const struct object_id *, struct 
object_id *, const char *);
 
 /*
  * whitespace rules.
diff --git a/match-trees.c b/match-trees.c
index 1ce0954a..9977752a 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -229,9 +229,9 @@ static int splice_tree(const unsigned char *hash1,
  * other hand, it could cover tree one and we might need to pick a
  * subtree of it.
  */
-void shift_tree(const unsigned char *hash1,
-   const unsigned char *hash2,
-   unsigned char *shifted,
+void shift_tree(const struct object_id *hash1,
+   const struct object_id *hash2,
+   struct object_id *shifted,
int depth_limit)
 {
char *add_prefix;
@@ -245,7 +245,7 @@ void shift_tree(const unsigned char *hash1,
if (!depth_limit)
depth_limit = 2;
 
-   add_score = del_score = score_trees(hash1, hash2);
+   add_score = del_score = score_trees(hash1->hash, hash2->hash);
add_prefix = xcalloc(1, 1);
del_prefix = xcalloc(1, 1);
 
@@ -253,16 +253,16 @@ void shift_tree(const unsigned char *hash1,
 * See if one's subtree resembles two; if so we need to prefix
 * two with a few fake trees to match the prefix.
 */
-   match_trees(hash1, hash2, _score, _prefix, "", depth_limit);
+   match_trees(hash1->hash, hash2->hash, _score, _prefix, "", 
depth_limit);
 
/*
 * See if two's subtree resembles one; if so we need to
 * pick only subtree of two.
 */
-   match_trees(hash2, hash1, _score, _prefix, "", depth_limit);
+   match_trees(hash2->hash, hash1->hash, _score, _prefix, "", 
depth_limit);
 
/* Assume we do not have to do any shifting */
-   hashcpy(shifted, hash2);
+   oidcpy(shifted, hash2);
 
if (add_score < del_score) {
/* We need to pick a subtree of two */
@@ -271,16 +271,16 @@ void shift_tree(const unsigned char *hash1,
if (!*del_prefix)
return;
 
-   if (get_tree_entry(hash2, del_prefix, shifted, ))
+   if (get_tree_entry(hash2->hash, del_prefix, shifted->hash, 
))
die("cannot find path %s in tree %s",
-   del_prefix, sha1_to_hex(hash2));
+   del_prefix, oid_to_hex(hash2));
return;
}
 
if (!*add_prefix)
return;
 
-   splice_tree(hash1, add_prefix, hash2, shifted);
+   splice_tree(hash1->hash, add_prefix, hash2->hash, shifted->hash);
 }
 
 /*
@@ -288,44 +288,44 @@ void shift_tree(const unsigned char *hash1,
  * Unfortunately we cannot fundamentally tell which one to
  * be prefixed, as recursive merge can work in either direction.
  */
-void shift_tree_by(const unsigned char *hash1,
-  const unsigned char *hash2,
-  unsigned char *shifted,
+void shift_tree_by(const struct object_id *hash1,
+  const struct object_id *hash2,
+  struct object_id *shifted,
   const char *shift_prefix)
 {
-   unsigned char sub1[20], sub2[20];
+   struct object_id sub1, sub2;
unsigned mode1, mode2;
unsigned candidate = 0;
 
/* Can hash2 be a tree at shift_prefix in tree hash1? */
-   if (!get_tree_entry(hash1, shift_prefix, sub1, ) &&
+   if (!get_tree_entry(hash1->hash, shift_prefix, sub1.hash, ) &&
S_ISDIR(mode1))
candidate |= 1;
 
/* Can hash1 be a tree at shift_prefix in tree hash2? */
-   if (!get_tree_entry(hash2, shift_prefix, sub2, ) &&
+   if (!get_tree_entry(hash2->hash, shift_prefix, sub2.hash, ) &&
S_ISDIR(mode2))
candidate |= 2;
 
if (candidate == 3) {
/* Both are plausible -- we need to evaluate the score */
-   int best_score = score_trees(hash1, hash2);
+   int best_score = score_trees(hash1->hash, hash2->hash);
int score;
 
candidate = 0;
-   score = score_trees(sub1, hash2);
+   score = 

[PATCH v2 0/6] object_id Part 3

2016-04-22 Thread brian m. carlson
This is the third of a series of patches to convert instances of
unsigned char[20] to struct object_id.  The focus in this series was to
convert test-match-trees, related functions, and some dependencies.
struct name_entry was converted as part of these dependencies.

The riskiest part of this series is the conversion of struct name_entry.
Compatibility with unconverted code requires dereferencing the new oid
member, but there are at least some places where we explicitly check
that it is not NULL.  Most of these are protected by a check for a
nonzero mode.

This series rebases onto next cleanly and is not likely to conflict with
other topics in flight.  My intention is to send smaller series more
frequently, with the goal of avoiding or minimizing conflicts where
possible.

Changes from v1:
* Remove dereference of object_id pointer in boolean context.

brian m. carlson (6):
  Introduce a get_oid function.
  test-match-trees: convert to use struct object_id
  match-trees: convert shift_tree and shift_tree_by to object_id
  Convert struct name_entry to use struct object_id.
  tree-walk: convert tree_entry_extract to struct object_id
  match-trees: convert several leaf functions to struct object_id

 builtin/grep.c |  6 ++---
 builtin/merge-tree.c   | 18 +++
 builtin/pack-objects.c |  4 ++--
 builtin/reflog.c   |  4 ++--
 cache-tree.c   |  4 ++--
 cache.h|  6 +++--
 fsck.c | 10 -
 http-push.c|  4 ++--
 list-objects.c |  6 ++---
 match-trees.c  | 60 +-
 merge-recursive.c  |  4 ++--
 notes.c|  4 ++--
 revision.c |  4 ++--
 sha1_name.c|  9 
 test-match-trees.c | 14 ++--
 tree-diff.c|  8 +++
 tree-walk.c| 16 +++---
 tree-walk.h|  8 +++
 tree.c | 10 -
 unpack-trees.c |  4 ++--
 walker.c   |  4 ++--
 21 files changed, 109 insertions(+), 98 deletions(-)

-- 
2.8.1.369.geae769a

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 01/10] t0027: Make more reliable

2016-04-22 Thread Junio C Hamano
tbo...@web.de writes:

> From: Torsten Bögershausen 
> Subject: Re: [PATCH v6 01/10] t0027: Make more reliable

"Make more reliable" does not sound very grammatical.

> Make the commit_chk_wrnNNO test in t0027 more reliable:

Neither is the colon at the end.

> When the attributes of a commited file are changed and the file is otherwise
> unchanged, Git may not detect that the next commit may need to treat the
> file as changed.
> This happens when lstat() doesn't detect a change, since neither inode,
> mtime nor size are changed.
>
> Add a singe "Z" character to change the file size.
> Ignore it when comparing the files later.
>
> Signed-off-by: Torsten Bögershausen 
> ---
>  Changes since v5:
>  - send the whole series, now 10/10
>  - Removed the "will change in future" in one commit msg
>  - Don't leak the filer in 4/10
>  t/t0027-auto-crlf.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index f33962b..9fe539b 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -12,7 +12,7 @@ fi
>  
>  compare_files () {
>   tr '\015\000' QN <"$1" >"$1".expect &&
> - tr '\015\000' QN <"$2" >"$2".actual &&
> + tr '\015\000' QN <"$2" | tr -d 'Z' >"$2".actual &&
>   test_cmp "$1".expect "$2".actual &&
>   rm "$1".expect "$2".actual
>  }
> @@ -114,6 +114,7 @@ commit_chk_wrnNNO () {
>   do
>   fname=${pfx}_$f.txt &&
>   cp $f $fname &&
> + printf Z >>"$fname" &&
>   git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
>   git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname 
> >"${pfx}_$f.err" 2>&1
>   done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 4/6] Convert struct name_entry to use struct object_id.

2016-04-22 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 builtin/grep.c |  6 +++---
 builtin/merge-tree.c   | 18 +-
 builtin/pack-objects.c |  4 ++--
 builtin/reflog.c   |  4 ++--
 cache-tree.c   |  4 ++--
 fsck.c |  4 ++--
 http-push.c|  4 ++--
 list-objects.c |  6 +++---
 match-trees.c  |  2 +-
 notes.c|  4 ++--
 revision.c |  4 ++--
 tree-diff.c|  6 +++---
 tree-walk.c|  6 +++---
 tree-walk.h|  6 +++---
 tree.c | 10 +-
 unpack-trees.c |  4 ++--
 walker.c   |  4 ++--
 17 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 111b6f6c..462e6079 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -438,7 +438,7 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
strbuf_add(base, entry.path, te_len);
 
if (S_ISREG(entry.mode)) {
-   hit |= grep_sha1(opt, entry.sha1, base->buf, tn_len,
+   hit |= grep_sha1(opt, entry.oid->hash, base->buf, 
tn_len,
 check_attr ? base->buf + tn_len : 
NULL);
}
else if (S_ISDIR(entry.mode)) {
@@ -447,10 +447,10 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
void *data;
unsigned long size;
 
-   data = lock_and_read_sha1_file(entry.sha1, , 
);
+   data = lock_and_read_sha1_file(entry.oid->hash, , 
);
if (!data)
die(_("unable to read tree (%s)"),
-   sha1_to_hex(entry.sha1));
+   oid_to_hex(entry.oid));
 
strbuf_addch(base, '/');
init_tree_desc(, data, size);
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index ca570041..5b7ab9b9 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -150,15 +150,15 @@ static void show_result(void)
 /* An empty entry never compares same, not even to another empty entry */
 static int same_entry(struct name_entry *a, struct name_entry *b)
 {
-   return  a->sha1 &&
-   b->sha1 &&
-   !hashcmp(a->sha1, b->sha1) &&
+   return  a->oid &&
+   b->oid &&
+   !oidcmp(a->oid, b->oid) &&
a->mode == b->mode;
 }
 
 static int both_empty(struct name_entry *a, struct name_entry *b)
 {
-   return !(a->sha1 || b->sha1);
+   return !(a->oid || b->oid);
 }
 
 static struct merge_list *create_entry(unsigned stage, unsigned mode, const 
unsigned char *sha1, const char *path)
@@ -188,8 +188,8 @@ static void resolve(const struct traverse_info *info, 
struct name_entry *ours, s
return;
 
path = traverse_path(info, result);
-   orig = create_entry(2, ours->mode, ours->sha1, path);
-   final = create_entry(0, result->mode, result->sha1, path);
+   orig = create_entry(2, ours->mode, ours->oid->hash, path);
+   final = create_entry(0, result->mode, result->oid->hash, path);
 
final->link = orig;
 
@@ -213,7 +213,7 @@ static void unresolved_directory(const struct traverse_info 
*info,
 
newbase = traverse_path(info, p);
 
-#define ENTRY_SHA1(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->sha1 : NULL)
+#define ENTRY_SHA1(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->oid->hash : 
NULL)
buf0 = fill_tree_descriptor(t+0, ENTRY_SHA1(n + 0));
buf1 = fill_tree_descriptor(t+1, ENTRY_SHA1(n + 1));
buf2 = fill_tree_descriptor(t+2, ENTRY_SHA1(n + 2));
@@ -239,7 +239,7 @@ static struct merge_list *link_entry(unsigned stage, const 
struct traverse_info
path = entry->path;
else
path = traverse_path(info, n);
-   link = create_entry(stage, n->mode, n->sha1, path);
+   link = create_entry(stage, n->mode, n->oid->hash, path);
link->link = entry;
return link;
 }
@@ -314,7 +314,7 @@ static int threeway_callback(int n, unsigned long mask, 
unsigned long dirmask, s
}
 
if (same_entry(entry+0, entry+1)) {
-   if (entry[2].sha1 && !S_ISDIR(entry[2].mode)) {
+   if (entry[2].oid && !S_ISDIR(entry[2].mode)) {
/* We did not touch, they modified -- take theirs */
resolve(info, entry+1, entry+2);
return mask;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index a27de5b3..d56b2c2d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1186,7 +1186,7 @@ static void add_pbase_object(struct tree_desc *tree,
if (cmp < 0)
return;
if (name[cmplen] != '/') {
-   

[PATCH v2 5/6] tree-walk: convert tree_entry_extract to struct object_id

2016-04-22 Thread brian m. carlson
Signed-off-by: brian m. carlson 
---
 fsck.c|  6 +++---
 match-trees.c | 12 ++--
 tree-diff.c   |  2 +-
 tree-walk.c   | 10 +-
 tree-walk.h   |  4 ++--
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/fsck.c b/fsck.c
index 606eba8c..92b17f5d 100644
--- a/fsck.c
+++ b/fsck.c
@@ -450,11 +450,11 @@ static int fsck_tree(struct tree *item, struct 
fsck_options *options)
while (desc.size) {
unsigned mode;
const char *name;
-   const unsigned char *sha1;
+   const struct object_id *oid;
 
-   sha1 = tree_entry_extract(, , );
+   oid = tree_entry_extract(, , );
 
-   has_null_sha1 |= is_null_sha1(sha1);
+   has_null_sha1 |= is_null_oid(oid);
has_full_path |= !!strchr(name, '/');
has_empty_name |= !*name;
has_dot |= !strcmp(name, ".");
diff --git a/match-trees.c b/match-trees.c
index 751f8f20..8ca7c68f 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -131,14 +131,14 @@ static void match_trees(const unsigned char *hash1,
 
while (one.size) {
const char *path;
-   const unsigned char *elem;
+   const struct object_id *elem;
unsigned mode;
int score;
 
elem = tree_entry_extract(, , );
if (!S_ISDIR(mode))
goto next;
-   score = score_trees(elem, hash2);
+   score = score_trees(elem->hash, hash2);
if (*best_score < score) {
free(*best_match);
*best_match = xstrfmt("%s%s", base, path);
@@ -146,7 +146,7 @@ static void match_trees(const unsigned char *hash1,
}
if (recurse_limit) {
char *newbase = xstrfmt("%s%s/", base, path);
-   match_trees(elem, hash2, best_score, best_match,
+   match_trees(elem->hash, hash2, best_score, best_match,
newbase, recurse_limit - 1);
free(newbase);
}
@@ -191,15 +191,15 @@ static int splice_tree(const unsigned char *hash1,
while (desc.size) {
const char *name;
unsigned mode;
-   const unsigned char *sha1;
+   const struct object_id *oid;
 
-   sha1 = tree_entry_extract(, , );
+   oid = tree_entry_extract(, , );
if (strlen(name) == toplen &&
!memcmp(name, prefix, toplen)) {
if (!S_ISDIR(mode))
die("entry %s in tree %s is not a tree",
name, sha1_to_hex(hash1));
-   rewrite_here = (unsigned char *) sha1;
+   rewrite_here = (unsigned char *) oid->hash;
break;
}
update_tree_entry();
diff --git a/tree-diff.c b/tree-diff.c
index 402f9ff2..ff4e0d3c 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -183,7 +183,7 @@ static struct combine_diff_path *emit_path(struct 
combine_diff_path *p,
 
if (t) {
/* path present in resulting tree */
-   sha1 = tree_entry_extract(t, , );
+   sha1 = tree_entry_extract(t, , )->hash;
pathlen = tree_entry_len(>entry);
isdir = S_ISDIR(mode);
} else {
diff --git a/tree-walk.c b/tree-walk.c
index fab57dd5..ce278424 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -433,10 +433,10 @@ static int find_tree_entry(struct tree_desc *t, const 
char *name, unsigned char
int namelen = strlen(name);
while (t->size) {
const char *entry;
-   const unsigned char *sha1;
+   const struct object_id *oid;
int entrylen, cmp;
 
-   sha1 = tree_entry_extract(t, , mode);
+   oid = tree_entry_extract(t, , mode);
entrylen = tree_entry_len(>entry);
update_tree_entry(t);
if (entrylen > namelen)
@@ -447,7 +447,7 @@ static int find_tree_entry(struct tree_desc *t, const char 
*name, unsigned char
if (cmp < 0)
break;
if (entrylen == namelen) {
-   hashcpy(result, sha1);
+   hashcpy(result, oid->hash);
return 0;
}
if (name[entrylen] != '/')
@@ -455,10 +455,10 @@ static int find_tree_entry(struct tree_desc *t, const 
char *name, unsigned char
if (!S_ISDIR(*mode))
break;
if (++entrylen == namelen) {
-   hashcpy(result, sha1);
+   hashcpy(result, oid->hash);
return 0;
}
- 

Re: [PATCH v5 2/4] format-patch: add '--base' option to record base tree info

2016-04-22 Thread Junio C Hamano
Xiaolong Ye  writes:

> +test_expect_success 'format-patch --base' '
> + git checkout side &&
> + git format-patch --stdout --base=HEAD~~~ -1 >patch &&
> + grep -e "^base-commit:" -A3 patch >actual &&

The -A3 is GNUism.  To do this portably, perhaps you can do

sed -n -e "/^base-commit:/,+3p"

or something like that.

But more importantly, grabbing 3 lines (and always 3 lines) will not
catch a future bug that somebody else may introduce to this code
that shows extra "prerequisite-patch-id:" after them.

> + echo "base-commit: $(git rev-parse HEAD~~~)" >expected &&
> + echo "prerequisite-patch-id: $(git show --patch HEAD~~ | git patch-id 
> --stable | awk "{print \$1}")" >>expected &&
> + echo "prerequisite-patch-id: $(git show --patch HEAD~ | git patch-id 
> --stable | awk "{print \$1}")" >>expected &&
> + test_cmp expected actual
> +'
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 3/4] format-patch: introduce --base=auto option

2016-04-22 Thread Junio C Hamano
Xiaolong Ye  writes:

> + if (upstream) {
> + unsigned char sha1[20];
> + if (get_sha1(upstream, sha1))
> + die(_("Failed to resolve '%s' as a valid 
> ref."), upstream);
> + struct commit *commit = lookup_commit_or_die(sha1, 
> "upstream base");
> + struct commit_list *base_list = 
> get_merge_bases_many(commit, total, list);

This introduces decl-after-statement.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 4/4] t1700-split-index: add test for rev-parse --shared-index-path

2016-04-22 Thread Michael Rappazzo
Signed-off-by: Michael Rappazzo 
---
 t/t1700-split-index.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 8aef49f..d2d9e02 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,4 +200,21 @@ EOF
test_cmp expect actual
 '
 
+test_expect_success 'rev-parse --shared-index-path' '
+   rm -rf .git &&
+   test_create_repo . &&
+   git update-index --split-index &&
+   ls -t .git/sharedindex* | tail -n 1 >expect &&
+   git rev-parse --shared-index-path >actual &&
+   test_cmp expect actual &&
+   mkdir work &&
+   test_when_finished "rm -rf work" &&
+   (
+   cd work &&
+   ls -t ../.git/sharedindex* | tail -n 1 >expect &&
+   git rev-parse --shared-index-path >actual &&
+   test_cmp expect actual
+   )
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/4] rev-parse: adjust results when they should be relative

2016-04-22 Thread Michael Rappazzo
Differences from v1[1]:
 - Simplify implementation based on review comments
 - Included similar changes in rev-parse for --git-path and --shared-index-path
 - Added tests and separated them into individual commits

[1] http://thread.gmane.org/gmane.comp.version-control.git/290669

Michael Rappazzo (4):
  rev-parse: fix some options when executed from subpath of main tree
  t1500-rev-parse: add tests executed from sub path of the main worktree
  t2027-worktree-list: add and adjust tests related to git-rev-parse
  t1700-split-index: add test for rev-parse --shared-index-path

 builtin/rev-parse.c  | 19 ++-
 t/t1500-rev-parse.sh | 37 +
 t/t1700-split-index.sh   | 17 +
 t/t2027-worktree-list.sh | 10 +-
 4 files changed, 77 insertions(+), 6 deletions(-)

-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/4] t2027-worktree-list: add and adjust tests related to git-rev-parse

2016-04-22 Thread Michael Rappazzo
Adjust the incorrect expectation for `rev-parse --git-common-dir`.

Add a test for `git rev-parse --git-path` executed from a linked
worktree.

Signed-off-by: Michael Rappazzo 
---
 t/t2027-worktree-list.sh | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 1b1b65a..16eec6e 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -14,10 +14,18 @@ test_expect_success 'rev-parse --git-common-dir on main 
worktree' '
test_cmp expected actual &&
mkdir sub &&
git -C sub rev-parse --git-common-dir >actual2 &&
-   echo sub/.git >expected2 &&
+   echo ../.git >expected2 &&
test_cmp expected2 actual2
 '
 
+test_expect_success 'rev-parse --git-path objects linked worktree' '
+   echo "$(git rev-parse 
--show-toplevel)/.git/worktrees/linked-tree/objects" >expect &&
+   test_when_finished "rm -rf linked-tree && git worktree prune" &&
+   git worktree add --detach linked-tree master &&
+   git -C linked-tree rev-parse --git-path objects >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success '"list" all worktrees from main' '
echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) 
[$(git symbolic-ref --short HEAD)]" >expect &&
test_when_finished "rm -rf here && git worktree prune" &&
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/4] t1500-rev-parse: add tests executed from sub path of the main worktree

2016-04-22 Thread Michael Rappazzo
Signed-off-by: Michael Rappazzo 
---
 t/t1500-rev-parse.sh | 37 +
 1 file changed, 37 insertions(+)

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 48ee077..1e220f7 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -36,6 +36,7 @@ test_rev_parse() {
 # label is-bare is-inside-git is-inside-work prefix git-dir
 
 ROOT=$(pwd)
+original_core_bare=$(git config core.bare)
 
 test_rev_parse toplevel false false true '' .git
 
@@ -84,4 +85,40 @@ test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true 
false false ''
 git config --unset core.bare
 test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
 
+#cleanup from the above
+cd ..
+rm -r work
+mv repo.git .git || exit 1
+unset GIT_DIR
+unset GIT_CONFIG
+git config core.bare $original_core_bare
+
+test_expect_success 'git-common-dir from worktree root' '
+   echo .git >expect &&
+   git rev-parse --git-common-dir >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'git-common-dir inside sub-dir' '
+   mkdir -p path/to/child &&
+   test_when_finished "rm -rf path" &&
+   echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
+   git -C path/to/child rev-parse --git-common-dir >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'git-path from worktree root' '
+   echo .git/objects >expect &&
+   git rev-parse --git-path objects >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'git-path inside sub-dir' '
+   mkdir -p path/to/child &&
+   test_when_finished "rm -rf path" &&
+   echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" 
>expect &&
+   git -C path/to/child rev-parse --git-path objects >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/4] rev-parse: fix some options when executed from subpath of main tree

2016-04-22 Thread Michael Rappazzo
Executing `git-rev-parse` with `--git-common-dir`, `--git-path `,
or `--shared-index-path` from the root of the main worktree results in
a relative path to the git dir.

When executed from a subdirectory of the main tree, however, it incorrectly
returns a path which starts 'sub/path/.git'.  Change this to return the
proper relative path to the git directory.

Signed-off-by: Michael Rappazzo 
---
 builtin/rev-parse.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index c961b74..1da2e10 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -564,10 +564,13 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
}
 
if (!strcmp(arg, "--git-path")) {
+   struct strbuf sb = STRBUF_INIT;
if (!argv[i + 1])
die("--git-path requires an argument");
-   puts(git_path("%s", argv[i + 1]));
-   i++;
+
+   puts(relative_path(xstrfmt("%s/%s", get_git_dir(), 
argv[++i]),
+   prefix, ));
+   strbuf_release();
continue;
}
if (as_is) {
@@ -787,8 +790,9 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, "--git-common-dir")) {
-   const char *pfx = prefix ? prefix : "";
-   puts(prefix_filename(pfx, strlen(pfx), 
get_git_common_dir()));
+   struct strbuf sb = STRBUF_INIT;
+   puts(relative_path(get_git_common_dir(), 
prefix, ));
+   strbuf_release();
continue;
}
if (!strcmp(arg, "--is-inside-git-dir")) {
@@ -811,7 +815,12 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
die(_("Could not read the index"));
if (the_index.split_index) {
const unsigned char *sha1 = 
the_index.split_index->base_sha1;
-   puts(git_path("sharedindex.%s", 
sha1_to_hex(sha1)));
+   struct strbuf sb = STRBUF_INIT;
+
+   puts(relative_path(
+   xstrfmt("%s/sharedindex.%s", 
get_git_dir(), sha1_to_hex(sha1)),
+   prefix, ));
+   strbuf_release();
}
continue;
}
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/4] format-patch: add '--base' option to record base tree info

2016-04-22 Thread Junio C Hamano
Xiaolong Ye  writes:

> +static struct commit *get_base_commit(const char *base_commit,
> +   struct commit **list,
> +   int total)
> +{
> + struct commit *base = NULL;
> + struct commit **rev;
> + int i = 0, rev_nr = 0;
> +
> + base = lookup_commit_reference_by_name(base_commit);
> + if (!base)
> + die(_("Unknown commit %s"), base_commit);
> +
> + ALLOC_ARRAY(rev, total);
> + for (i = 0; i < total; i++)
> + rev[i] = list[i];
> +
> + rev_nr = total;
> + /*
> +  * Get merge base through pair-wise computations
> +  * and store it in rev[0].
> +  */
> + while (rev_nr > 1) {
> + for (i = 0; i < rev_nr / 2; i++) {
> + struct commit_list *merge_base;
> + merge_base = get_merge_bases(rev[2 * i], rev[2 * i + 
> 1]);
> + if (!merge_base || merge_base->next)
> + die(_("Failed to find exact merge base"));
> +
> + rev[i] = merge_base->item;
> + }

So merge-base(0,1) is stored in rev[0], merge-base(2,3) is then
stored in rev[1], etc. and the last item, if rev_nr is odd, is left
in rev[rev_nr-1].  When the loop finishes, i is left as rev_nr/2
and...

> + if (rev_nr % 2)
> + rev[i] = rev[2 * i];

... when rev_nr is odd, that left-over thing moved down here.
E.g. if rev_nr == 5, the loop is left with i==2, rev[0] and rev[1]
are filled with pairwise merge bases, and this moves rev[4] to
rev[2], so that we can further process rev[0,1,2] with rev_nr set to
3 (i.e. (rev_nr + 1) / 2 below).

Sounds correct.

> + rev_nr = (rev_nr + 1) / 2;
> + }
> +
> + if (!in_merge_bases(base, rev[0]))
> + die(_("base commit should be the ancestor of revision list"));
> +
> + for (i = 0; i < total; i++) {
> + if (base == list[i])
> + die(_("base commit shouldn't be in revision list"));
> + }
> +
> + free(rev);
> + return base;
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/4] format-patch: add '--base' option to record base tree info

2016-04-22 Thread Junio C Hamano
Xiaolong Ye  writes:

> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index eed2981..a6ce727 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1460,4 +1460,19 @@ test_expect_success 'format-patch -o overrides 
> format.outputDirectory' '
>   test_path_is_dir patchset
>  '
>  
> +test_expect_success 'format-patch --base' '
> + git checkout side &&
> + git format-patch --stdout --base=HEAD~~~ -1 >patch &&
> + grep -e "^base-commit:" -A3 patch >actual &&
> + echo "base-commit: $(git rev-parse HEAD~~~)" >expected &&

HEAD~3 would be easier to read (and HEAD~2 is easier than HEAD~~).

> + echo "prerequisite-patch-id: $(git show --patch HEAD~~ | git patch-id 
> --stable | awk "{print \$1}")" >>expected &&
> + echo "prerequisite-patch-id: $(git show --patch HEAD~ | git patch-id 
> --stable | awk "{print \$1}")" >>expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'format-patch --base error handling' '
> + ! git format-patch --base=HEAD~ -2 &&
> + ! git format-patch --base=HEAD~ -3
> +'

When making sure that "git" exits with a failure in a controlled way
(i.e. you want to consider "git" that segfaults as not passing the
test), do not use "! git cmd", but use "test_must_fail git cmd"
instead.

You now have a quite elaborate logic in base validation in this
round.  Is the topology of the history used in this test still
complex enough to make sure the logic is being tested?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-lib: simplify '--option=value' parsing

2016-04-22 Thread Junio C Hamano
Jeff King  writes:

> For some reason I had always assumed that complicated pattern matching
> with "#" was non-POSIX, but I checked and it is definitely in there.

No, the offending code really dates back to the days back when I was
writing most of the shell scripts (or others were mostly copying
from what I have written).  I had this irrational aversion against
variable substitutions that are beyond ${var-default}, ${var=assign}
and ${var+isset}.

The conversion looks fine (modulo one missed one you noticed).

Thanks for cleaning things up.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] name-rev: include taggerdate in considering the best name

2016-04-22 Thread Junio C Hamano
Linus Torvalds  writes:

> ... I think this is still the simplest model we can use
> without trying to really do a topo-sort. And in many ways it's the
> simplest one to explain to people too: "we try to use the oldest
> reference we can find as a base for the resulting name" is not a
> complex or hard concept to explain.

Yes, the more I look at Dscho's patch, the more like it exactly for
that reason.  Its behaviour is very simple from the end-user's point
of view, unlike the historical one.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] test-lib: simplify '--option=value' parsing

2016-04-22 Thread SZEDER Gábor
To get the 'value' from '--option=value', test-lib.sh parses said
option running 'expr' with a regexp.  This involves a subshell, an
external process, and a lot of non-alphanumeric characters in the
regexp.

Use a much simpler POSIX-defined shell parameter expansion instead to
do the same.

Signed-off-by: SZEDER Gábor 
---

>>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> I count 5 cases in my copy of test-lib.sh. I think you are missing
> "--run".

Oh, indeed, '--run' managed to hide between these
-l|--l|--lo|--lon|--long patterns.


 t/test-lib.sh | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0b47eb6bb299..79afa8748eec 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -202,13 +202,13 @@ do
}
run_list=$1; shift ;;
--run=*)
-   run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
+   run_list=${1#--*=}; shift ;;
-h|--h|--he|--hel|--help)
help=t; shift ;;
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
verbose=t; shift ;;
--verbose-only=*)
-   verbose_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+   verbose_only=${1#--*=}
shift ;;
-q|--q|--qu|--qui|--quie|--quiet)
# Ignore --quiet under a TAP::Harness. Saying how many tests
@@ -222,15 +222,15 @@ do
valgrind=memcheck
shift ;;
--valgrind=*)
-   valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
+   valgrind=${1#--*=}
shift ;;
--valgrind-only=*)
-   valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+   valgrind_only=${1#--*=}
shift ;;
--tee)
shift ;; # was handled already
--root=*)
-   root=$(expr "z$1" : 'z[^=]*=\(.*\)')
+   root=${1#--*=}
shift ;;
--chain-lint)
GIT_TEST_CHAIN_LINT=1
-- 
2.8.1.99.g5d5236f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


make test Unexpected passes

2016-04-22 Thread Ramsay Jones
Hi Ben, Junio,

Tonight, the testsuite passed with a couple of 'unexpected passes', viz:

$ tail -17 ptest-out
[13:24:29]
All tests successful.

Test Summary Report
---
t3421-rebase-topology-linear.sh  (Wstat: 0 Tests: 76 Failed: 0)
  TODO passed:   50, 54
t6036-recursive-corner-cases.sh  (Wstat: 0 Tests: 22 Failed: 0)
  TODO passed:   11
Files=746, Tests=13515, 445 wallclock secs ( 3.83 usr  0.61 sys + 52.78 cusr 
27.89 csys = 85.11 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory `/home/ramsay/git/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory `/home/ramsay/git/t'
make[1]: Leaving directory `/home/ramsay/git/t'
$ 

In the first case, t3421-*.sh, git bisect fingered commit f32ec670
("git-rebase--merge: don't include absent parent as a base", 20-04-2016).

In the second case, t6036-*.sh, git bisect fingered commit b61f9d6e
("ll-merge: use a longer conflict marker for internal merge", 14-04-2016).

I won't have any time tonight to look into this any further (are these
false positives?), so I thought I would just make sure you were aware
of these 'unexpected passes'.

ATB,
Ramsay Jones

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 14/16] branch, tag: use porcelain output

2016-04-22 Thread Karthik Nayak
Call ref-filter's setup_ref_filter_porcelain_msg() to enable
translated messages for the %(upstream:tack) atom. Although branch.c
doesn't currently use ref-filter's printing API's, this will ensure
that when it does in the future patches, we do not need to worry about
translation.

Written-by: Matthieu Moy 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c | 2 ++
 builtin/tag.c| 2 ++
 2 files changed, 4 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index d467840..ecd7ffc 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -632,6 +632,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPT_END(),
};
 
+   setup_ref_filter_porcelain_msg();
+
memset(, 0, sizeof(filter));
filter.kind = FILTER_REFS_BRANCHES;
filter.abbrev = -1;
diff --git a/builtin/tag.c b/builtin/tag.c
index 528a1ba..3b51be1 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -379,6 +379,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_END()
};
 
+   setup_ref_filter_porcelain_msg();
+
git_config(git_tag_config, sorting_tail);
 
memset(, 0, sizeof(opt));
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 16/16] branch: implement '--format' option

2016-04-22 Thread Karthik Nayak
Implement the '--format' option provided by 'ref-filter'. This lets the
user list branches as per desired format similar to the implementation
in 'git for-each-ref'.

Add tests and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-branch.txt |  7 ++-
 builtin/branch.c | 14 +-
 t/t3203-branch-output.sh | 14 ++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 4a7037f..8af132f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[--list] [-v [--abbrev= | --no-abbrev]]
[--column[=] | --no-column]
[(--merged | --no-merged | --contains) []] [--sort=]
-   [--points-at ] [...]
+   [--points-at ] [--format=] [...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f]  
[]
 'git branch' (--set-upstream-to= | -u ) []
 'git branch' --unset-upstream []
@@ -246,6 +246,11 @@ start-point is either a local or remote-tracking branch.
 --points-at ::
Only list branches of the given object.
 
+--format ::
+   A string that interpolates `%(fieldname)` from the object
+   pointed at by a ref being shown.  The format is the same as
+   that of linkgit:git-for-each-ref[1].
+
 Examples
 
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 07068d2..6847ac3 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -28,6 +28,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r] (-d | -D) ..."),
N_("git branch [] (-m | -M) [] "),
N_("git branch [] [-r | -a] [--points-at]"),
+   N_("git branch [] [-r | -a] [--format]"),
NULL
 };
 
@@ -340,14 +341,14 @@ static char *build_format(struct ref_filter *filter, int 
maxwidth, const char *r
return strbuf_detach(, NULL);
 }
 
-static void print_ref_list(struct ref_filter *filter, struct ref_sorting 
*sorting)
+static void print_ref_list(struct ref_filter *filter, struct ref_sorting 
*sorting, const char *format)
 {
int i;
struct ref_array array;
int maxwidth = 0;
const char *remote_prefix = "";
struct strbuf out = STRBUF_INIT;
-   char *format;
+   char *to_free = NULL;
 
/*
 * If we are listing more than just remote branches,
@@ -364,7 +365,8 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
if (filter->verbose)
maxwidth = calc_maxwidth(, strlen(remote_prefix));
 
-   format = build_format(filter, maxwidth, remote_prefix);
+   if (!format)
+   format = to_free = build_format(filter, maxwidth, 
remote_prefix);
verify_ref_format(format);
 
/*
@@ -392,7 +394,7 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
}
 
ref_array_clear();
-   free(format);
+   free(to_free);
 }
 
 static void rename_branch(const char *oldname, const char *newname, int force)
@@ -491,6 +493,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
enum branch_track track;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = 
+   const char *format = NULL;
 
struct option options[] = {
OPT_GROUP(N_("Generic options")),
@@ -531,6 +534,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPTION_CALLBACK, 0, "points-at", _at, 
N_("object"),
N_("print only branches of the object"), 0, 
parse_opt_object_name
},
+   OPT_STRING(  0 , "format", , N_("format"), N_("format to 
use for the output")),
OPT_END(),
};
 
@@ -591,7 +595,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
filter.kind |= FILTER_REFS_DETACHED_HEAD;
filter.name_patterns = argv;
-   print_ref_list(, sorting);
+   print_ref_list(, sorting, format);
print_columns(, colopts, NULL);
string_list_clear(, 0);
return 0;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 980c732..d8edaf2 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -196,4 +196,18 @@ test_expect_success 'local-branch symrefs shortened 
properly' '
test_cmp expect actual
 '
 
+test_expect_success 'git branch --format option' '
+   cat >expect <<-\EOF &&
+   Refname is (HEAD detached from fromtag)
+   Refname is refs/heads/ambiguous
+   Refname is refs/heads/branch-one
+   Refname is refs/heads/branch-two
+   Refname 

[PATCH v5 04/16] ref-filter: move get_head_description() from branch.c

2016-04-22 Thread Karthik Nayak
Move the implementation of get_head_description() from branch.c to
ref-filter.  This gives a description of the HEAD ref if called. This
is used as the refname for the HEAD ref whenever the
FILTER_REFS_DETACHED_HEAD option is used. Make it public because we
need it to calculate the length of the HEAD refs description in
branch.c:calc_maxwidth() when we port branch.c to use ref-filter
APIs.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c | 31 ---
 ref-filter.c | 38 --
 ref-filter.h |  2 ++
 3 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0adba62..d467840 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -361,37 +361,6 @@ static void add_verbose_info(struct strbuf *out, struct 
ref_array_item *item,
strbuf_release();
 }
 
-static char *get_head_description(void)
-{
-   struct strbuf desc = STRBUF_INIT;
-   struct wt_status_state state;
-   memset(, 0, sizeof(state));
-   wt_status_get_state(, 1);
-   if (state.rebase_in_progress ||
-   state.rebase_interactive_in_progress)
-   strbuf_addf(, _("(no branch, rebasing %s)"),
-   state.branch);
-   else if (state.bisect_in_progress)
-   strbuf_addf(, _("(no branch, bisect started on %s)"),
-   state.branch);
-   else if (state.detached_from) {
-   /* TRANSLATORS: make sure these match _("HEAD detached at ")
-  and _("HEAD detached from ") in wt-status.c */
-   if (state.detached_at)
-   strbuf_addf(, _("(HEAD detached at %s)"),
-   state.detached_from);
-   else
-   strbuf_addf(, _("(HEAD detached from %s)"),
-   state.detached_from);
-   }
-   else
-   strbuf_addstr(, _("(no branch)"));
-   free(state.branch);
-   free(state.onto);
-   free(state.detached_from);
-   return strbuf_detach(, NULL);
-}
-
 static void format_and_print_ref_item(struct ref_array_item *item, int 
maxwidth,
  struct ref_filter *filter, const char 
*remote_prefix)
 {
diff --git a/ref-filter.c b/ref-filter.c
index 7d3af1c..fcb3353 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
 #include "utf8.h"
 #include "git-compat-util.h"
 #include "version.h"
+#include "wt-status.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -1077,6 +1078,37 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
*s = refname;
 }
 
+char *get_head_description(void)
+{
+   struct strbuf desc = STRBUF_INIT;
+   struct wt_status_state state;
+   memset(, 0, sizeof(state));
+   wt_status_get_state(, 1);
+   if (state.rebase_in_progress ||
+   state.rebase_interactive_in_progress)
+   strbuf_addf(, _("(no branch, rebasing %s)"),
+   state.branch);
+   else if (state.bisect_in_progress)
+   strbuf_addf(, _("(no branch, bisect started on %s)"),
+   state.branch);
+   else if (state.detached_from) {
+   /* TRANSLATORS: make sure these match _("HEAD detached at ")
+  and _("HEAD detached from ") in wt-status.c */
+   if (state.detached_at)
+   strbuf_addf(, _("(HEAD detached at %s)"),
+   state.detached_from);
+   else
+   strbuf_addf(, _("(HEAD detached from %s)"),
+   state.detached_from);
+   }
+   else
+   strbuf_addstr(, _("(no branch)"));
+   free(state.branch);
+   free(state.onto);
+   free(state.detached_from);
+   return strbuf_detach(, NULL);
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  */
@@ -1116,9 +1148,11 @@ static void populate_value(struct ref_array_item *ref)
name++;
}
 
-   if (starts_with(name, "refname"))
+   if (starts_with(name, "refname")) {
refname = ref->refname;
-   else if (starts_with(name, "symref"))
+   if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+   refname = get_head_description();
+   } else if (starts_with(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (starts_with(name, "upstream")) {
const char *branch_name;
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..4aea594 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -106,5 +106,7 @@ int 

[PATCH v5 11/16] ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()

2016-04-22 Thread Karthik Nayak
Use the recently introduced refname_atom_parser_internal() within
remote_ref_atom_parser(), this provides a common base for all the ref
printing atoms, allowing %(upstream) and %(push) to also use the
':strip' option.

The atoms '%(push)' and '%(upstream)' will retain the ':track' and
':trackshort' atom modifiers to themselves as they have no meaning in
context to the '%(refname)' and '%(symref)' atoms.

Update the documentation and tests to reflect the same.

Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt | 27 ++-
 ref-filter.c   | 26 +++---
 t/t6300-for-each-ref.sh|  2 ++
 3 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index e0c0a12..3eaf770 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -114,21 +114,22 @@ objectname::
 
 upstream::
The name of a local ref which can be considered ``upstream''
-   from the displayed ref. Respects `:short` in the same way as
-   `refname` above.  Additionally respects `:track` to show
-   "[ahead N, behind M]" and `:trackshort` to show the terse
-   version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-   or "=" (in sync). `:track` also prints "[gone]" whenever
-   unknown upstream ref is encountered. Append `:track,nobracket`
-   to show tracking information without brackets (i.e "ahead N,
-   behind M").  Has no effect if the ref does not have tracking
-   information associated with it.
+   from the displayed ref. Respects `:short` and `:strip` in the
+   same way as `refname` above.  Additionally respects `:track`
+   to show "[ahead N, behind M]" and `:trackshort` to show the
+   terse version: ">" (ahead), "<" (behind), "<>" (ahead and
+   behind), or "=" (in sync). `:track` also prints "[gone]"
+   whenever unknown upstream ref is encountered. Append
+   `:track,nobracket` to show tracking information without
+   brackets (i.e "ahead N, behind M").  Has no effect if the ref
+   does not have tracking information associated with it.
 
 push::
-   The name of a local ref which represents the `@{push}` location
-   for the displayed ref. Respects `:short`, `:track`, and
-   `:trackshort` options as `upstream` does. Produces an empty
-   string if no `@{push}` ref is configured.
+   The name of a local ref which represents the `@{push}`
+   location for the displayed ref. Respects `:short`, `:strip`,
+   `:track`, and `:trackshort` options as `upstream`
+   does. Produces an empty string if no `@{push}` ref is
+   configured.
 
 HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/ref-filter.c b/ref-filter.c
index 933f8ca..3b42aab 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -52,7 +52,8 @@ static struct used_atom {
char color[COLOR_MAXLEN];
struct align align;
struct {
-   enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT } 
option;
+   enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
+   struct refname_atom refname;
unsigned int nobracket: 1;
} remote_ref;
struct {
@@ -102,7 +103,9 @@ static void remote_ref_atom_parser(struct used_atom *atom, 
const char *arg)
int i;
 
if (!arg) {
-   atom->u.remote_ref.option = RR_NORMAL;
+   atom->u.remote_ref.option = RR_REF;
+   refname_atom_parser_internal(>u.remote_ref.refname,
+arg, atom->name);
return;
}
 
@@ -112,16 +115,17 @@ static void remote_ref_atom_parser(struct used_atom 
*atom, const char *arg)
for (i = 0; i < params.nr; i++) {
const char *s = params.items[i].string;
 
-   if (!strcmp(s, "short"))
-   atom->u.remote_ref.option = RR_SHORTEN;
-   else if (!strcmp(s, "track"))
+   if (!strcmp(s, "track"))
atom->u.remote_ref.option = RR_TRACK;
else if (!strcmp(s, "trackshort"))
atom->u.remote_ref.option = RR_TRACKSHORT;
else if (!strcmp(s, "nobracket"))
atom->u.remote_ref.nobracket = 1;
-   else
-   die(_("unrecognized format: %%(%s)"), atom->name);
+   else {
+   atom->u.remote_ref.option = RR_REF;
+   
refname_atom_parser_internal(>u.remote_ref.refname,
+arg, atom->name);
+   }
}
 
string_list_clear(, 0);
@@ -1100,8 +1104,8 @@ static void fill_remote_ref_details(struct used_atom 

[PATCH v5 02/16] ref-filter: implement %(if:equals=) and %(if:notequals=)

2016-04-22 Thread Karthik Nayak
Implement %(if:equals=) wherein the if condition is only
satisfied if the value obtained between the %(if:...) and %(then) atom
is the same as the given ''.

Similarly, implement (if:notequals=) wherein the if condition
is only satisfied if the value obtained between the %(if:...) and
%(then) atom is differnt from the given ''.

This is done by introducing 'if_atom_parser()' which parses the given
%(if) atom and then stores the data in used_atom which is later passed
on to the used_atom of the %(then) atom, so that it can do the required
comparisons.

Add tests and Documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  3 +++
 ref-filter.c   | 43 +-
 t/t6302-for-each-ref-filter.sh | 18 
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 473b6bf..3e7db10 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -152,6 +152,9 @@ if::
evaluating the string before %(then), this is useful when we
use the %(HEAD) atom which prints either "*" or " " and we
want to apply the 'if' condition only on the 'HEAD' ref.
+   Append ":equals=" or ":notequals=" to compare
+   the value between the %(if:...) and %(then) atoms with the
+   given string.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 12e646c..857a8b5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -22,6 +22,8 @@ struct align {
 };
 
 struct if_then_else {
+   const char *if_equals,
+   *not_equals;
unsigned int then_atom_seen : 1,
else_atom_seen : 1,
condition_satisfied : 1;
@@ -49,6 +51,10 @@ static struct used_atom {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, 
C_SUB } option;
unsigned int nlines;
} contents;
+   struct {
+   const char *if_equals,
+   *not_equals;
+   } if_then_else;
enum { O_FULL, O_SHORT } objectname;
} u;
 } *used_atom;
@@ -169,6 +175,19 @@ static void align_atom_parser(struct used_atom *atom, 
const char *arg)
string_list_clear(, 0);
 }
 
+static void if_atom_parser(struct used_atom *atom, const char *arg)
+{
+   if (!arg)
+   return;
+   else if (skip_prefix(arg, "equals=", >u.if_then_else.if_equals))
+;
+   else if (skip_prefix(arg, "notequals=", 
>u.if_then_else.not_equals))
+   ;
+   else
+   die(_("unrecognized %%(if) argument: %s"), arg);
+}
+
+
 static struct {
const char *name;
cmp_type cmp_type;
@@ -209,7 +228,7 @@ static struct {
{ "color", FIELD_STR, color_atom_parser },
{ "align", FIELD_STR, align_atom_parser },
{ "end" },
-   { "if" },
+   { "if", FIELD_STR, if_atom_parser },
{ "then" },
{ "else" },
 };
@@ -410,6 +429,9 @@ static void if_atom_handler(struct atom_value *atomv, 
struct ref_formatting_stat
struct ref_formatting_stack *new;
struct if_then_else *if_then_else = xcalloc(sizeof(struct 
if_then_else), 1);
 
+   if_then_else->if_equals = atomv->atom->u.if_then_else.if_equals;
+   if_then_else->not_equals = atomv->atom->u.if_then_else.not_equals;
+
push_stack_element(>stack);
new = state->stack;
new->at_end = if_then_else_handler;
@@ -441,10 +463,17 @@ static void then_atom_handler(struct atom_value *atomv, 
struct ref_formatting_st
die(_("format: %%(then) atom used after %%(else)"));
if_then_else->then_atom_seen = 1;
/*
-* If there exists non-empty string between the 'if' and
-* 'then' atom then the 'if' condition is satisfied.
+* If the 'equals' or 'notequals' attribute is used then
+* perform the required comparison. If not, only non-empty
+* strings satisfy the 'if' condition.
 */
-   if (cur->output.len && !is_empty(cur->output.buf))
+   if (if_then_else->if_equals) {
+   if (!strcmp(if_then_else->if_equals, cur->output.buf))
+   if_then_else->condition_satisfied = 1;
+   } else  if (if_then_else->not_equals) {
+   if (strcmp(if_then_else->not_equals, cur->output.buf))
+   if_then_else->condition_satisfied = 1;
+   } else if (cur->output.len && !is_empty(cur->output.buf))
if_then_else->condition_satisfied = 1;
strbuf_reset(>output);
 }
@@ -1137,7 +1166,11 @@ static void 

[PATCH v5 15/16] branch: use ref-filter printing APIs

2016-04-22 Thread Karthik Nayak
Port branch.c to use ref-filter APIs for printing. This clears out
most of the code used in branch.c for printing and replaces them with
calls made to the ref-filter library.

Introduce build_format() which gets the format required for printing
of refs. Make amendments to print_ref_list() to reflect these changes.

Change calc_maxwidth() to also account for the length of HEAD ref, by
calling ref-filter:get_head_discription().

Also change the test in t6040 to reflect the changes.

Before this patch, all cross-prefix symrefs weren't shortened. Since
we're using ref-filter APIs, we shorten all symrefs by default. We also
allow the user to change the format if needed with the introduction of
the '--format' option in the next patch.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Helped-by: Stefan Beller 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c | 234 ++-
 t/t3203-branch-output.sh |   2 +-
 t/t6040-tracking-info.sh |   2 +-
 3 files changed, 70 insertions(+), 168 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ecd7ffc..07068d2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -36,12 +36,12 @@ static unsigned char head_sha1[20];
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
-   GIT_COLOR_RESET,
-   GIT_COLOR_NORMAL,   /* PLAIN */
-   GIT_COLOR_RED,  /* REMOTE */
-   GIT_COLOR_NORMAL,   /* LOCAL */
-   GIT_COLOR_GREEN,/* CURRENT */
-   GIT_COLOR_BLUE, /* UPSTREAM */
+   "%(color:reset)",
+   "%(color:reset)",   /* PLAIN */
+   "%(color:red)", /* REMOTE */
+   "%(color:reset)",   /* LOCAL */
+   "%(color:green)",   /* CURRENT */
+   "%(color:blue)",/* UPSTREAM */
 };
 enum color_branch {
BRANCH_COLOR_RESET = 0,
@@ -277,162 +277,6 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
return(ret);
 }
 
-static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
-   int show_upstream_ref)
-{
-   int ours, theirs;
-   char *ref = NULL;
-   struct branch *branch = branch_get(branch_name);
-   const char *upstream;
-   struct strbuf fancy = STRBUF_INIT;
-   int upstream_is_gone = 0;
-   int added_decoration = 1;
-
-   if (stat_tracking_info(branch, , , ) < 0) {
-   if (!upstream)
-   return;
-   upstream_is_gone = 1;
-   }
-
-   if (show_upstream_ref) {
-   ref = shorten_unambiguous_ref(upstream, 0);
-   if (want_color(branch_use_color))
-   strbuf_addf(, "%s%s%s",
-   branch_get_color(BRANCH_COLOR_UPSTREAM),
-   ref, 
branch_get_color(BRANCH_COLOR_RESET));
-   else
-   strbuf_addstr(, ref);
-   }
-
-   if (upstream_is_gone) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: gone]"), fancy.buf);
-   else
-   added_decoration = 0;
-   } else if (!ours && !theirs) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s]"), fancy.buf);
-   else
-   added_decoration = 0;
-   } else if (!ours) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf, 
theirs);
-   else
-   strbuf_addf(stat, _("[behind %d]"), theirs);
-
-   } else if (!theirs) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: ahead %d]"), fancy.buf, ours);
-   else
-   strbuf_addf(stat, _("[ahead %d]"), ours);
-   } else {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: ahead %d, behind %d]"),
-   fancy.buf, ours, theirs);
-   else
-   strbuf_addf(stat, _("[ahead %d, behind %d]"),
-   ours, theirs);
-   }
-   strbuf_release();
-   if (added_decoration)
-   strbuf_addch(stat, ' ');
-   free(ref);
-}
-
-static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
-struct ref_filter *filter, const char *refname)
-{
-   struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
-   const char *sub = _("  invalid ref ");
-   struct commit *commit = item->commit;
-
-   if (!parse_commit(commit)) {
-   pp_commit_easy(CMIT_FMT_ONELINE, commit, );
-   sub = subject.buf;
-   }
-
-   if (item->kind == FILTER_REFS_BRANCHES)
-   

[PATCH v5 06/16] ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams

2016-04-22 Thread Karthik Nayak
Borrowing from branch.c's implementation print "[gone]" whenever an
unknown upstream ref is encountered instead of just ignoring it.

This makes sure that when branch.c is ported over to using ref-filter
APIs for printing, this feature is not lost.

Make changes to t/t6300-for-each-ref.sh and
Documentation/git-for-each-ref.txt to reflect this change.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Helped-by : Jacob Keller 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt | 3 ++-
 ref-filter.c   | 4 +++-
 t/t6300-for-each-ref.sh| 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 5370fe5..f1e46a7 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -119,7 +119,8 @@ upstream::
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
or "=" (in sync).  Has no effect if the ref does not have
-   tracking information associated with it.
+   tracking information associated with it. `:track` also prints
+   "[gone]" whenever unknown upstream ref is encountered.
 
 push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index da2b9ee..3bb6923 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1049,8 +1049,10 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
else if (atom->u.remote_ref == RR_TRACK) {
if (stat_tracking_info(branch, _ours,
-  _theirs, NULL))
+  _theirs, NULL)) {
+   *s = "[gone]";
return;
+   }
 
if (!num_ours && !num_theirs)
*s = "";
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2be0a3f..a92b36f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -382,7 +382,7 @@ test_expect_success 'Check that :track[short] cannot be 
used with other atoms' '
 
 test_expect_success 'Check that :track[short] works when upstream is invalid' '
cat >expected <<-\EOF &&
-
+   [gone]
 
EOF
test_when_finished "git config branch.master.merge refs/heads/master" &&
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 03/16] ref-filter: modify "%(objectname:short)" to take length

2016-04-22 Thread Karthik Nayak
Add support for %(objectname:short=) which would print the
abbreviated unique objectname of given length. When no length is
specified, the length is 'DEFAULT_ABBREV'. The minimum length is
'MINIMUM_ABBREV'. The length may be exceeded to ensure that the provided
object name is unique.

Add tests and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Helped-by: Jacob Keller 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  4 
 ref-filter.c   | 25 +++--
 t/t6300-for-each-ref.sh| 10 ++
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 3e7db10..5370fe5 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -107,6 +107,10 @@ objectsize::
 objectname::
The object name (aka SHA-1).
For a non-ambiguous abbreviation of the object name append `:short`.
+   For an abbreviation of the object name with desired length append
+   `:short=`, where the minimum length is MINIMUM_ABBREV. The
+   length may be exceeded to ensure unique object names.
+
 
 upstream::
The name of a local ref which can be considered ``upstream''
diff --git a/ref-filter.c b/ref-filter.c
index 857a8b5..7d3af1c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,7 +55,10 @@ static struct used_atom {
const char *if_equals,
*not_equals;
} if_then_else;
-   enum { O_FULL, O_SHORT } objectname;
+   struct {
+   enum { O_FULL, O_LENGTH, O_SHORT } option;
+   unsigned int length;
+   } objectname;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -118,10 +121,17 @@ static void contents_atom_parser(struct used_atom *atom, 
const char *arg)
 static void objectname_atom_parser(struct used_atom *atom, const char *arg)
 {
if (!arg)
-   atom->u.objectname = O_FULL;
+   atom->u.objectname.option = O_FULL;
else if (!strcmp(arg, "short"))
-   atom->u.objectname = O_SHORT;
-   else
+   atom->u.objectname.option = O_SHORT;
+   else if (skip_prefix(arg, "short=", )) {
+   atom->u.objectname.option = O_LENGTH;
+   if (strtoul_ui(arg, 10, >u.objectname.length) ||
+   atom->u.objectname.length == 0)
+   die(_("positive value expected objectname:short=%s"), 
arg);
+   if (atom->u.objectname.length < MINIMUM_ABBREV)
+   atom->u.objectname.length = MINIMUM_ABBREV;
+   } else
die(_("unrecognized %%(objectname) argument: %s"), arg);
 }
 
@@ -591,12 +601,15 @@ static int grab_objectname(const char *name, const 
unsigned char *sha1,
   struct atom_value *v, struct used_atom *atom)
 {
if (starts_with(name, "objectname")) {
-   if (atom->u.objectname == O_SHORT) {
+   if (atom->u.objectname.option == O_SHORT) {
v->s = xstrdup(find_unique_abbrev(sha1, 
DEFAULT_ABBREV));
return 1;
-   } else if (atom->u.objectname == O_FULL) {
+   } else if (atom->u.objectname.option == O_FULL) {
v->s = xstrdup(sha1_to_hex(sha1));
return 1;
+   } else if (atom->u.objectname.option == O_LENGTH) {
+   v->s = xstrdup(find_unique_abbrev(sha1, 
atom->u.objectname.length));
+   return 1;
} else
die("BUG: unknown %%(objectname) option");
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 19a2823..2be0a3f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -60,6 +60,8 @@ test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
+test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
+test_atom head objectname:short=10 $(git rev-parse --short=10 
refs/heads/master)
 test_atom head tree $(git rev-parse refs/heads/master^{tree})
 test_atom head parent ''
 test_atom head numparent 0
@@ -99,6 +101,8 @@ test_atom tag objecttype tag
 test_atom tag objectsize 154
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
+test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
+test_atom head objectname:short=10 $(git rev-parse --short=10 
refs/heads/master)
 test_atom tag tree ''
 test_atom tag 

[PATCH v5 07/16] ref-filter: add support for %(upstream:track,nobracket)

2016-04-22 Thread Karthik Nayak
Add support for %(upstream:track,nobracket) which will print the
tracking information without the brackets (i.e. "ahead N, behind M").
This is needed when we port branch.c to use ref-filter's printing APIs.

Add test and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  8 +++--
 ref-filter.c   | 67 +-
 t/t6300-for-each-ref.sh|  2 ++
 3 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index f1e46a7..5c85f27 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -118,9 +118,11 @@ upstream::
`refname` above.  Additionally respects `:track` to show
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-   or "=" (in sync).  Has no effect if the ref does not have
-   tracking information associated with it. `:track` also prints
-   "[gone]" whenever unknown upstream ref is encountered.
+   or "=" (in sync). `:track` also prints "[gone]" whenever
+   unknown upstream ref is encountered. Append `:track,nobracket`
+   to show tracking information without brackets (i.e "ahead N,
+   behind M").  Has no effect if the ref does not have tracking
+   information associated with it.
 
 push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index 3bb6923..b898bcd 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -46,8 +46,10 @@ static struct used_atom {
union {
char color[COLOR_MAXLEN];
struct align align;
-   enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
-   remote_ref;
+   struct {
+   enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT } 
option;
+   unsigned int nobracket: 1;
+   } remote_ref;
struct {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, 
C_SUB } option;
unsigned int nlines;
@@ -75,16 +77,33 @@ static void color_atom_parser(struct used_atom *atom, const 
char *color_value)
 
 static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 {
-   if (!arg)
-   atom->u.remote_ref = RR_NORMAL;
-   else if (!strcmp(arg, "short"))
-   atom->u.remote_ref = RR_SHORTEN;
-   else if (!strcmp(arg, "track"))
-   atom->u.remote_ref = RR_TRACK;
-   else if (!strcmp(arg, "trackshort"))
-   atom->u.remote_ref = RR_TRACKSHORT;
-   else
-   die(_("unrecognized format: %%(%s)"), atom->name);
+   struct string_list params = STRING_LIST_INIT_DUP;
+   int i;
+
+   if (!arg) {
+   atom->u.remote_ref.option = RR_NORMAL;
+   return;
+   }
+
+   atom->u.remote_ref.nobracket = 0;
+   string_list_split(, arg, ',', -1);
+
+   for (i = 0; i < params.nr; i++) {
+   const char *s = params.items[i].string;
+
+   if (!strcmp(s, "short"))
+   atom->u.remote_ref.option = RR_SHORTEN;
+   else if (!strcmp(s, "track"))
+   atom->u.remote_ref.option = RR_TRACK;
+   else if (!strcmp(s, "trackshort"))
+   atom->u.remote_ref.option = RR_TRACKSHORT;
+   else if (!strcmp(s, "nobracket"))
+   atom->u.remote_ref.nobracket = 1;
+   else
+   die(_("unrecognized format: %%(%s)"), atom->name);
+   }
+
+   string_list_clear(, 0);
 }
 
 static void body_atom_parser(struct used_atom *atom, const char *arg)
@@ -1045,25 +1064,27 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
struct branch *branch, const char **s)
 {
int num_ours, num_theirs;
-   if (atom->u.remote_ref == RR_SHORTEN)
+   if (atom->u.remote_ref.option == RR_SHORTEN)
*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
-   else if (atom->u.remote_ref == RR_TRACK) {
+   else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, _ours,
   _theirs, NULL)) {
-   *s = "[gone]";
-   return;
-   }
-
-   if (!num_ours && !num_theirs)
+   *s = xstrdup("gone");
+   } else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
-   *s = xstrfmt("[behind %d]", num_theirs);
+ 

[PATCH v5 13/16] ref-filter: allow porcelain to translate messages in the output

2016-04-22 Thread Karthik Nayak
Introduce setup_ref_filter_porcelain_msg() so that the messages used in
the atom %(upstream:track) can be translated if needed. This is needed
as we port branch.c to use ref-filter's printing API's.

Written-by: Matthieu Moy 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 28 
 ref-filter.h |  2 ++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 97977a5..74c4869 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -15,6 +15,26 @@
 #include "version.h"
 #include "wt-status.h"
 
+static struct ref_msg {
+   const char *gone;
+   const char *ahead;
+   const char *behind;
+   const char *ahead_behind;
+} msgs = {
+   "gone",
+   "ahead %d",
+   "behind %d",
+   "ahead %d, behind %d"
+};
+
+void setup_ref_filter_porcelain_msg(void)
+{
+   msgs.gone = _("gone");
+   msgs.ahead = _("ahead %d");
+   msgs.behind = _("behind %d");
+   msgs.ahead_behind = _("ahead %d, behind %d");
+}
+
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
 struct align {
@@ -1130,15 +1150,15 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, _ours,
   _theirs, NULL)) {
-   *s = xstrdup("gone");
+   *s = xstrdup(msgs.gone);
} else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
-   *s = xstrfmt("behind %d", num_theirs);
+   *s = xstrfmt(msgs.behind, num_theirs);
else if (!num_theirs)
-   *s = xstrfmt("ahead %d", num_ours);
+   *s = xstrfmt(msgs.ahead, num_ours);
else
-   *s = xstrfmt("ahead %d, behind %d",
+   *s = xstrfmt(msgs.ahead_behind,
 num_ours, num_theirs);
if (!atom->u.remote_ref.nobracket && *s[0]) {
const char *to_free = *s;
diff --git a/ref-filter.h b/ref-filter.h
index 0014b92..da17145 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -111,5 +111,7 @@ struct ref_sorting *ref_default_sorting(void);
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 /*  Get the current HEAD's description */
 char *get_head_description(void);
+/*  Set up translated strings in the output. */
+void setup_ref_filter_porcelain_msg(void);
 
 #endif /*  REF_FILTER_H  */
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 08/16] ref-filter: make "%(symref)" atom work with the ':short' modifier

2016-04-22 Thread Karthik Nayak
The "%(symref)" atom doesn't work when used with the ':short' modifier
because we strictly match only 'symref' for setting the 'need_symref'
indicator. Fix this by using comparing with valid_atom rather than used_atom.

Add tests for %(symref) and %(symref:short) while we're here.

Helped-by: Junio C Hamano 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c|  2 +-
 t/t6300-for-each-ref.sh | 24 
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index b898bcd..083cc27 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -338,7 +338,7 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
valid_atom[i].parser(_atom[at], arg);
if (*atom == '*')
need_tagged = 1;
-   if (!strcmp(used_atom[at].name, "symref"))
+   if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
return at;
 }
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2c5f177..b06ea1c 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -38,6 +38,7 @@ test_atom() {
case "$1" in
head) ref=refs/heads/master ;;
 tag) ref=refs/tags/testtag ;;
+sym) ref=refs/heads/sym ;;
   *) ref=$1 ;;
esac
printf '%s\n' "$3" >expected
@@ -565,4 +566,27 @@ test_expect_success 'Verify sort with multiple keys' '
refs/tags/bogo refs/tags/master > actual &&
test_cmp expected actual
 '
+
+test_expect_success 'Add symbolic ref for the following tests' '
+   git symbolic-ref refs/heads/sym refs/heads/master
+'
+
+cat >expected < actual &&
+   test_cmp expected actual
+'
+
+cat >expected < actual &&
+   test_cmp expected actual
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 00/16] port branch.c to use ref-filter's printing options

2016-04-22 Thread Karthik Nayak
This is part of unification of the commands 'git tag -l, git branch -l
and git for-each-ref'. This ports over branch.c to use ref-filter's
printing options.

Initially posted here: $(gmane/279226). It was decided that this series
would follow up after refactoring ref-filter parsing mechanism, which
is now merged into master (9606218b32344c5c756f7c29349d3845ef60b80c).

v1 can be found here: $(gmane/288342)
v2 can be found here: $(gmane/288863)
v3 can be found here: $(gmane/290299)
v4 can be found here: $(gmane/291106)

Changes in this version:
1. Rebased on top of jk/branch-shortening-funny-symrefs ($gmane/291295).
2. Introduced refname_atom_parser_internal() to act as a common ground
for ref-printing atoms. Written by Jeff ($gmane/291497).
3. Ensure that ':dir', 'base', 'strip' is available to all ref-printing
atoms. Written by Jeff.
4. Fix a memory leak which was reported by Stefan ($gmane/291703).

Thanks to Junio, Jeff and Stefan for suggestions on the previous version.

Karthik Nayak (16):
  ref-filter: include reference to 'used_atom' within 'atom_value'
  ref-filter: implement %(if:equals=) and
%(if:notequals=)
  ref-filter: modify "%(objectname:short)" to take length
  ref-filter: move get_head_description() from branch.c
  ref-filter: introduce format_ref_array_item()
  ref-filter: make %(upstream:track) prints "[gone]" for invalid
upstreams
  ref-filter: add support for %(upstream:track,nobracket)
  ref-filter: make "%(symref)" atom work with the ':short' modifier
  ref-filter: introduce refname_atom_parser_internal()
  ref-filter: introduce symref_atom_parser() and refname_atom_parser()
  ref-filter: make remote_ref_atom_parser() use
refname_atom_parser_internal()
  ref-filter: add `:dir` and `:base` options for ref printing atoms
  ref-filter: allow porcelain to translate messages in the output
  branch, tag: use porcelain output
  branch: use ref-filter printing APIs
  branch: implement '--format' option

 Documentation/git-branch.txt   |   7 +-
 Documentation/git-for-each-ref.txt |  42 +++--
 builtin/branch.c   | 275 +-
 builtin/tag.c  |   2 +
 ref-filter.c   | 333 +
 ref-filter.h   |   7 +
 t/t3203-branch-output.sh   |  16 +-
 t/t6040-tracking-info.sh   |   2 +-
 t/t6300-for-each-ref.sh|  73 +++-
 t/t6302-for-each-ref-filter.sh |  18 ++
 10 files changed, 488 insertions(+), 287 deletions(-)

Interdiff:

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 0954269..4a5e9ea 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -116,21 +116,23 @@ objectname::
 
 upstream::
The name of a local ref which can be considered ``upstream''
-   from the displayed ref. Respects `:short` in the same way as
-   `refname` above.  Additionally respects `:track` to show
-   "[ahead N, behind M]" and `:trackshort` to show the terse
-   version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-   or "=" (in sync). `:track` also prints "[gone]" whenever
-   unknown upstream ref is encountered. Append `:track,nobracket`
-   to show tracking information without brackets (i.e "ahead N,
-   behind M").  Has no effect if the ref does not have tracking
-   information associated with it.
+   from the displayed ref. Respects `:short`, `:strip`, `:base`
+   and `:dir` in the same way as `refname` above.  Additionally
+   respects `:track` to show "[ahead N, behind M]" and
+   `:trackshort` to show the terse version: ">" (ahead), "<"
+   (behind), "<>" (ahead and behind), or "=" (in sync). `:track`
+   also prints "[gone]" whenever unknown upstream ref is
+   encountered. Append `:track,nobracket` to show tracking
+   information without brackets (i.e "ahead N, behind M").  Has
+   no effect if the ref does not have tracking information
+   associated with it.
 
 push::
-   The name of a local ref which represents the `@{push}` location
-   for the displayed ref. Respects `:short`, `:track`, and
-   `:trackshort` options as `upstream` does. Produces an empty
-   string if no `@{push}` ref is configured.
+   The name of a local ref which represents the `@{push}`
+   location for the displayed ref. Respects `:short`, `:strip`,
+   `:track`, `:trackshort`, `:base` and `:dir` options as
+   `upstream` does. Produces an empty string if no `@{push}` ref
+   is configured.
 
 HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
@@ -165,6 +167,12 @@ if::
the value between the %(if:...) and %(then) atoms with the
given string.
 
+symref::
+   The ref which the given symbolic ref refers to. If not a
+   symbolic ref, nothing is printed. Respects the `:short`,
+   `:strip`, `:base` and `:dir` 

[PATCH v5 09/16] ref-filter: introduce refname_atom_parser_internal()

2016-04-22 Thread Karthik Nayak
Since there are multiple atoms which print refs ('%(refname)',
'%(symref)', '%(push)', '%upstream'), it makes sense to have a common
ground for parsing them. This would allow us to share implementations of
the atom modifiers between these atoms.

Introduce refname_atom_parser_internal() to act as a common parsing
function for ref printing atoms. This would eventually be used to
introduce refname_atom_parser() and symref_atom_parser() and also be
internally used in remote_ref_atom_parser().

Helped-by: Jeff King 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index 083cc27..778fe02 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -30,6 +30,11 @@ struct if_then_else {
condition_satisfied : 1;
 };
 
+struct refname_atom {
+   enum { R_NORMAL, R_SHORT, R_STRIP } option;
+   unsigned int strip;
+};
+
 /*
  * An atom is a valid field atom listed below, possibly prefixed with
  * a "*" to denote deref_tag().
@@ -62,6 +67,7 @@ static struct used_atom {
enum { O_FULL, O_LENGTH, O_SHORT } option;
unsigned int length;
} objectname;
+   struct refname_atom refname;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -75,6 +81,21 @@ static void color_atom_parser(struct used_atom *atom, const 
char *color_value)
die(_("unrecognized color: %%(color:%s)"), color_value);
 }
 
+static void refname_atom_parser_internal(struct refname_atom *atom,
+const char *arg, const char *name)
+{
+   if (!arg)
+   atom->option = R_NORMAL;
+   else if (!strcmp(arg, "short"))
+   atom->option = R_SHORT;
+   else if (skip_prefix(arg, "strip=", )) {
+   atom->option = R_STRIP;
+   if (strtoul_ui(arg, 10, >strip) || atom->strip <= 0)
+   die(_("positive value expected refname:strip=%s"), arg);
+   }   else
+   die(_("unrecognized %%(%s) argument: %s"), name, arg);
+}
+
 static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 {
struct string_list params = STRING_LIST_INIT_DUP;
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 10/16] ref-filter: introduce symref_atom_parser() and refname_atom_parser()

2016-04-22 Thread Karthik Nayak
Using refname_atom_parser_internal(), introduce symref_atom_parser() and
refname_atom_parser() which will parse the atoms %(symref) and
%(refname) respectively. Store the parsed information into the
'used_atom' structure based on the modifiers used along with the atoms.

Now the '%(symref)' atom supports the ':strip' atom modifier. Update the
Documentation and tests to reflect this.

Helped-by: Jeff King 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  5 +++
 ref-filter.c   | 78 ++
 t/t6300-for-each-ref.sh|  9 +
 3 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 5c85f27..e0c0a12 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -163,6 +163,11 @@ if::
the value between the %(if:...) and %(then) atoms with the
given string.
 
+symref::
+   The ref which the given symbolic ref refers to. If not a
+   symbolic ref, nothing is printed. Respects the `:short` and
+   `:strip` options in the same way as `refname` above.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index 778fe02..933f8ca 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -176,6 +176,16 @@ static void objectname_atom_parser(struct used_atom *atom, 
const char *arg)
die(_("unrecognized %%(objectname) argument: %s"), arg);
 }
 
+static void symref_atom_parser(struct used_atom *atom, const char *arg)
+{
+   return refname_atom_parser_internal(>u.refname, arg, atom->name);
+}
+
+static void refname_atom_parser(struct used_atom *atom, const char *arg)
+{
+   return refname_atom_parser_internal(>u.refname, arg, atom->name);
+}
+
 static align_type parse_align_position(const char *s)
 {
if (!strcmp(s, "right"))
@@ -244,7 +254,7 @@ static struct {
cmp_type cmp_type;
void (*parser)(struct used_atom *atom, const char *arg);
 } valid_atom[] = {
-   { "refname" },
+   { "refname" , FIELD_STR, refname_atom_parser },
{ "objecttype" },
{ "objectsize", FIELD_ULONG },
{ "objectname", FIELD_STR, objectname_atom_parser },
@@ -273,7 +283,7 @@ static struct {
{ "contents", FIELD_STR, contents_atom_parser },
{ "upstream", FIELD_STR, remote_ref_atom_parser },
{ "push", FIELD_STR, remote_ref_atom_parser },
-   { "symref" },
+   { "symref", FIELD_STR, symref_atom_parser },
{ "flag" },
{ "HEAD" },
{ "color", FIELD_STR, color_atom_parser },
@@ -1058,21 +1068,16 @@ static inline char *copy_advance(char *dst, const char 
*src)
return dst;
 }
 
-static const char *strip_ref_components(const char *refname, const char 
*nr_arg)
+static const char *strip_ref_components(const char *refname, unsigned int len)
 {
-   char *end;
-   long nr = strtol(nr_arg, , 10);
-   long remaining = nr;
+   long remaining = len;
const char *start = refname;
 
-   if (nr < 1 || *end != '\0')
-   die(_(":strip= requires a positive integer argument"));
-
while (remaining) {
switch (*start++) {
case '\0':
-   die(_("ref '%s' does not have %ld components to 
:strip"),
-   refname, nr);
+   die(_("ref '%s' does not have %ud components to 
:strip"),
+   refname, len);
case '/':
remaining--;
break;
@@ -1081,6 +1086,16 @@ static const char *strip_ref_components(const char 
*refname, const char *nr_arg)
return start;
 }
 
+static const char *show_ref(struct refname_atom *atom, const char *refname)
+{
+   if (atom->option == R_SHORT)
+   return shorten_unambiguous_ref(refname, warn_ambiguous_refs);
+   else if (atom->option == R_STRIP)
+   return strip_ref_components(refname, atom->strip);
+   else
+   return refname;
+}
+
 static void fill_remote_ref_details(struct used_atom *atom, const char 
*refname,
struct branch *branch, const char **s)
 {
@@ -1153,6 +1168,21 @@ char *get_head_description(void)
return strbuf_detach(, NULL);
 }
 
+static const char *get_symref(struct used_atom *atom, struct ref_array_item 
*ref)
+{
+   if (!ref->symref)
+   return "";
+   else
+   return show_ref(>u.refname, ref->symref);
+}
+
+static const char *get_refname(struct used_atom *atom, struct ref_array_item 
*ref)
+{
+   if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+   return get_head_description();
+   return show_ref(>u.refname, 

[PATCH v5 05/16] ref-filter: introduce format_ref_array_item()

2016-04-22 Thread Karthik Nayak
To allow column display, we will need to first render the output in a
string list to allow print_columns() to compute the proper size of
each column before starting the actual output. Introduce the function
format_ref_array_item() that does the formatting of a ref_array_item
to an strbuf.

show_ref_array_item() is kept as a convenience wrapper around it which
obtains the strbuf and prints it the standard output.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 16 
 ref-filter.h |  3 +++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index fcb3353..da2b9ee 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1813,10 +1813,10 @@ static void append_literal(const char *cp, const char 
*ep, struct ref_formatting
}
 }
 
-void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+  int quote_style, struct strbuf *final_buf)
 {
const char *cp, *sp, *ep;
-   struct strbuf *final_buf;
struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
 
state.quote_style = quote_style;
@@ -1846,9 +1846,17 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
}
if (state.stack->prev)
die(_("format: %%(end) atom missing"));
-   final_buf = >output;
-   fwrite(final_buf->buf, 1, final_buf->len, stdout);
+   strbuf_addbuf(final_buf, >output);
pop_stack_element();
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
+{
+   struct strbuf final_buf = STRBUF_INIT;
+
+   format_ref_array_item(info, format, quote_style, _buf);
+   fwrite(final_buf.buf, 1, final_buf.len, stdout);
+   strbuf_release(_buf);
putchar('\n');
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index 4aea594..0014b92 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -98,6 +98,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep);
 int verify_ref_format(const char *format);
 /*  Sort the given ref_array as per the ref_sorting provided */
 void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
+/*  Based on the given format and quote_style, fill the strbuf */
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+  int quote_style, struct strbuf *final_buf);
 /*  Print the ref using the given format and quote_style */
 void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style);
 /*  Callback function for parsing the sort option */
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 12/16] ref-filter: add `:dir` and `:base` options for ref printing atoms

2016-04-22 Thread Karthik Nayak
Add the options `:dir` and `:base` to all ref printing ('%(refname)',
'%(symref)', '%(push)' and '%(upstream)') atoms. The `:dir` option gives
the directory (the part after $GIT_DIR/) of the ref without the
refname. The `:base` option gives the base directory of the given
ref (i.e. the directory following $GIT_DIR/refs/).

Add tests and documentation for the same.

Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt | 34 +++---
 ref-filter.c   | 29 +
 t/t6300-for-each-ref.sh| 24 
 3 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 3eaf770..4a5e9ea 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -96,7 +96,9 @@ refname::
slash-separated path components from the front of the refname
(e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
`` must be a positive integer.  If a displayed ref has fewer
-   components than ``, the command aborts with an error.
+   components than ``, the command aborts with an error. For the base
+   directory of the ref (i.e. foo in refs/foo/bar/boz) append
+   `:base`. For the entire directory path append `:dir`.
 
 objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
@@ -114,22 +116,23 @@ objectname::
 
 upstream::
The name of a local ref which can be considered ``upstream''
-   from the displayed ref. Respects `:short` and `:strip` in the
-   same way as `refname` above.  Additionally respects `:track`
-   to show "[ahead N, behind M]" and `:trackshort` to show the
-   terse version: ">" (ahead), "<" (behind), "<>" (ahead and
-   behind), or "=" (in sync). `:track` also prints "[gone]"
-   whenever unknown upstream ref is encountered. Append
-   `:track,nobracket` to show tracking information without
-   brackets (i.e "ahead N, behind M").  Has no effect if the ref
-   does not have tracking information associated with it.
+   from the displayed ref. Respects `:short`, `:strip`, `:base`
+   and `:dir` in the same way as `refname` above.  Additionally
+   respects `:track` to show "[ahead N, behind M]" and
+   `:trackshort` to show the terse version: ">" (ahead), "<"
+   (behind), "<>" (ahead and behind), or "=" (in sync). `:track`
+   also prints "[gone]" whenever unknown upstream ref is
+   encountered. Append `:track,nobracket` to show tracking
+   information without brackets (i.e "ahead N, behind M").  Has
+   no effect if the ref does not have tracking information
+   associated with it.
 
 push::
The name of a local ref which represents the `@{push}`
location for the displayed ref. Respects `:short`, `:strip`,
-   `:track`, and `:trackshort` options as `upstream`
-   does. Produces an empty string if no `@{push}` ref is
-   configured.
+   `:track`, `:trackshort`, `:base` and `:dir` options as
+   `upstream` does. Produces an empty string if no `@{push}` ref
+   is configured.
 
 HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
@@ -166,8 +169,9 @@ if::
 
 symref::
The ref which the given symbolic ref refers to. If not a
-   symbolic ref, nothing is printed. Respects the `:short` and
-   `:strip` options in the same way as `refname` above.
+   symbolic ref, nothing is printed. Respects the `:short`,
+   `:strip`, `:base` and `:dir` options in the same way as
+   `refname` above.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 3b42aab..97977a5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -31,7 +31,7 @@ struct if_then_else {
 };
 
 struct refname_atom {
-   enum { R_NORMAL, R_SHORT, R_STRIP } option;
+   enum { R_BASE, R_DIR, R_NORMAL, R_SHORT, R_STRIP } option;
unsigned int strip;
 };
 
@@ -93,7 +93,11 @@ static void refname_atom_parser_internal(struct refname_atom 
*atom,
atom->option = R_STRIP;
if (strtoul_ui(arg, 10, >strip) || atom->strip <= 0)
die(_("positive value expected refname:strip=%s"), arg);
-   }   else
+   } else if (!strcmp(arg, "dir"))
+   atom->option = R_DIR;
+   else if (!strcmp(arg, "base"))
+   atom->option = R_BASE;
+   else
die(_("unrecognized %%(%s) argument: %s"), name, arg);
 }
 
@@ -252,7 +256,6 @@ static void if_atom_parser(struct used_atom *atom, const 
char *arg)
die(_("unrecognized %%(if) argument: %s"), arg);
 }
 
-
 static struct {
const char *name;
cmp_type cmp_type;
@@ -1096,7 +1099,25 @@ static const char 

[PATCH v5 01/16] ref-filter: include reference to 'used_atom' within 'atom_value'

2016-04-22 Thread Karthik Nayak
Ensure that each 'atom_value' has a reference to its corresponding
'used_atom'. This let's us use values within 'used_atom' in the
'handler' function.

Hence we can get the %(align) atom's parameters directly from the
'used_atom' therefore removing the necessity of passing %(align) atom's
parameters to 'atom_value'.

This also acts as a preparatory patch for the upcoming patch where we
introduce %(if:equals=) and %(if:notequals=).

Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 41e73f0..12e646c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -230,11 +230,9 @@ struct ref_formatting_state {
 
 struct atom_value {
const char *s;
-   union {
-   struct align align;
-   } u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state 
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
+   struct used_atom *atom;
 };
 
 /*
@@ -370,7 +368,7 @@ static void align_atom_handler(struct atom_value *atomv, 
struct ref_formatting_s
push_stack_element(>stack);
new = state->stack;
new->at_end = end_align_handler;
-   new->at_end_data = >u.align;
+   new->at_end_data = >atom->u.align;
 }
 
 static void if_then_else_handler(struct ref_formatting_stack **stack)
@@ -1069,6 +1067,7 @@ static void populate_value(struct ref_array_item *ref)
struct branch *branch = NULL;
 
v->handler = append_atom;
+   v->atom = atom;
 
if (*name == '*') {
deref = 1;
@@ -1133,7 +1132,6 @@ static void populate_value(struct ref_array_item *ref)
v->s = " ";
continue;
} else if (starts_with(name, "align")) {
-   v->u.align = atom->u.align;
v->handler = align_atom_handler;
continue;
} else if (!strcmp(name, "end")) {
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: possible bug of git stash deleting uncommitted files in corner case

2016-04-22 Thread Remi Galan Alfonso
Junio C Hamano  writes:
> Remi Galan Alfonso 
> writes:
> 
> > Daniele Segato  wrote:
> > ...
> >> git version 1.9.1
> >
> > Contrary to what I expected, this seems to still be the case with:
> >   $ git --version
> >   git version 2.8.0.rc2
> 
> I do not think "git stash" has been updated in any major way to
> address correctness (including its corner case behaviour) ever since
> it was originally written, so it is very likely that any bug you see
> would be with it since the very old days.

For this bug it doesn't seem to be specifically linked to git stash,
since 'git status' doesn't display correct informations in the first
place (it doesn't show foo/bar as an untracked file).

I tried something quickly, based on Daniele's case:
git init
echo 'X' >foo
git add foo
git commit -m "Added foo"
rm foo
mkdir foo
echo 'B' >foo/bar

git status # foo/bar not shown in Untracked files

git add foo/bar

git status then shows as expected:
# On branch master
# Changes to be committed:
#   (use "git reset HEAD ..." to unstage)
# 
#   deleted:foo
#   new file:   foo/bar

However git stash fails this time:
# error: foo: is a directory - add individual files instead
# fatal: Unable to process path foo
# Cannot save the current worktree state

I am not sure what should be the correct behaviour here.
This however might be one of the corner cases you mentionned.

Thanks,
Rémi
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] name-rev: include taggerdate in considering the best name

2016-04-22 Thread Linus Torvalds
On Fri, Apr 22, 2016 at 11:11 AM, Jeff King  wrote:
>
> I confirmed that it does find the "optimal" tag for the case we've been
> discussing.

Yes. I'm a bit more worried about the date behavior for projects that
merge back stable branches into their development trees (is the
development tag better than the stable tag? the date doesn't really
say much), but I think this is still the simplest model we can use
without trying to really do a topo-sort. And in many ways it's the
simplest one to explain to people too: "we try to use the oldest
reference we can find as a base for the resulting name" is not a
complex or hard concept to explain.

> We could _also_ tweak the merge-weight as Linus's patch did, just
> because 1 has more basis than 65535. But I think it really matters a
> lot less at this point.

Yes. I still think that my tweak makes more sense than the existing
code, but it's a tiny tweak, compared to the date-based approach.
Unlikely to ever matter much.

Linus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] name-rev: include taggerdate in considering the best name

2016-04-22 Thread Junio C Hamano
Junio C Hamano  writes:

> Jeff King  writes:
>
>> That turned out to be quite simple (I wasn't sure originally if we'd
>> actually visit all of the tags, which is why I had conceived of this as
>> an initial pass; but of course it makes sense that we'd have to see all
>> of the tags in the existing code).
>> ...
>> We could _also_ tweak the merge-weight as Linus's patch did, just
>> because 1 has more basis than 65535. But I think it really matters a
>> lot less at this point.
>
> I agree, but if we were to go this route of keeping track of "some"
> attribute of the tip the traversal started from, I wonder if it is
> better to keep the actual tag object, not just its tagger date as an
> unsigned long, in the new field.

Actually, I take it back.  The "object" approach would not give us
enough flexibility to go beyond "date".  A light-weight tag that
directly point at a commit object can still yield "date" (probably
"committerdate" to be compared with other dates, be it the committer
date from another commit or the tagger date from a real tag), but if
we later wanted to do a v:refname kind of comparison, we'd need to
keep the name of the ref (we cannot go back from the commit object
to the refname), so at that point, we would be talking about adding
yet another field anyway to hold the refname, in addition to the
field we would be adding at this step.  As we do not want to be
always doing "name to object to date" conversion in this codepath,
adding an "unsigned long" date field is the right thing to do here.
A more elaborate future can add refname (or refname and object) as
additional fields, but we can wait because even after that update
the codepath to do date comparison likely would want to have direct
access to the date field anyway.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Subtree Split Includes Commits Outside Prefix Directory

2016-04-22 Thread ELI
I attempt a subtree push to a sub-project which I knew not to have had
any local modification since the last subtree push it had received,
but it failed.

To subproject
 ! [rejected]5a9ad640651d3d54387afa5b7eaf89ed0b392a01 ->
master (non-fast-forward)
error: failed to push some refs to 'subproject'
hint: Updates were rejected because a pushed branch tip is behind its remote
hint: counterpart. Check out this branch and integrate the remote changes
hint: (e.g. 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.


The first step I took to understand the cause was to do a subtree
split and inspect the commit history of the resulting branch.  I found
that it contained commits that did not touch any files in the
subproject.  Doing a git show on these commits revealed that they were
not empty commits, but contained diff information for paths in the
main project, and in some cases, other subprojects that exist in the
main project.


I then reviewed the commit history of contrib/subtree/git-subtree.sh
and determined that the last successful subtree push was performed
prior to the integration of this change:
https://git.kernel.org/cgit/git/git.git/commit/contrib/subtree/git-subtree.sh?id=933cfeb90b5d03b4096db6d60494a6eedea25d03

As a next step, I reversed that patch on my local install of git
subtree, and the result was a successful subtree push.


Unfortunately, I have not yet reproduced this with a test main project
and subprojects, and I cannot make the project I observed it in
public.


- Harpreet "Eli" Sangha
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

2016-04-22 Thread Jeff King
On Fri, Apr 22, 2016 at 10:22:42AM -0700, Junio C Hamano wrote:

> >> * jk/push-client-deadlock-fix (2016-04-20) 5 commits
> [...]
> Thanks.  Something like this, perhaps?
> 
>   "git push" from a corrupt repository that attempts to push a large
>   number of refs deadlocked; the thread to relay rejection notices
>   for these ref updates blocked on writing them to the main thread,
>   after the main thread at the receiving end notices that the push
>   failed and decides not to read these notices and return a failure.

Yep, that's perfect.

> >> * da/user-useconfigonly (2016-04-01) 2 commits
> [...]
> What often happens is that I start waiting, and then when
> necessary actions to move things forward is never taken, and there
> are many other topics that need my attention to move forward, I stop
> caring, especially when the topic is not something other people care
> about (if the original author does not care deeply enough, why
> should we?).
> 
> Let me read it over again as it has been a while and then move it
> forward with your Reviewed-by's.

It is a minor bugfix, but I think worth doing. Adding reviewed-by me is
definitely fine.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-lib: simplify '--option=value' parsing

2016-04-22 Thread Jeff King
On Fri, Apr 22, 2016 at 05:38:23PM +0200, SZEDER Gábor wrote:

> To get the 'value' from '--option=value', test-lib.sh parses said
> option running 'expr' with a regexp.  This involves a subshell, an
> external process, and a lot of non-alphanumeric characters in the
> regexp.
> 
> Use a much simpler shell parameter expansion instead to do the same.

Looks OK to me. I doubt the extra process is a killer (we only parse
options once per script), but the result is definitely more readable,
IMHO.

For some reason I had always assumed that complicated pattern matching
with "#" was non-POSIX, but I checked and it is definitely in there.

> ---
>  t/test-lib.sh | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

I count 5 cases in my copy of test-lib.sh. I think you are missing
"--run".

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] name-rev: include taggerdate in considering the best name

2016-04-22 Thread Junio C Hamano
Jeff King  writes:

> That turned out to be quite simple (I wasn't sure originally if we'd
> actually visit all of the tags, which is why I had conceived of this as
> an initial pass; but of course it makes sense that we'd have to see all
> of the tags in the existing code).
> ...
> We could _also_ tweak the merge-weight as Linus's patch did, just
> because 1 has more basis than 65535. But I think it really matters a
> lot less at this point.

I agree, but if we were to go this route of keeping track of "some"
attribute of the tip the traversal started from, I wonder if it is
better to keep the actual tag object, not just its tagger date as an
unsigned long, in the new field.

That way, a tweak may be able to even use the v:refname comparison
if we wanted to do so in the future.  It is easy to go from a tag
object to its tagger date, but it is impossible to go in the other
direction, i.e. given a tagger date to go back to the tag.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1] travis-ci: build documentation

2016-04-22 Thread Junio C Hamano
Matthieu Moy  writes:

> larsxschnei...@gmail.com writes:
>
>> +  if [[ "$TRAVIS_OS_NAME" = linux ]] && [[ "$CC" = gcc ]];
>
> [[ is a bashism, and doesn't bring anything here compared to the POSIX
> [ ... ], or "test" which is prefered in Git's source code.
>
> The ; or the newline is not needed either.

Honestly, I didn't know that we were even trying to be pure POSIX,
avoid bashism or GNUism, or in general to follow our shell scripting
style in the scriptlet in the .travis.yml file.

While I feel fairly strongly about keeping the generic part generic,
I am actually OK with things that are known to be used in a specific
environment to be specific to that environment.

Having said all that, if we are not benefiting from using features
beyond POSIX, then by all means we should strive to be writing our
stuff in a portable way, as we do not have firm control over from
where and to where people cut and paste code snippets.

And I do think bashism [[ ... ]] is *NOT* buying anything in this
particular case, so I do agree with you that

if test "$TRAVIS_OS_NAME" = linux && test "$CC" = gcc
then
...

or even

case "$TRAVIS_OS_NAME,$CC" in
linux,gcc)
...

is what I would have written instead if I were writing this
conditional.

If we were to shoot for "be POSIX unless we can clearly benefit from
being bash/gnu/linux specific in bash/gnu/linux specific parts", the
existing scriptlets in .travis.yml file has a few things that may
need to be cleaned up already.  There are "mkdir --parents" (POSIX
spells it "-p"), "pushd/popd" and invocation of "tar" is very GNU
specific in the part that appears in the case arm for "linux".

There also are existing instances of "useless ;" that would want to
be cleaned up regardless of portability issues.

>> +  then
>> +  echo ""
>> +  echo 
>> "" &&
>
> I usualy avoid "echo " as I'm not sure how
> portable it is across variants of "echo". Maybe this one is portable
> enough, I don't know. Perhaps printf, or cat << EOF ...?

Do you even need a long divider there?

> I think it makes sense to do some lightweight checks after "make doc",
> rather than just check the return code. For example, check that a few
> generated files exist and are non-empty, like
>
> test -s Documentation/git.html &&
> test -s Documentation/git.1

Yup, or the formatter does not give new/unknown warnings.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] name-rev: include taggerdate in considering the best name

2016-04-22 Thread Jeff King
On Fri, Apr 22, 2016 at 03:39:01PM +0200, Johannes Schindelin wrote:

> We most likely want the oldest tag that contained the commit to be
> reported. So let's remember the taggerdate, and make it more important
> than anything else when choosing the best name for a given commit.
> 
> Suggested by Linus Torvalds.
> 
> Note that we need to update t9903 because it tested for the old behavior
> (which preferred the description "b1~1" over "tags/t2~1").
> 
> We might want to introduce a --heed-taggerdate option, and make the new
> behavior dependent on that, if it turns out that some scripts rely on the
> old name-rev method.
> 
> Signed-off-by: Johannes Schindelin 
> ---
>  builtin/name-rev.c | 19 +--
>  t/t9903-bash-prompt.sh |  2 +-
>  2 files changed, 14 insertions(+), 7 deletions(-)

That turned out to be quite simple (I wasn't sure originally if we'd
actually visit all of the tags, which is why I had conceived of this as
an initial pass; but of course it makes sense that we'd have to see all
of the tags in the existing code).

I confirmed that it does find the "optimal" tag for the case we've been
discussing.

We could _also_ tweak the merge-weight as Linus's patch did, just
because 1 has more basis than 65535. But I think it really matters a
lot less at this point.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: possible bug of git stash deleting uncommitted files in corner case

2016-04-22 Thread Junio C Hamano
Remi Galan Alfonso 
writes:

> Daniele Segato  wrote:
> ...
>> git version 1.9.1
>
> Contrary to what I expected, this seems to still be the case with:
>   $ git --version
>   git version 2.8.0.rc2

I do not think "git stash" has been updated in any major way to
address correctness (including its corner case behaviour) ever since
it was originally written, so it is very likely that any bug you see
would be with it since the very old days.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] string_list: use string-list API in unsorted_string_list_lookup()

2016-04-22 Thread Ralf Thielow
Using the string-list API in function unsorted_string_list_lookup()
makes the code more readable.  So let's do this.

Signed-off-by: Ralf Thielow 
---
 string-list.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/string-list.c b/string-list.c
index 2a32a3f..8127e12 100644
--- a/string-list.c
+++ b/string-list.c
@@ -231,12 +231,13 @@ void string_list_sort(struct string_list *list)
 struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
 const char *string)
 {
-   int i;
+   struct string_list_item *item;
compare_strings_fn cmp = list->cmp ? list->cmp : strcmp;
 
-   for (i = 0; i < list->nr; i++)
-   if (!cmp(string, list->items[i].string))
-   return list->items + i;
+   for_each_string_list_item(item, list) {
+   if (!cmp(string, item->string))
+   return item;
+   }
return NULL;
 }
 
-- 
2.8.1.382.g1352ede

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 0 bot for Git

2016-04-22 Thread Junio C Hamano
Lars Schneider  writes:

> Thanks for the explanation. My intention was not to be offensive.
> I was curious about your workflow and I was wondering if the
> Travis CI integration could be useful for you in any way.

Don't worry; I didn't feel offended.  The Travis stuff running on
the branches at http://github.com/git/git would surely catch issues
on MacOSX and/or around git-p4 (neither of which I test myself when
merging to 'pu') before they hit 'next', and that is already helping
us greatly.

And if volunteers or bots pick up in-flight patches that have not
hit 'pu' and feed them to Travis through their repositories, that
would also help the project, so your work on hooking up our source
tree with Travis is greatly appreciated.

It was just that Travis running on broken-down topic branches that
appear in http://github.com/gitster/git would not help my workflow,
which was the main point of illustrating the way how these branches
work.

Thanks.


  
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

2016-04-22 Thread Junio C Hamano
Torsten Bögershausen  writes:

> * tb/convert-eol-autocrlf (2016-04-19) 4 commits
>  - convert.c: ident + core.autocrlf didn't work
>  - t0027: test cases for combined attributes
>  - convert: allow core.autocrlf=input and core.eol=crlf
>  - t0027: avoid false "unchanged" due to lstat() matching after a change
>
>  Setting core.autocrlf to 'input' and core.eol to 'crlf' used to be
>  rejected, but because the code gives precedence to core.autcrlf,
>  there is no need to, hence we no longer reject the combination.
>
>  Will merge to 'next'.
> I know that I asked for an early merge of 4/4, but there is a new version
> with a fix for the leaking filter coming out this evening, european time.
> Please hold it.

I'll drop what has been queued and wait for a reroll.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

2016-04-22 Thread Junio C Hamano
Johannes Schindelin  writes:

> Hi Junio,
>
> On Thu, 21 Apr 2016, Junio C Hamano wrote:
>
>> * js/am-3-merge-recursive-direct (2015-10-12) 2 commits
> ...
> I hope to find the time next week to go through the entire call graph and
> verify that we are only die()ing in case if really critical errors (such
> as out-of-memory, in which case we traditionally just die).

I'll drop what has been queued and wait for a reroll.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

2016-04-22 Thread Junio C Hamano
Pranit Bauva  writes:

> On Fri, Apr 22, 2016 at 3:50 AM, Junio C Hamano  wrote:
>
>> [Cooking]
>>
>> * pb/commit-verbose-config (2016-04-19) 6 commits
>>  - commit: add a commit.verbose config variable
>>  - t7507-commit-verbose: improve test coverage by testing number of diffs
>>  - parse-options.c: make OPTION_COUNTUP respect "unspecified" values
>>  - t0040-parse-options: improve test coverage
>>  - test-parse-options: print quiet as integer
>>  - t0040-test-parse-options.sh: fix style issues
>>
>>  "git commit" learned to pay attention to "commit.verbose"
>>  configuration variable and act as if "--verbose" option was
>>  given from the command line.
>>
>>  Is this going to be rerolled?
>>  ($gmane/291382)
>
> The changes weren't that big enough and I had my end semester exams
> coming so I decided not to re-roll it.
> If you think contrary, I can do a re-roll with the changes suggested
> by Eric Sunshine.

I agree that this was pretty close to be done.  Let me see if I can
find time to finish it up in the coming few days.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 0/6] Move PGP verification out of verify-tag

2016-04-22 Thread Eric Sunshine
On Fri, Apr 22, 2016 at 10:51 AM,   wrote:
> This is a follow up of [1], [2], [3], [4], [5], [6], and [7].  patches 1/6,
> 2/6, and 3/6, are the same as the corresponding commits in pu.
>
> v8:
> Minor nits, I decided to quickly reroll to drop the extern qualifier in tag.c:
>   * Eric pointed out that we could block-scope the declaration of name and 
> sha1
> in b/verify-tag.c, for 4/6
>   * There was a typo in 6/6
>   * I dropped the extern qualifier in tag.c for 5/6 as suggested by Ramsay
> Jones[8]

Thanks, this version seems to address the few minor review comments
from v7. Regardless of the whitespace nit in patch 5/6, this series is
still:

Reviewed-by: Eric Sunshine 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

2016-04-22 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Apr 21, 2016 at 03:20:39PM -0700, Junio C Hamano wrote:
>
>> * jk/push-client-deadlock-fix (2016-04-20) 5 commits
>>  - t5504: drop sigpipe=ok from push tests
>>  - fetch-pack: isolate sigpipe in demuxer thread
>>  - send-pack: isolate sigpipe in demuxer thread
>>  - run-command: teach async threads to ignore SIGPIPE
>>  - send-pack: close demux pipe before finishing async process
>> 
>>  "git push" from a corrupt repository that attempts to push a large
>>  number of refs deadlocked waiting for a rejection from the
>>  receiving end that will never come.
>> 
>>  Will merge to 'next'.
>
> Minor nit, but the deadlock is the other way around: the rejection
> showed up and our demuxer is blocked writing to a reader who does not
> care about it.
>
> Might be worth fixing since this text goes into the topic merge commit
> (though I really hope nobody ever has to debug it enough ever again for
> that distinction to matter :) ).

Thanks.  Something like this, perhaps?

  "git push" from a corrupt repository that attempts to push a large
  number of refs deadlocked; the thread to relay rejection notices
  for these ref updates blocked on writing them to the main thread,
  after the main thread at the receiving end notices that the push
  failed and decides not to read these notices and return a failure.


>> * da/user-useconfigonly (2016-04-01) 2 commits
>>  - ident: give "please tell me" message upon useConfigOnly error
>>  - ident: check for useConfigOnly before auto-detection of name/email
>> 
>>  The "user.useConfigOnly" configuration variable makes it an error
>>  if users do not explicitly set user.name and user.email.  However,
>>  its check was not done early enough and allowed another error to
>>  trigger, reporting that the default value we guessed from the
>>  system setting was unusable.  This was a suboptimal end-user
>>  experience as we want the users to set user.name/user.email without
>>  relying on the auto-detection at all.
>> 
>>  Waiting for Acks.
>>  ($gmane/290340)
>
> I think you are waiting for the Ack from the original author on your
> tweaks. But FWIW, what you have queued looks good to me.

What often happens is that I start waiting, and then when
necessary actions to move things forward is never taken, and there
are many other topics that need my attention to move forward, I stop
caring, especially when the topic is not something other people care
about (if the original author does not care deeply enough, why
should we?).

Let me read it over again as it has been a while and then move it
forward with your Reviewed-by's.

>> * dk/gc-more-wo-pack (2016-01-13) 4 commits
>>  - gc: clean garbage .bitmap files from pack dir
>>  - t5304: ensure non-garbage files are not deleted
>>  - t5304: test .bitmap garbage files
>>  - prepare_packed_git(): find more garbage
>> 
>>  Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
>>  .bitmap and .keep files.
>> 
>>  Waiting for a reroll.
>>  ($gmane/284368).
>
> This one's getting pretty stale, but as I recall was pretty close to
> done.  I'll try to give it a look in the next couple of days.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 5/6] verify-tag: move tag verification code to tag.c

2016-04-22 Thread Eric Sunshine
On Fri, Apr 22, 2016 at 10:52 AM,   wrote:
> The PGP verification routine for tags could be accessed by other modules
> that require to do so.
>
> Publish the verify_tag function in tag.c and rename it to gpg_verify_tag
> so it does not conflict with builtin/mktag's static function.
>
> Helped-by: Junio C Hamano 
> Signed-off-by: Santiago Torres 
> ---
> diff --git a/tag.c b/tag.c
> @@ -6,6 +6,59 @@
> +int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,

Nit: This line has trailing whitespace. Probably not worth a re-roll.

> +   unsigned flags)
> +{
> +   enum object_type type;
> +   char *buf;
> +   unsigned long size;
> +   int ret;
> +
> +   type = sha1_object_info(sha1, NULL);
> +   if (type != OBJ_TAG)
> +   return error("%s: cannot verify a non-tag object of type %s.",
> +   name_to_report ?
> +   name_to_report :
> +   find_unique_abbrev(sha1, DEFAULT_ABBREV),
> +   typename(type));
> +
> +   buf = read_sha1_file(sha1, , );
> +   if (!buf)
> +   return error("%s: unable to read file.",
> +   name_to_report ?
> +   name_to_report :
> +   find_unique_abbrev(sha1, DEFAULT_ABBREV));
> +
> +   ret = run_gpg_verify(buf, size, flags);
> +
> +   free(buf);
> +   return ret;
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] test-lib: simplify '--option=value' parsing

2016-04-22 Thread SZEDER Gábor
To get the 'value' from '--option=value', test-lib.sh parses said
option running 'expr' with a regexp.  This involves a subshell, an
external process, and a lot of non-alphanumeric characters in the
regexp.

Use a much simpler shell parameter expansion instead to do the same.

Signed-off-by: SZEDER Gábor 
---
 t/test-lib.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0b47eb6bb299..8373d5b5c900 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -208,7 +208,7 @@ do
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
verbose=t; shift ;;
--verbose-only=*)
-   verbose_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+   verbose_only=${1#--*=}
shift ;;
-q|--q|--qu|--qui|--quie|--quiet)
# Ignore --quiet under a TAP::Harness. Saying how many tests
@@ -222,15 +222,15 @@ do
valgrind=memcheck
shift ;;
--valgrind=*)
-   valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
+   valgrind=${1#--*=}
shift ;;
--valgrind-only=*)
-   valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+   valgrind_only=${1#--*=}
shift ;;
--tee)
shift ;; # was handled already
--root=*)
-   root=$(expr "z$1" : 'z[^=]*=\(.*\)')
+   root=${1#--*=}
shift ;;
--chain-lint)
GIT_TEST_CHAIN_LINT=1
-- 
2.8.1.99.g5d5236f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 1/6] builtin/verify-tag.c: ignore SIGPIPE in gpg-interface

2016-04-22 Thread santiago
From: Santiago Torres 

The verify_signed_buffer() function may trigger a SIGPIPE when the
GPG child process terminates early (due to a bad keyid, for example)
and Git tries to write to it afterwards.  Previously, ignoring
SIGPIPE was done in builtin/verify-tag.c to avoid this issue.

However, any other caller who wants to call verify_signed_buffer()
would have to do the same.

Use sigchain_push(SIGPIPE, SIG_IGN) in verify_signed_buffer(),
pretty much like in sign_buffer(), so that any caller is not
required to perform this task.

This will avoid possible mistakes by further developers using
verify_signed_buffer().

Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..2259938 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
return error(_("could not run gpg."));
}
 
+   sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
 
@@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command();
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 02/10] convert: allow core.autocrlf=input and core.eol=crlf

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

Even though the configuration parser errors out when core.autocrlf
is set to 'input' when core.eol is set to 'crlf', there is no need
to do so, because the core.autocrlf setting trumps core.eol.

Allow all combinations of core.crlf and core.eol and document
that core.autocrlf overrides core.eol.

Signed-off-by: Torsten Bögershausen 
---
 Documentation/config.txt | 6 +++---
 config.c | 4 
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..155f988 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -337,9 +337,9 @@ core.quotePath::
 
 core.eol::
Sets the line ending type to use in the working directory for
-   files that have the `text` property set.  Alternatives are
-   'lf', 'crlf' and 'native', which uses the platform's native
-   line ending.  The default value is `native`.  See
+   files that have the `text` property set when core.autocrlf is false.
+   Alternatives are 'lf', 'crlf' and 'native', which uses the platform's
+   native line ending.  The default value is `native`.  See
linkgit:gitattributes[5] for more information on end-of-line
conversion.
 
diff --git a/config.c b/config.c
index 4c92699..c8ac9c2 100644
--- a/config.c
+++ b/config.c
@@ -803,8 +803,6 @@ static int git_default_core_config(const char *var, const 
char *value)
 
if (!strcmp(var, "core.autocrlf")) {
if (value && !strcasecmp(value, "input")) {
-   if (core_eol == EOL_CRLF)
-   return error("core.autocrlf=input conflicts 
with core.eol=crlf");
auto_crlf = AUTO_CRLF_INPUT;
return 0;
}
@@ -830,8 +828,6 @@ static int git_default_core_config(const char *var, const 
char *value)
core_eol = EOL_NATIVE;
else
core_eol = EOL_UNSET;
-   if (core_eol == EOL_CRLF && auto_crlf == AUTO_CRLF_INPUT)
-   return error("core.autocrlf=input conflicts with 
core.eol=crlf");
return 0;
}
 
-- 
2.8.0.rc2.2.g1a4d45a.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 6/6] tag -v: verify directly rather than exec-ing verify-tag

2016-04-22 Thread santiago
From: Santiago Torres 

Instead of having tag -v fork to run verify-tag, use the
gpg_verify_tag() function directly.

Helped-by: Eric Sunshine 
Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..7b2918e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,13 +104,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
+   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 0/6] Move PGP verification out of verify-tag

2016-04-22 Thread santiago
From: Santiago Torres 

This is a follow up of [1], [2], [3], [4], [5], [6], and [7].  patches 1/6,
2/6, and 3/6, are the same as the corresponding commits in pu.

v8:  
Minor nits, I decided to quickly reroll to drop the extern qualifier in tag.c:
  * Eric pointed out that we could block-scope the declaration of name and sha1
in b/verify-tag.c, for 4/6
  * There was a typo in 6/6
  * I dropped the extern qualifier in tag.c for 5/6 as suggested by Ramsay
Jones[8]

v7: 
Mostly style/clarity changes. Thanks Peff, Eric and Junio for the
feedback! In summary: 

 * Eric pointed out issues with 3/6's commit message. It doesn't match the one 
   in pu though. I also took the opportunity to update payload_size to a size_t
   as Peff suggested.
 * 4/6 I updated report_name to name_to_report, I updated the commit message 
   and addressed some nits in the code, one of the fixes removed all three nits
   that Eric pointed out. I updated 5/6 to match these changes
 * I gave the commit message on 6/6 another go.

v6: 
 * As Junio suggested, updated 4/6, to include the name argument and the
   ternary operator to provide more descriptive error messages. I propagated
   these changes to 5/6 and 6/6 as well. I'm unsure about the 80-column
   on 4/6, the ternary operator is rather long.
 * Updated and reviewed the commit messages based on Eric and Junio's
   feedback

v5:
Added helpful feedback by Eric

 * Reordering of the patches, to avoid temporal inclusion of a regression
 * Fix typos here and there.
 * Review commit messages, as some weren't representative of what the patches
   were doing anymore.
 * Updated t7030 to include Peff's suggestion, and added a helped-by line here
   as it was mostly Peff's code.
 * Updated the error-handling/printing issues that were introduced when.
   libifying the verify_tag function.

v4:

Thanks Eric, Jeff, and Hannes for the feedback.

 * I relocated the sigchain_push call so it comes after the error on
   gpg-interface (thanks Hannnes for catching this).
 * I updated the unit test to match the discussion on [3]. Now it generates
   the expected output of the tag on the fly for comparison. (This is just
   copy and paste from [3], but I verified that it works by breaking the
   while)
 * I split moving the code and renaming the variables into two patches so
   these are easier to review.
 * I used an adapter on builtin/tag.c instead of redefining all the fn*
   declarations everywhere. This introduces an issue with the way git tag -v
   resolves refnames though. I added a new commit to restore the previous
   behavior of git-tag. I'm not sure if I should've split this into two commits
   though.

v3:
Thanks Eric, Jeff, for the feedback.

 * I separated the patch in multiple sub-patches.
 * I compared the behavior of previous git tag -v and git verify-tag 
   invocations to make sure the behavior is the same
 * I dropped the multi-line comment, as suggested.
 * I fixed the issue with the missing brackets in the while (this is 
   now detected by the test).

v2:

 * I moved the pgp-verification code to tag.c 
 * I added extra arguments so git tag -v and git verify-tag both work
   with the same function
 * Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:
 
The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.


This applies on v2.8.0. 
Thanks!
-Santiago

[1] http://thread.gmane.org/gmane.comp.version-control.git/287649
[2] http://thread.gmane.org/gmane.comp.version-control.git/289836
[3] http://thread.gmane.org/gmane.comp.version-control.git/290608
[4] http://thread.gmane.org/gmane.comp.version-control.git/290731
[5] http://thread.gmane.org/gmane.comp.version-control.git/290790
[6] http://thread.gmane.org/gmane.comp.version-control.git/291780
[7] http://thread.gmane.org/gmane.comp.version-control.git/291887
[8] http://thread.gmane.org/gmane.comp.version-control.git/292029



Santiago Torres (6):
  builtin/verify-tag.c: ignore SIGPIPE in gpg-interface
  t7030: test verifying multiple tags
  verify-tag: update variable name and type
  verify-tag: prepare verify_tag for libification
  verify-tag: move tag verification code to tag.c
  tag -v: verify directly rather than exec-ing verify-tag

 builtin/tag.c |  8 +--
 builtin/verify-tag.c  | 61 ++-
 gpg-interface.c   |  2 ++
 t/t7030-verify-tag.sh | 13 +++
 tag.c | 53 
 tag.h |  2 ++
 6 files changed, 78 insertions(+), 61 deletions(-)

-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 5/6] verify-tag: move tag verification code to tag.c

2016-04-22 Thread santiago
From: Santiago Torres 

The PGP verification routine for tags could be accessed by other modules
that require to do so.

Publish the verify_tag function in tag.c and rename it to gpg_verify_tag
so it does not conflict with builtin/mktag's static function.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 55 +---
 tag.c| 53 ++
 tag.h|  2 ++
 3 files changed, 56 insertions(+), 54 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index a3d3a43..99f8148 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,59 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   size_t payload_size;
-   int ret;
-
-   memset(, 0, sizeof(sigc));
-
-   payload_size = parse_signature(buf, size);
-
-   if (size == payload_size) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, payload_size);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, payload_size, buf + payload_size,
-   size - payload_size, );
-   print_signature_buffer(, flags);
-
-   signature_check_clear();
-   return ret;
-}
-
-static int verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
-{
-   enum object_type type;
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   name_to_report ?
-   name_to_report :
-   find_unique_abbrev(sha1, DEFAULT_ABBREV),
-   typename(type));
-
-   buf = read_sha1_file(sha1, , );
-   if (!buf)
-   return error("%s: unable to read file.",
-   name_to_report ?
-   name_to_report :
-   find_unique_abbrev(sha1, DEFAULT_ABBREV));
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -104,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (verify_tag(sha1, name, flags))
+   else if (gpg_verify_tag(sha1, name, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index d72f742..8363a0e 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,59 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   size_t payload_size;
+   int ret;
+
+   memset(, 0, sizeof(sigc));
+
+   payload_size = parse_signature(buf, size);
+
+   if (size == payload_size) {
+   if (flags & GPG_VERIFY_VERBOSE)
+   write_in_full(1, buf, payload_size);
+   return error("no signature found");
+   }
+
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, );
+   print_signature_buffer(, flags);
+
+   signature_check_clear();
+   return ret;
+}
+
+int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report, 
+   unsigned flags)
+{
+   enum object_type type;
+   char *buf;
+   unsigned long size;
+   int ret;
+
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   name_to_report ?
+   name_to_report :
+   find_unique_abbrev(sha1, DEFAULT_ABBREV),
+   typename(type));
+
+   buf = read_sha1_file(sha1, , );
+   if (!buf)
+   return error("%s: unable to read file.",
+   name_to_report ?
+   name_to_report :
+   find_unique_abbrev(sha1, DEFAULT_ABBREV));
+
+   ret = run_gpg_verify(buf, size, flags);
+
+   free(buf);
+   return ret;
+}
+
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
while (o && 

[PATCH v8 3/6] verify-tag: update variable name and type

2016-04-22 Thread santiago
From: Santiago Torres 

The run_gpg_verify() function has two variables, size and len.

This may come off as confusing when reading the code. Clarify which one
pertains to the length of the tag headers by renaming len to
payload_size. Additionally, change the type of payload_size to size_t to
match the return type of parse_signature.

Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 builtin/verify-tag.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..fa26e40 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = {
 static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 {
struct signature_check sigc;
-   int len;
+   size_t payload_size;
int ret;
 
memset(, 0, sizeof(sigc));
 
-   len = parse_signature(buf, size);
+   payload_size = parse_signature(buf, size);
 
-   if (size == len) {
+   if (size == payload_size) {
if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
+   write_in_full(1, buf, payload_size);
return error("no signature found");
}
 
-   ret = check_signature(buf, len, buf + len, size - len, );
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, );
print_signature_buffer(, flags);
 
signature_check_clear();
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 2/6] t7030: test verifying multiple tags

2016-04-22 Thread santiago
From: Santiago Torres 

The verify-tag command supports multiple tag names to verify, but
existing tests only test for invocation with a single tag.

Add a test invoking it with multiple tags.

Helped-by: Jeff King 
Signed-off-by: Santiago Torres 
Reviewed-by: Eric Sunshine 
Signed-off-by: Junio C Hamano 
---
 t/t7030-verify-tag.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..07079a4 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,17 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   tags="fourth-signed sixth-signed seventh-signed" &&
+   for i in $tags
+   do
+   git verify-tag -v --raw $i || return 1
+   done >expect.stdout 2>expect.stderr.1 &&
+   grep "^.GNUPG:." expect.stderr &&
+   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+   grep "^.GNUPG:." actual.stderr &&
+   test_cmp expect.stdout actual.stdout &&
+   test_cmp expect.stderr actual.stderr
+'
+
 test_done
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 4/6] verify-tag: prepare verify_tag for libification

2016-04-22 Thread santiago
From: Santiago Torres 

The current interface of verify_tag() resolves reference names to SHA1,
however, the plan is to make this functionality public and the current
interface is cumbersome for callers: they are expected to supply the
textual representation of a sha1/refname. In many cases, this requires
them to turn the sha1 to hex representation, just to be converted back
inside verify_tag.

Add a SHA1 parameter to use instead of the name parameter, and rename
the name parameter to "name_to_report" for reporting purposes only.

Helped-by: Junio C Hamano 
Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index fa26e40..a3d3a43 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -42,25 +42,28 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-static int verify_tag(const char *name, unsigned flags)
+static int verify_tag(const unsigned char *sha1, const char *name_to_report,
+   unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
char *buf;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
+   name_to_report ?
+   name_to_report :
+   find_unique_abbrev(sha1, DEFAULT_ABBREV),
+   typename(type));
 
buf = read_sha1_file(sha1, , );
if (!buf)
-   return error("%s: unable to read file.", name);
+   return error("%s: unable to read file.",
+   name_to_report ?
+   name_to_report :
+   find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
@@ -96,8 +99,13 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   while (i < argc) {
+   unsigned char sha1[20];
+   const char *name = argv[i++];
+   if (get_sha1(name, sha1))
+   had_error = !!error("tag '%s' not found.", name);
+   else if (verify_tag(sha1, name, flags))
had_error = 1;
+   }
return had_error;
 }
-- 
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 01/10] t0027: Make more reliable

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

Make the commit_chk_wrnNNO test in t0027 more reliable:
When the attributes of a commited file are changed and the file is otherwise
unchanged, Git may not detect that the next commit may need to treat the
file as changed.
This happens when lstat() doesn't detect a change, since neither inode,
mtime nor size are changed.

Add a singe "Z" character to change the file size.
Ignore it when comparing the files later.

Signed-off-by: Torsten Bögershausen 
---
 Changes since v5:
 - send the whole series, now 10/10
 - Removed the "will change in future" in one commit msg
 - Don't leak the filer in 4/10
 t/t0027-auto-crlf.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index f33962b..9fe539b 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -12,7 +12,7 @@ fi
 
 compare_files () {
tr '\015\000' QN <"$1" >"$1".expect &&
-   tr '\015\000' QN <"$2" >"$2".actual &&
+   tr '\015\000' QN <"$2" | tr -d 'Z' >"$2".actual &&
test_cmp "$1".expect "$2".actual &&
rm "$1".expect "$2".actual
 }
@@ -114,6 +114,7 @@ commit_chk_wrnNNO () {
do
fname=${pfx}_$f.txt &&
cp $f $fname &&
+   printf Z >>"$fname" &&
git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname 
>"${pfx}_$f.err" 2>&1
done
-- 
2.8.0.rc2.2.g1a4d45a.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 03/10] t0027: test cases for combined attributes

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

Add more test cases for the not normalized files ("NNO").  The
"text" attribute is most important, use it as the first parameter.
"ident", if set, is the second paramater followed by the eol
attribute.  The eol attribute overrides core.autocrlf, which
overrides core.eol.
indent is not yet uses, this will be done in the next commit.

Use loops to test more combinations of attributes, like
"* text eol=crlf" or especially "*text=auto eol=crlf".

Signed-off-by: Torsten Bögershausen 
---
 t/t0027-auto-crlf.sh | 298 ++-
 1 file changed, 129 insertions(+), 169 deletions(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 9fe539b..fd5e326 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -52,14 +52,17 @@ create_gitattributes () {
 create_NNO_files () {
for crlf in false true input
do
-   for attr in "" auto text -text lf crlf
+   for attr in "" auto text -text
do
-   pfx=NNO_${crlf}_attr_${attr} &&
-   cp CRLF_mix_LF ${pfx}_LF.txt &&
-   cp CRLF_mix_LF ${pfx}_CRLF.txt &&
-   cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
-   cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
-   cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
+   for aeol in "" lf crlf
+   do
+   pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
+   cp CRLF_mix_LF ${pfx}_LF.txt &&
+   cp CRLF_mix_LF ${pfx}_CRLF.txt &&
+   cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
+   cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
+   cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
+   done
done
done
 }
@@ -100,16 +103,17 @@ commit_check_warn () {
 }
 
 commit_chk_wrnNNO () {
-   crlf=$1
-   attr=$2
-   lfwarn=$3
-   crlfwarn=$4
-   lfmixcrlf=$5
-   lfmixcr=$6
-   crlfnul=$7
-   pfx=NNO_${crlf}_attr_${attr}
+   attr=$1 ; shift
+   aeol=$1 ; shift
+   crlf=$1 ; shift
+   lfwarn=$1 ; shift
+   crlfwarn=$1 ; shift
+   lfmixcrlf=$1 ; shift
+   lfmixcr=$1 ; shift
+   crlfnul=$1 ; shift
+   pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
#Commit files on top of existing file
-   create_gitattributes "$attr" &&
+   create_gitattributes "$attr" $aeol &&
for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
do
fname=${pfx}_$f.txt &&
@@ -122,19 +126,19 @@ commit_chk_wrnNNO () {
test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
check_warning "$lfwarn" ${pfx}_LF.err
'
-   test_expect_success "commit NNO files crlf=$crlf attr=$attr CRLF" '
+   test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf 
CRLF" '
check_warning "$crlfwarn" ${pfx}_CRLF.err
'
 
-   test_expect_success "commit NNO files crlf=$crlf attr=$attr 
CRLF_mix_LF" '
+   test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf 
CRLF_mix_LF" '
check_warning "$lfmixcrlf" ${pfx}_CRLF_mix_LF.err
'
 
-   test_expect_success "commit NNO files crlf=$crlf attr=$attr LF_mix_cr" '
+   test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf 
LF_mix_cr" '
check_warning "$lfmixcr" ${pfx}_LF_mix_CR.err
'
 
-   test_expect_success "commit NNO files crlf=$crlf attr=$attr CRLF_nul" '
+   test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf 
CRLF_nul" '
check_warning "$crlfnul" ${pfx}_CRLF_nul.err
'
 }
@@ -163,6 +167,7 @@ stats_ascii () {
 
 # contruct the attr/ returned by git ls-files --eol
 # Take none (=empty), one or two args
+# convert.c: eol=XX overrides text=auto
 attr_ascii () {
case $1,$2 in
-text,*)   echo "-text" ;;
@@ -170,8 +175,8 @@ attr_ascii () {
text,lf)   echo "text eol=lf" ;;
text,crlf) echo "text eol=crlf" ;;
auto,) echo "text=auto" ;;
-   auto,lf)   echo "text=auto eol=lf" ;;
-   auto,crlf) echo "text=auto eol=crlf" ;;
+   auto,lf)   echo "text eol=lf" ;;
+   auto,crlf) echo "text eol=crlf" ;;
lf,)   echo "text eol=lf" ;;
crlf,) echo "text eol=crlf" ;;
,) echo "" ;;
@@ -196,28 +201,29 @@ check_files_in_repo () {
 }
 
 check_in_repo_NNO () {
-   crlf=$1
-   attr=$2
-   lfname=$3
-   crlfname=$4
-   lfmixcrlf=$5
-   lfmixcr=$6
-   crlfnul=$7
-   pfx=NNO_${crlf}_attr_${attr}_
-   test_expect_success "compare_files $lfname ${pfx}LF.txt" '
-   compare_files $lfname ${pfx}LF.txt
+   attr=$1 ; shift
+   aeol=$1 ; shift
+   crlf=$1 ; shift

[PATCH v6 08/10] convert.c: more safer crlf handling with text attribute

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

A follow-up after a discussion how to fix the flaky execution
of t0025, gmane/$284352.

This patch extends the work done in commit c480539:
"Make it work also for un-normalized repositories". Make sure that CRLF
can be converted round trip, or don't convert them at all.

The old handling would treat a file as unchanged after checkout,
as long as it is not touched in the work tree and mtime matches the value
recorded in the index.
When the mtime is changed in the working tree, or the inode is changed,
the file is reported as modified.

The following sequence is now handled reproducable:
$ git init
$ printf "line1\r\n" >file.bat
$ git add file.bat
$ git commit -m "Add file with CRLF" file.bat
$ echo "*.bat text eol=crlf" >.gitattributes
$ git commit -m "bat files should have CRLF"
$ git status
 # nothing to commit, working directory clean
$ git push 
$ printf "newline\r\n" >>file.bat
$ mv file.bat file.sav
$ git checkout file.bat
$ git status
 #modified:   file.bat

The new handling makes sure that after running "git reset --hard".
"git status" reports the working tree as clean regarding CRLF conversion.
It makes sure that the Git-internal eol conversion is
doing roundtrip. A user can still write an external smudge/clean filter
outside Git, which doesn't do a roundtrip and the working directory is
not clean.

The functionality of has_cr_in_index() is turned into has_crlf_in_index(),
and the function is integrated into would_convert_crlf_at_commit().

Check for CRLF in the index instead of CR, the bit CONVERT_STAT_BITS_ANY_CR
is no longer used and removed, as well as "lonecr" in struct text_stat.

Rewrite check_safe_crlf() in convert.c to simulate checkin-checkout,
to detect whether any line endings are converted.

Add a warning, similar to the CRLF-LF replacement, when a file is commited,
and after the next checkout the line endings are not they should be.

Modify the lf_to_crlf_filter:
Files with LF are converted into CRLF, file with CRLF are not changed.
Files with mixed line endings are not converted, the filter fails, and Git
falls back to the non-streaming handling, see write_entry().

Signed-off-by: Torsten Bögershausen 
---
 Documentation/gitattributes.txt |  19 ++--
 convert.c   | 233 +---
 t/t0025-crlf-auto.sh|   8 +-
 t/t0027-auto-crlf.sh|  92 
 4 files changed, 212 insertions(+), 140 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index d7a124b..836461d 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -110,7 +110,7 @@ repository upon 'git add' and 'git commit'.
 `text`
 ^^
 
-This attribute enables and controls end-of-line normalization.  When a
+This attribute enables and controls end-of-line conversion.  When a
 text file is normalized, its line endings are converted to LF in the
 repository.  To control what line ending style is used in the working
 directory, use the `eol` attribute for a single file and the
@@ -120,8 +120,11 @@ Note that `core.autocrlf` overrides `core.eol`
 Set::
 
Setting the `text` attribute on a path enables end-of-line
-   normalization and marks the path as a text file.  End-of-line
+   conversion and marks the path as a text file.  End-of-line
conversion takes place without guessing the content type.
+   Files that have been commited with CRLF before the text attribute
+   is set and commited are not normalized. No end-of-line conversion
+   is done at checkout or checkin.
 
 Unset::
 
@@ -131,9 +134,9 @@ Unset::
 Set to string value "auto"::
 
When `text` is set to "auto", the path is marked for automatic
-   end-of-line conversion.  If Git decides that the content is
-   text, its line endings are converted to LF on checkin.
-   When the file has been commited with CRLF, no conversion is done.
+   end-of-line normalization.  If Git decides that the content is
+   text, and the path has no CRLF in the index,
+   its line endings are converted to LF on checkin.
 
 Unspecified::
 
@@ -148,8 +151,10 @@ unspecified.
 ^
 
 This attribute sets a specific line-ending style to be used in the
-working directory.  It enables end-of-line conversion without any
-content checks, effectively setting the `text` attribute.
+working directory.  It sets the `text` attribute, unless `text=auto`
+is specified.
+When the file had been commited with CRLF in the index, no conversion
+is done at checkout or commit.
 
 Set to string value "crlf"::
 
diff --git a/convert.c b/convert.c
index 3782172..8d4c42a 100644
--- a/convert.c
+++ b/convert.c
@@ -17,7 +17,8 @@
 #define CONVERT_STAT_BITS_TXT_LF0x1
 #define CONVERT_STAT_BITS_TXT_CRLF  0x2
 #define CONVERT_STAT_BITS_BIN   0x4
-#define CONVERT_STAT_BITS_ANY_CR0x8
+
+#define CONVERT_STAT_BITS_MIXED (CONVERT_STAT_BITS_TXT_LF | 

[PATCH v6 07/10] convert: unify the "auto" handling of CRLF

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

Before this change,
$ echo "* text=auto" >.gitattributes
$ echo "* eol=crlf" >>.gitattributes

would have the same effect as
$ echo "* text" >.gitattributes
$ git config core.eol crlf

Since the 'eol' attribute had higher priority than 'text=auto', this may
corrupt binary files and is not what most users expect to happen.

Make the 'eol' attribute to obey 'text=auto', and now
$ echo "* text=auto" >.gitattributes
$ echo "* eol=crlf" >>.gitattributes
behaves the same as
$ echo "* text=auto" >.gitattributes
$ git config core.eol crlf

In other words,
$ echo "* text=auto eol=crlf" >.gitattributes
has the same effect as
$ git config core.autocrlf true

and
$ echo "* text=auto eol=lf" >.gitattributes
has the same effect as
$ git config core.autocrlf input

Signed-off-by: Torsten Bögershausen 
---
 Documentation/config.txt| 14 +++---
 Documentation/gitattributes.txt | 15 +--
 convert.c   | 42 +
 convert.h   |  3 ++-
 t/t0025-crlf-auto.sh|  4 ++--
 t/t0027-auto-crlf.sh| 32 +++
 t/t6038-merge-text-auto.sh  | 23 ++
 7 files changed, 73 insertions(+), 60 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 155f988..117c2f3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -389,13 +389,13 @@ file with mixed line endings would be reported by the 
`core.safecrlf`
 mechanism.
 
 core.autocrlf::
-   Setting this variable to "true" is almost the same as setting
-   the `text` attribute to "auto" on all files except that text
-   files are not guaranteed to be normalized: files that contain
-   `CRLF` in the repository will not be touched.  Use this
-   setting if you want to have `CRLF` line endings in your
-   working directory even though the repository does not have
-   normalized line endings.  This variable can be set to 'input',
+   Setting this variable to "true" is the same as setting
+   the `text` attribute to "auto" on all files and core.eol to "crlf".
+   Set to true if you want to have `CRLF` line endings in your
+   working directory and the repository has LF line endings.
+   Text files are guaranteed not to be normalized: files that contain
+   `CRLF` in the repository will not be touched.
+   This variable can be set to 'input',
in which case no output conversion is performed.
 
 core.symlinks::
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e3b1de8..d7a124b 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -115,6 +115,7 @@ text file is normalized, its line endings are converted to 
LF in the
 repository.  To control what line ending style is used in the working
 directory, use the `eol` attribute for a single file and the
 `core.eol` configuration variable for all text files.
+Note that `core.autocrlf` overrides `core.eol`
 
 Set::
 
@@ -130,8 +131,9 @@ Unset::
 Set to string value "auto"::
 
When `text` is set to "auto", the path is marked for automatic
-   end-of-line normalization.  If Git decides that the content is
-   text, its line endings are normalized to LF on checkin.
+   end-of-line conversion.  If Git decides that the content is
+   text, its line endings are converted to LF on checkin.
+   When the file has been commited with CRLF, no conversion is done.
 
 Unspecified::
 
@@ -146,7 +148,7 @@ unspecified.
 ^
 
 This attribute sets a specific line-ending style to be used in the
-working directory.  It enables end-of-line normalization without any
+working directory.  It enables end-of-line conversion without any
 content checks, effectively setting the `text` attribute.
 
 Set to string value "crlf"::
@@ -186,9 +188,10 @@ the working directory, and prevent .jpg files from being 
normalized
 regardless of their content.
 
 
+*   text=auto
 *.txt  text
-*.vcproj   eol=crlf
-*.sh   eol=lf
+*.vcproj   text eol=crlf
+*.sh   text eol=lf
 *.jpg  -text
 
 
@@ -198,7 +201,7 @@ normalization in Git.
 
 If you simply want to have CRLF line endings in your working directory
 regardless of the repository you are working with, you can set the
-config variable "core.autocrlf" without changing any attributes.
+config variable "core.autocrlf" without using any attributes.
 
 
 [core]
diff --git a/convert.c b/convert.c
index 24ab095..3782172 100644
--- a/convert.c
+++ b/convert.c
@@ -227,7 +227,9 @@ static enum eol output_eol(enum crlf_action crlf_action)
return EOL_LF;
case CRLF_UNDEFINED:
case CRLF_AUTO_CRLF:
+   return EOL_CRLF;
case CRLF_AUTO_INPUT:
+   return 

[PATCH v6 06/10] convert.c: stream and early out

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

When statistics are done for the autocrlf handling, the search in
the content can be stopped, if e.g
- a search for binary is done, and a NUL character is found
- a search for CRLF is done, and the first CRLF is found.

Similar when statistics for binary vs non-binary are gathered:
Whenever a lone CR or NUL is found, the search can be aborted.

When checking out files in "auto" mode, any file that has a "lone CR"
or a CRLF will not be converted, so the search can be aborted early.

Add the new bit, CONVERT_STAT_BITS_ANY_CR,
which is set for either lone CR or CRLF.

Many binary files have a NUL very early (within the first few bytes,
latest within the first 1..2K).
It is often not necessary to load the whole content of a file or blob
into memory.

Use a streaming handling for blobs and files in the worktree.

Signed-off-by: Torsten Bögershausen 
---
 convert.c | 159 --
 1 file changed, 103 insertions(+), 56 deletions(-)

diff --git a/convert.c b/convert.c
index b1614bf..24ab095 100644
--- a/convert.c
+++ b/convert.c
@@ -3,6 +3,7 @@
 #include "run-command.h"
 #include "quote.h"
 #include "sigchain.h"
+#include "streaming.h"
 
 /*
  * convert.c - convert a file when checking it out and checking it in.
@@ -13,10 +14,10 @@
  * translation when the "text" attribute or "auto_crlf" option is set.
  */
 
-/* Stat bits: When BIN is set, the txt bits are unset */
 #define CONVERT_STAT_BITS_TXT_LF0x1
 #define CONVERT_STAT_BITS_TXT_CRLF  0x2
 #define CONVERT_STAT_BITS_BIN   0x4
+#define CONVERT_STAT_BITS_ANY_CR0x8
 
 enum crlf_action {
CRLF_UNDEFINED,
@@ -31,30 +32,36 @@ enum crlf_action {
 
 struct text_stat {
/* NUL, CR, LF and CRLF counts */
-   unsigned nul, lonecr, lonelf, crlf;
+   unsigned stat_bits, lonecr, lonelf, crlf;
 
/* These are just approximations! */
unsigned printable, nonprintable;
 };
 
-static void gather_stats(const char *buf, unsigned long size, struct text_stat 
*stats)
+static void do_gather_stats(const char *buf, unsigned long size,
+   struct text_stat *stats, unsigned earlyout)
 {
unsigned long i;
 
-   memset(stats, 0, sizeof(*stats));
-
+   if (!buf || !size)
+   return;
for (i = 0; i < size; i++) {
unsigned char c = buf[i];
if (c == '\r') {
+   stats->stat_bits |= CONVERT_STAT_BITS_ANY_CR;
if (i+1 < size && buf[i+1] == '\n') {
stats->crlf++;
i++;
-   } else
+   stats->stat_bits |= CONVERT_STAT_BITS_TXT_CRLF;
+   } else {
stats->lonecr++;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
+   }
continue;
}
if (c == '\n') {
stats->lonelf++;
+   stats->stat_bits |= CONVERT_STAT_BITS_TXT_LF;
continue;
}
if (c == 127)
@@ -67,7 +74,7 @@ static void gather_stats(const char *buf, unsigned long size, 
struct text_stat *
stats->printable++;
break;
case 0:
-   stats->nul++;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
/* fall through */
default:
stats->nonprintable++;
@@ -75,6 +82,8 @@ static void gather_stats(const char *buf, unsigned long size, 
struct text_stat *
}
else
stats->printable++;
+   if (stats->stat_bits & earlyout)
+   break; /* We found what we have been searching for */
}
 
/* If file ends with EOF then don't count this EOF as non-printable. */
@@ -86,41 +95,62 @@ static void gather_stats(const char *buf, unsigned long 
size, struct text_stat *
  * The same heuristics as diff.c::mmfile_is_binary()
  * We treat files with bare CR as binary
  */
-static int convert_is_binary(unsigned long size, const struct text_stat *stats)
+static void convert_nonprintable(struct text_stat *stats)
 {
-   if (stats->lonecr)
-   return 1;
-   if (stats->nul)
-   return 1;
if ((stats->printable >> 7) < stats->nonprintable)
-   return 1;
-   return 0;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
+}
+
+static void gather_stats(const char *buf, unsigned long size,
+struct text_stat *stats, unsigned earlyout)
+{
+   memset(stats, 0, sizeof(*stats));
+   do_gather_stats(buf, size, stats, earlyout);
+   

[PATCH v6 09/10] t6038; use crlf on all platforms

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

t6038 uses different code, dependig if NATIVE_CRLF is set ot not.
When the native line endings are LF, merge.renormalize is not tested very well.
Change the test to always use CRLF by setting core.eol=crlf.
After doing so, the test fails:

rm '.gitattributes'
rm 'control_file'
rm 'file'
rm 'inert_file'
HEAD is now at 0d9ffb6 add line from b
error: addinfo_cache failed for path 'file'
file: unmerged (cbd69ec7cd12dd0989e853923867d94c8519aa52)
file: unmerged (ad55e240aeb42e0d9a0e18d6d8b02dd82ee3e527)
file: unmerged (99b633103c15c20cebebf821133ab526b0ff90b2)
fatal: git write-tree failed to write a tree
Merging:
0d9ffb6 add line from b
virtual a
found 1 common ancestor:
1c56df1 Initial
Auto-merging file
not ok 4 - Merge addition of text=auto

This will be addressed in the next commit.
---
 t/t6038-merge-text-auto.sh | 37 +++--
 1 file changed, 11 insertions(+), 26 deletions(-)

diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
index 33b77ee..0108ead 100755
--- a/t/t6038-merge-text-auto.sh
+++ b/t/t6038-merge-text-auto.sh
@@ -25,6 +25,7 @@ compare_files () {
 
 test_expect_success setup '
git config core.autocrlf false &&
+   git config core.eol crlf &&
 
echo first line | append_cr >file &&
echo first line >control_file &&
@@ -79,10 +80,8 @@ test_expect_success 'Merge after setting text=auto' '
same line
EOF
 
-   if test_have_prereq NATIVE_CRLF; then
-   append_cr expected.temp &&
-   mv expected.temp expected
-   fi &&
+   append_cr expected.temp &&
+   mv expected.temp expected &&
git config merge.renormalize true &&
git rm -fr . &&
rm -f .gitattributes &&
@@ -97,10 +96,8 @@ test_expect_success 'Merge addition of text=auto' '
same line
EOF
 
-   if test_have_prereq NATIVE_CRLF; then
-   append_cr expected.temp &&
-   mv expected.temp expected
-   fi &&
+   append_cr expected.temp &&
+   mv expected.temp expected &&
git config merge.renormalize true &&
git rm -fr . &&
rm -f .gitattributes &&
@@ -111,15 +108,9 @@ test_expect_success 'Merge addition of text=auto' '
 
 test_expect_success 'Detect CRLF/LF conflict after setting text=auto' '
echo "<<<" >expected &&
-   if test_have_prereq NATIVE_CRLF; then
-   echo first line | append_cr >>expected &&
-   echo same line | append_cr >>expected &&
-   echo === | append_cr >>expected
-   else
-   echo first line >>expected &&
-   echo same line >>expected &&
-   echo === >>expected
-   fi &&
+   echo first line | append_cr >>expected &&
+   echo same line | append_cr >>expected &&
+   echo === | append_cr >>expected &&
echo first line | append_cr >>expected &&
echo same line | append_cr >>expected &&
echo ">>>" >>expected &&
@@ -135,15 +126,9 @@ test_expect_success 'Detect LF/CRLF conflict from addition 
of text=auto' '
echo "<<<" >expected &&
echo first line | append_cr >>expected &&
echo same line | append_cr >>expected &&
-   if test_have_prereq NATIVE_CRLF; then
-   echo === | append_cr >>expected &&
-   echo first line | append_cr >>expected &&
-   echo same line | append_cr >>expected
-   else
-   echo === >>expected &&
-   echo first line >>expected &&
-   echo same line >>expected
-   fi &&
+   echo === | append_cr >>expected &&
+   echo first line | append_cr >>expected &&
+   echo same line | append_cr >>expected &&
echo ">>>" >>expected &&
git config merge.renormalize false &&
rm -f .gitattributes &&
-- 
2.8.0.rc2.2.g1a4d45a.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 10/10] ce_compare_data() did not respect conversion

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

We define the working tree file is clean if either:

  * the result of running convert_to_git() on the working tree
contents matches what is in the index (because that would mean
doing another "git add" on the path is a no-op); OR

  * the result of running convert_to_working_tree() on the content
in the index matches what is in the working tree (because that
would mean doing another "git checkout -f" on the path is a
no-op).

Add an extra check in ce_compare_data() in read_cache.c.

When a file has CRLF in the index, and is checked out into the working tree,
but left unchabged, it is not normalized at the next commit.

Helped-by: Junio C Hamano 
Signed-off-by: Torsten Bögershausen 
---
 read-cache.c   | 61 ++
 t/t6038-merge-text-auto.sh | 14 +--
 2 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index a3ef967..48c4b31 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -156,17 +156,78 @@ void fill_stat_cache_info(struct cache_entry *ce, struct 
stat *st)
ce_mark_uptodate(ce);
 }
 
+/*
+ * Compare the data in buf with the data in the file pointed by fd and
+ * return 0 if they are identical, and non-zero if they differ.
+ */
+static int compare_with_fd(const char *input, ssize_t len, int fd)
+{
+   for (;;) {
+   char buf[1024 * 16];
+   ssize_t chunk_len, read_len;
+
+   chunk_len = sizeof(buf) < len ? sizeof(buf) : len;
+   read_len = xread(fd, buf, chunk_len ? chunk_len : 1);
+
+   if (!read_len)
+   /* EOF on the working tree file */
+   return !len ? 0 : -1;
+
+   if (!len)
+   /* we expected there is nothing left */
+   return -1;
+
+   if (memcmp(buf, input, read_len))
+   return -1;
+   input += read_len;
+   len -= read_len;
+   }
+}
+
 static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
 {
int match = -1;
int fd = open(ce->name, O_RDONLY);
 
+   /*
+* Would another "git add" on the path change what is in the
+* index for the path?
+*/
if (fd >= 0) {
unsigned char sha1[20];
if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
match = hashcmp(sha1, ce->sha1);
/* index_fd() closed the file descriptor already */
}
+   if (!match)
+   return match;
+
+   /*
+* Would another "git checkout -f" out of the index change
+* what is in the working tree file?
+*/
+   fd = open(ce->name, O_RDONLY);
+   if (fd >= 0) {
+   enum object_type type;
+   unsigned long size_long;
+   void *data = read_sha1_file(ce->sha1, , _long);
+
+   if (type == OBJ_BLOB) {
+   struct strbuf worktree = STRBUF_INIT;
+   if (convert_to_working_tree(ce->name, data,
+   size_long,
+   )) {
+   size_t size;
+   free(data);
+   data = strbuf_detach(, );
+   size_long = size;
+   }
+   if (!compare_with_fd(data, size_long, fd))
+   match = 0;
+   }
+   free(data);
+   close(fd);
+   }
return match;
 }
 
diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
index 0108ead..565daf3 100755
--- a/t/t6038-merge-text-auto.sh
+++ b/t/t6038-merge-text-auto.sh
@@ -108,9 +108,9 @@ test_expect_success 'Merge addition of text=auto' '
 
 test_expect_success 'Detect CRLF/LF conflict after setting text=auto' '
echo "<<<" >expected &&
-   echo first line | append_cr >>expected &&
-   echo same line | append_cr >>expected &&
-   echo === | append_cr >>expected &&
+   echo first line >>expected &&
+   echo same line >>expected &&
+   echo === >>expected &&
echo first line | append_cr >>expected &&
echo same line | append_cr >>expected &&
echo ">>>" >>expected &&
@@ -121,14 +121,13 @@ test_expect_success 'Detect CRLF/LF conflict after 
setting text=auto' '
fuzz_conflict file >file.fuzzy &&
compare_files expected file.fuzzy
 '
-
 test_expect_success 'Detect LF/CRLF conflict from addition of text=auto' '
echo "<<<" >expected &&
echo first line | append_cr >>expected &&
echo same line | append_cr >>expected &&
-   echo === | append_cr >>expected &&
-   echo first line | append_cr >>expected 

[PATCH v6 05/10] read-cache: factor out get_sha1_from_index() helper

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

Factor out the retrieval of the sha1 for a given path in
read_blob_data_from_index() into the function get_sha1_from_index().

This will be used in the next commit, when convert.c can do the
analyze for "text=auto" without slurping the whole blob into memory
at once.

Add a wrapper definition get_sha1_from_cache().

Signed-off-by: Torsten Bögershausen 
---
 cache.h  |  3 +++
 read-cache.c | 29 ++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index 2711048..867ceb5 100644
--- a/cache.h
+++ b/cache.h
@@ -379,6 +379,7 @@ extern void free_name_hash(struct index_state *istate);
 #define unmerge_cache_entry_at(at) unmerge_index_entry_at(_index, at)
 #define unmerge_cache(pathspec) unmerge_index(_index, pathspec)
 #define read_blob_data_from_cache(path, sz) 
read_blob_data_from_index(_index, (path), (sz))
+#define get_sha1_from_cache(path)  get_sha1_from_index (_index, (path))
 #endif
 
 enum object_type {
@@ -1040,6 +1041,8 @@ static inline void *read_sha1_file(const unsigned char 
*sha1, enum object_type *
return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
 }
 
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path);
+
 /*
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
diff --git a/read-cache.c b/read-cache.c
index d9fb78b..a3ef967 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2263,13 +2263,27 @@ int index_name_is_other(const struct index_state 
*istate, const char *name,
 
 void *read_blob_data_from_index(struct index_state *istate, const char *path, 
unsigned long *size)
 {
-   int pos, len;
+   const unsigned char *sha1;
unsigned long sz;
enum object_type type;
void *data;
 
-   len = strlen(path);
-   pos = index_name_pos(istate, path, len);
+   sha1 = get_sha1_from_index(istate, path);
+   if (!sha1)
+   return NULL;
+   data = read_sha1_file(sha1, , );
+   if (!data || type != OBJ_BLOB) {
+   free(data);
+   return NULL;
+   }
+   if (size)
+   *size = sz;
+   return data;
+}
+
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path)
+{
+   int pos = index_name_pos(istate, path, strlen(path));
if (pos < 0) {
/*
 * We might be in the middle of a merge, in which
@@ -2285,14 +2299,7 @@ void *read_blob_data_from_index(struct index_state 
*istate, const char *path, un
}
if (pos < 0)
return NULL;
-   data = read_sha1_file(istate->cache[pos]->sha1, , );
-   if (!data || type != OBJ_BLOB) {
-   free(data);
-   return NULL;
-   }
-   if (size)
-   *size = sz;
-   return data;
+   return (istate->cache[pos]->sha1);
 }
 
 void stat_validity_clear(struct stat_validity *sv)
-- 
2.8.0.rc2.2.g1a4d45a.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 04/10] convert.c: ident + core.autocrlf didn't work

2016-04-22 Thread tboegi
From: Torsten Bögershausen 

When the ident attributes is set, get_stream_filter() did not obey
core.autocrlf=true, and the file was checked out with LF.

Change the rule when a streaming filter can be used:
- if an external filter is specified, don't use a stream filter.
- if the worktree eol is CRLF and "auto" is active, don't use a stream filter.
- Otherwise the stream filter can be used.

Add test cases in t0027.

Signed-off-by: Torsten Bögershausen 
---
 convert.c| 19 +++
 t/t0027-auto-crlf.sh |  2 +-
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/convert.c b/convert.c
index f524b8d..b1614bf 100644
--- a/convert.c
+++ b/convert.c
@@ -1380,27 +1380,22 @@ static struct stream_filter *ident_filter(const 
unsigned char *sha1)
 struct stream_filter *get_stream_filter(const char *path, const unsigned char 
*sha1)
 {
struct conv_attrs ca;
-   enum crlf_action crlf_action;
struct stream_filter *filter = NULL;
 
convert_attrs(, path);
-
if (ca.drv && (ca.drv->smudge || ca.drv->clean))
-   return filter;
+   return NULL;
+
+   if (ca.crlf_action == CRLF_AUTO || ca.crlf_action == CRLF_AUTO_CRLF)
+   return NULL;
 
if (ca.ident)
filter = ident_filter(sha1);
 
-   crlf_action = ca.crlf_action;
-
-   if ((crlf_action == CRLF_BINARY) ||
-   crlf_action == CRLF_AUTO_INPUT ||
-   (crlf_action == CRLF_TEXT_INPUT))
-   filter = cascade_filter(filter, _filter_singleton);
-
-   else if (output_eol(crlf_action) == EOL_CRLF &&
-!(crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_CRLF))
+   if (output_eol(ca.crlf_action) == EOL_CRLF)
filter = cascade_filter(filter, lf_to_crlf_filter());
+   else
+   filter = cascade_filter(filter, _filter_singleton);
 
return filter;
 }
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index fd5e326..9372589 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -493,7 +493,7 @@ fi
 export CRLF_MIX_LF_CR MIX NL
 
 # Same handling with and without ident
-for id in ""
+for id in "" ident
 do
for ceol in lf crlf native
do
-- 
2.8.0.rc2.2.g1a4d45a.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] mmap(win32): avoid expensive fstat() call

2016-04-22 Thread Johannes Schindelin
On Windows, we have to emulate the fstat() call to fill out information
that takes extra effort to obtain, such as the file permissions/type.

If all we want is the file size, we can use the much cheaper
GetFileSizeEx() function (available since Windows XP).

Suggested by Philip Kelley.

Signed-off-by: Johannes Schindelin 
---
 compat/win32mmap.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/compat/win32mmap.c b/compat/win32mmap.c
index b836169..519d51f 100644
--- a/compat/win32mmap.c
+++ b/compat/win32mmap.c
@@ -2,26 +2,24 @@
 
 void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t 
offset)
 {
-   HANDLE hmap;
+   HANDLE osfhandle, hmap;
void *temp;
-   off_t len;
-   struct stat st;
+   LARGE_INTEGER len;
uint64_t o = offset;
uint32_t l = o & 0x;
uint32_t h = (o >> 32) & 0x;
 
-   if (!fstat(fd, ))
-   len = st.st_size;
-   else
+   osfhandle = (HANDLE)_get_osfhandle(fd);
+   if (!GetFileSizeEx(osfhandle, ))
die("mmap: could not determine filesize");
 
-   if ((length + offset) > len)
-   length = xsize_t(len - offset);
+   if ((length + offset) > len.QuadPart)
+   length = xsize_t(len.QuadPart - offset);
 
if (!(flags & MAP_PRIVATE))
die("Invalid usage of mmap when built with USE_WIN32_MMAP");
 
-   hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
+   hmap = CreateFileMapping(osfhandle, NULL,
prot == PROT_READ ? PAGE_READONLY : PAGE_WRITECOPY, 0, 0, NULL);
 
if (!hmap) {
-- 
2.8.1.306.gff998f2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/3] mmap(win32): avoid copy-on-write when it is unnecessary

2016-04-22 Thread Johannes Schindelin
Often we are mmap()ing read-only. In those cases, it is wasteful to map in
copy-on-write mode. Even worse: it can cause errors where we run out of
space in the page file.

So let's be extra careful to map files in read-only mode whenever
possible.

Signed-off-by: Johannes Schindelin 
---
 compat/win32mmap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/compat/win32mmap.c b/compat/win32mmap.c
index 3a39f0f..b836169 100644
--- a/compat/win32mmap.c
+++ b/compat/win32mmap.c
@@ -22,14 +22,15 @@ void *git_mmap(void *start, size_t length, int prot, int 
flags, int fd, off_t of
die("Invalid usage of mmap when built with USE_WIN32_MMAP");
 
hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
-   PAGE_WRITECOPY, 0, 0, NULL);
+   prot == PROT_READ ? PAGE_READONLY : PAGE_WRITECOPY, 0, 0, NULL);
 
if (!hmap) {
errno = EINVAL;
return MAP_FAILED;
}
 
-   temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);
+   temp = MapViewOfFileEx(hmap, prot == PROT_READ ?
+   FILE_MAP_READ : FILE_MAP_COPY, h, l, length, start);
 
if (!CloseHandle(hmap))
warning("unable to close file mapping handle");
-- 
2.8.1.306.gff998f2


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/3] Improve the mmap() emulation on Windows

2016-04-22 Thread Johannes Schindelin
Kevin David reported a problem last October where a simple git fetch
would produce this error output:

fatal: mmap failed: No error
fatal: write error: Invalid argument

The reason was that several bits of our mmap() emulation left room for
improvement. These patches fix it, and made it already into Git
for Windows v2.6.2 and have been working without problems ever since.


Johannes Schindelin (3):
  win32mmap: set errno appropriately
  mmap(win32): avoid copy-on-write when it is unnecessary
  mmap(win32): avoid expensive fstat() call

 compat/win32mmap.c | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

-- 
2.8.1.306.gff998f2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] win32mmap: set errno appropriately

2016-04-22 Thread Johannes Schindelin
It is not really helpful when a `git fetch` fails with the message:

fatal: mmap failed: No error

In the particular instance encountered by a colleague of yours truly,
the Win32 error code was ERROR_COMMITMENT_LIMIT which means that the
page file is not big enough.

Let's make the message

fatal: mmap failed: File too large

instead, which is only marginally better, but which can be associated
with the appropriate work-around: setting `core.packedGitWindowSize` to
a relatively small value.

Signed-off-by: Johannes Schindelin 
---
 compat/win32mmap.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/compat/win32mmap.c b/compat/win32mmap.c
index 80a8c9a..3a39f0f 100644
--- a/compat/win32mmap.c
+++ b/compat/win32mmap.c
@@ -24,15 +24,21 @@ void *git_mmap(void *start, size_t length, int prot, int 
flags, int fd, off_t of
hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
PAGE_WRITECOPY, 0, 0, NULL);
 
-   if (!hmap)
+   if (!hmap) {
+   errno = EINVAL;
return MAP_FAILED;
+   }
 
temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);
 
if (!CloseHandle(hmap))
warning("unable to close file mapping handle");
 
-   return temp ? temp : MAP_FAILED;
+   if (temp)
+   return temp;
+
+   errno = GetLastError() == ERROR_COMMITMENT_LIMIT ? EFBIG : EINVAL;
+   return MAP_FAILED;
 }
 
 int git_munmap(void *start, size_t length)
-- 
2.8.1.306.gff998f2


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2016, #06; Thu, 21)

2016-04-22 Thread Johannes Schindelin
Hi Junio,

On Thu, 21 Apr 2016, Junio C Hamano wrote:

> * js/am-3-merge-recursive-direct (2015-10-12) 2 commits
>  - am: make a direct call to merge_recursive
>  - merge_recursive_options: introduce the "gently" flag
> 
>  The merge_recursive_generic() function has been made a bit safer to
>  call from inside a process.  "git am -3" was taught to make a direct
>  call to the function when falling back to three-way merge.
> 
>  Being able to make a direct call would be good in general, but as a
>  performance thing, the change needs to be backed up by numbers.
> 
>  Needs review.
> 
>  I haven't gone through the "gently" change with fine toothed comb;
>  I can see that the change avoids calling die(), but I haven't made
>  sure that the program states (e.g. what's in the in-core index) are
>  adjusted sensibly when it returns to the caller instead of dying,
>  or the codepaths that used to die() are free of resource leaks.
>  The original code certainly did not care the program states at the
>  point of dying exactly because it knew it is going to exit, but now
>  they have to care, and they need to be audited.

I actually found a bug in my implementation, when I needed it in my
rebase--helper branch: at some point, we should return 128 instead of -1,
to indicate that we won't even start merging (because we would overwrite
untracked files).

I hope to find the time next week to go through the entire call graph and
verify that we are only die()ing in case if really critical errors (such
as out-of-memory, in which case we traditionally just die).

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] name-rev: include taggerdate in considering the best name

2016-04-22 Thread Johannes Schindelin
We most likely want the oldest tag that contained the commit to be
reported. So let's remember the taggerdate, and make it more important
than anything else when choosing the best name for a given commit.

Suggested by Linus Torvalds.

Note that we need to update t9903 because it tested for the old behavior
(which preferred the description "b1~1" over "tags/t2~1").

We might want to introduce a --heed-taggerdate option, and make the new
behavior dependent on that, if it turns out that some scripts rely on the
old name-rev method.

Signed-off-by: Johannes Schindelin 
---
 builtin/name-rev.c | 19 +--
 t/t9903-bash-prompt.sh |  2 +-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 092e03c..57be35f 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -10,6 +10,7 @@
 
 typedef struct rev_name {
const char *tip_name;
+   unsigned long taggerdate;
int generation;
int distance;
 } rev_name;
@@ -20,7 +21,8 @@ static long cutoff = LONG_MAX;
 #define MERGE_TRAVERSAL_WEIGHT 65535
 
 static void name_rev(struct commit *commit,
-   const char *tip_name, int generation, int distance,
+   const char *tip_name, unsigned long taggerdate,
+   int generation, int distance,
int deref)
 {
struct rev_name *name = (struct rev_name *)commit->util;
@@ -43,9 +45,12 @@ static void name_rev(struct commit *commit,
name = xmalloc(sizeof(rev_name));
commit->util = name;
goto copy_data;
-   } else if (name->distance > distance) {
+   } else if (name->taggerdate > taggerdate ||
+   (name->taggerdate == taggerdate &&
+name->distance > distance)) {
 copy_data:
name->tip_name = tip_name;
+   name->taggerdate = taggerdate;
name->generation = generation;
name->distance = distance;
} else
@@ -66,11 +71,11 @@ copy_data:
new_name = xstrfmt("%.*s^%d", (int)len, 
tip_name,
   parent_number);
 
-   name_rev(parents->item, new_name, 0,
+   name_rev(parents->item, new_name, taggerdate, 0,
distance + MERGE_TRAVERSAL_WEIGHT, 0);
} else {
-   name_rev(parents->item, tip_name, generation + 1,
-   distance + 1, 0);
+   name_rev(parents->item, tip_name, taggerdate,
+   generation + 1, distance + 1, 0);
}
}
 }
@@ -140,6 +145,7 @@ static int name_ref(const char *path, const struct 
object_id *oid, int flags, vo
struct name_ref_data *data = cb_data;
int can_abbreviate_output = data->tags_only && data->name_only;
int deref = 0;
+   unsigned long taggerdate = ULONG_MAX;
 
if (data->tags_only && !starts_with(path, "refs/tags/"))
return 0;
@@ -164,12 +170,13 @@ static int name_ref(const char *path, const struct 
object_id *oid, int flags, vo
break; /* broken repository */
o = parse_object(t->tagged->oid.hash);
deref = 1;
+   taggerdate = t->date;
}
if (o && o->type == OBJ_COMMIT) {
struct commit *commit = (struct commit *)o;
 
path = name_ref_abbrev(path, can_abbreviate_output);
-   name_rev(commit, xstrdup(path), 0, 0, deref);
+   name_rev(commit, xstrdup(path), taggerdate, 0, 0, deref);
}
return 0;
 }
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index ffbfa0e..0db4469 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -107,7 +107,7 @@ test_expect_success 'prompt - describe detached head - 
contains' '
 '
 
 test_expect_success 'prompt - describe detached head - branch' '
-   printf " ((b1~1))" >expected &&
+   printf " ((tags/t2~1))" >expected &&
git checkout b1^ &&
test_when_finished "git checkout master" &&
(
-- 
2.8.1.207.g7b140d3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: history damage in linux.git

2016-04-22 Thread Johannes Schindelin
Hi Linus,

On Thu, 21 Apr 2016, Linus Torvalds wrote:

> On Thu, Apr 21, 2016 at 11:05 AM, Jeff King  wrote:
> >
> > Sadly, neither git's internal version-sorting nor GNU's "sort -V"
> > knows that "v1.0-rc1" comes before "v1.0", so I had to rely on
> > "--sort=taggerdate".
> 
> I'm not seeing the "sadly".
> 
> I think "--sort=taggerdate" is pretty much the only sane sort there is
> for tags, unless you do a true and full topological one (ie sort based
> on by how many commits that tag encompasses, but also by how each tag
> contains another tag).

Turns out it is pretty easy to implement this in name-rev. Expect the
patch in your inbox in a minute.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 13/13] branch: do not rename a branch under bisect or rebase

2016-04-22 Thread Nguyễn Thái Ngọc Duy
The branch name in that case could be saved in rebase's head_name or
bisect's BISECT_START files. Ideally we should try to update them as
well. But it's trickier (*). Let's play safe and see if the user
complains about inconveniences before doing that.

(*) If we do it, bisect and rebase need to provide an API to rename
branches. We can't do it in worktree.c or builtin/branch.c because
when other people change rebase/bisect code, they may not be aware of
this code and accidentally break it (e.g. rename the branch file, or
refer to the branch in new files). It's a lot more work.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/branch.c| 25 +
 t/t2025-worktree-add.sh |  8 
 worktree.c  |  8 
 worktree.h  |  3 +++
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index bcde87d..b488c3f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -524,6 +524,29 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
ref_array_clear();
 }
 
+static void reject_rebase_or_bisect_branch(const char *target)
+{
+   struct worktree **worktrees = get_worktrees();
+   int i;
+
+   for (i = 0; worktrees[i]; i++) {
+   struct worktree *wt = worktrees[i];
+
+   if (!wt->is_detached)
+   continue;
+
+   if (is_worktree_being_rebased(wt, target))
+   die(_("Branch %s is being rebased at %s"),
+   target, wt->path);
+
+   if (is_worktree_being_bisected(wt, target))
+   die(_("Branch %s is being bisected at %s"),
+   target, wt->path);
+   }
+
+   free_worktrees(worktrees);
+}
+
 static void rename_branch(const char *oldname, const char *newname, int force)
 {
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = 
STRBUF_INIT;
@@ -553,6 +576,8 @@ static void rename_branch(const char *oldname, const char 
*newname, int force)
 
validate_new_branchname(newname, , force, clobber_head_ok);
 
+   reject_rebase_or_bisect_branch(oldref.buf);
+
strbuf_addf(, "Branch: renamed %s to %s",
 oldref.buf, newref.buf);
 
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 8f53944..3a22fc5 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -254,6 +254,10 @@ test_expect_success 'not allow to delete a branch under 
rebase' '
)
 '
 
+test_expect_success 'rename a branch under rebase not allowed' '
+   test_must_fail git branch -M under-rebase rebase-with-new-name
+'
+
 test_expect_success 'check out from current worktree branch ok' '
(
cd under-rebase &&
@@ -276,4 +280,8 @@ test_expect_success 'checkout a branch under bisect' '
)
 '
 
+test_expect_success 'rename a branch under bisect not allowed' '
+   test_must_fail git branch -M under-bisect bisect-with-new-name
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index aab4b95..4817d60 100644
--- a/worktree.c
+++ b/worktree.c
@@ -216,8 +216,8 @@ const char *get_worktree_git_dir(const struct worktree *wt)
return git_common_path("worktrees/%s", wt->id);
 }
 
-static int is_worktree_being_rebased(const struct worktree *wt,
-const char *target)
+int is_worktree_being_rebased(const struct worktree *wt,
+ const char *target)
 {
struct wt_status_state state;
int found_rebase;
@@ -234,8 +234,8 @@ static int is_worktree_being_rebased(const struct worktree 
*wt,
return found_rebase;
 }
 
-static int is_worktree_being_bisected(const struct worktree *wt,
- const char *target)
+int is_worktree_being_bisected(const struct worktree *wt,
+  const char *target)
 {
struct wt_status_state state;
int found_rebase;
diff --git a/worktree.h b/worktree.h
index 0da8c1f..1394909 100644
--- a/worktree.h
+++ b/worktree.h
@@ -42,6 +42,9 @@ extern void free_worktrees(struct worktree **);
 extern const struct worktree *find_shared_symref(const char *symref,
 const char *target);
 
+int is_worktree_being_rebased(const struct worktree *wt, const char *target);
+int is_worktree_being_bisected(const struct worktree *wt, const char *target);
+
 /*
  * Similar to git_path() but can produce paths for a specified
  * worktree instead of current one
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 12/13] worktree.c: check whether branch is bisected in another worktree

2016-04-22 Thread Nguyễn Thái Ngọc Duy
Similar to the rebase case, we want to detect if "HEAD" in some worktree
is being bisected because

1) we do not want to checkout this branch in another worktree, after
   bisect is done it will want to go back to this branch

2) we do not want to delete the branch is either or git bisect will
   fail to return to the (long gone) branch

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 t/t2025-worktree-add.sh | 13 +
 worktree.c  | 19 +++
 2 files changed, 32 insertions(+)

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index da54327..8f53944 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -263,4 +263,17 @@ test_expect_success 'check out from current worktree 
branch ok' '
)
 '
 
+test_expect_success 'checkout a branch under bisect' '
+   git worktree add under-bisect &&
+   (
+   cd under-bisect &&
+   git bisect start &&
+   git bisect bad &&
+   git bisect good HEAD~2 &&
+   git worktree list | grep "under-bisect.*detached HEAD" &&
+   test_must_fail git worktree add new-bisect under-bisect &&
+   ! test -d new-bisect
+   )
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index 5043756..aab4b95 100644
--- a/worktree.c
+++ b/worktree.c
@@ -234,6 +234,21 @@ static int is_worktree_being_rebased(const struct worktree 
*wt,
return found_rebase;
 }
 
+static int is_worktree_being_bisected(const struct worktree *wt,
+ const char *target)
+{
+   struct wt_status_state state;
+   int found_rebase;
+
+   memset(, 0, sizeof(state));
+   found_rebase = wt_status_check_bisect(wt, ) &&
+   state.branch &&
+   starts_with(target, "refs/heads/") &&
+   !strcmp(state.branch, target + strlen("refs/heads/"));
+   free(state.branch);
+   return found_rebase;
+}
+
 /*
  * note: this function should be able to detect shared symref even if
  * HEAD is temporarily detached (e.g. in the middle of rebase or
@@ -261,6 +276,10 @@ const struct worktree *find_shared_symref(const char 
*symref,
existing = wt;
break;
}
+   if (is_worktree_being_bisected(wt, target)) {
+   existing = wt;
+   break;
+   }
}
 
strbuf_reset();
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >