Re: difflame

2017-02-02 Thread Edmundo Carmona Antoranz
On Thu, Feb 2, 2017 at 10:46 PM, Edmundo Carmona Antoranz
 wrote:
> I have been "scripting around git blame --reverse" for some days now.
> Mind taking a look? I've been working on blame-deletions for this.

blame-deletions branch, that is

Sorry for the previous top-posting.


Re: difflame

2017-02-02 Thread Edmundo Carmona Antoranz
I have been "scripting around git blame --reverse" for some days now.
Mind taking a look? I've been working on blame-deletions for this.

22:41 $ ../difflame/difflame.py HEAD~5 HEAD
diff --git a/file b/file
index b414108..051c298 100644
--- a/file
+++ b/file
@@ -1,6 +1,9 @@
^1513353 (Edmundo 2017-02-02 22:41:45 -0600 1) 1
f20952eb (Edmundo 2017-02-02 22:41:45 -0600 2) 2
bb04dc7c (Edmundo 2017-02-02 22:41:45 -0600 3) 3
-de3c07b (Edmundo 2017-02-02 22:41:47 -060 4) 4
058ea125 (Edmundo 2017-02-02 22:41:45 -0600 4) 5
85fc6b81 (Edmundo 2017-02-02 22:41:45 -0600 5) 6
+2cd990a6 (Edmundo 2017-02-02 22:41:45 -0600 6) 7
+ab0be970 (Edmundo 2017-02-02 22:41:45 -0600 7) 8
+944452c0 (Edmundo 2017-02-02 22:41:45 -0600 8) 9
+6641edb0 (Edmundo 2017-02-02 22:41:45 -0600 9) 10


$ git show de3c07b
commit de3c07bc21a83472d5c5ddf172dcb742665924dd (HEAD -> master)
Author: Edmundo 
Date:   Thu Feb 2 22:41:47 2017 -0600

   drop 4

diff --git a/file b/file
index f00c965..051c298 100644
--- a/file
+++ b/file
@@ -1,7 +1,6 @@
1
2
3
-4
5
6
7


Next step: solve the
find-real-deletion-revision-when-there-are-multiple-child-nodes
problem and let me read the discussion around git blame --reverse.

Thanks in advance.

On Mon, Jan 30, 2017 at 8:37 PM, Edmundo Carmona Antoranz
 wrote:
> Maybe a little work on blame to get the actual revision where some
> lines were "deleted"?
>
> Something like git blame --blame-deletion that could be based on --reverse?
>
> On Mon, Jan 30, 2017 at 7:35 PM, Edmundo Carmona Antoranz
>  wrote:
>> I'm thinking of something like this:
>>
>> Say I just discovered a problem in a file I want to see who worked
>> on it since some revision that I know is working fine (or even
>> something as generic as HEAD~100..). It could be a number of people
>> with different revisions. I would diff to see what changed and
>> blame the added lines (blame reverse to try to get as close as
>> possible with a single command in case I want to see what happened
>> with something that was deleted). If I could get blame information of
>> added/deleted lines in a single run, that would help a lot.
>>
>> Lo and behold: difflame.
>>
>>
>>
>> On Mon, Jan 30, 2017 at 5:26 PM, Jeff King  wrote:
>>> On Mon, Jan 30, 2017 at 01:08:41PM -0800, Junio C Hamano wrote:
>>>
 Jeff King  writes:

 > On Tue, Jan 17, 2017 at 11:24:02PM -0600, Edmundo Carmona Antoranz wrote:
 >
 >> For a very long time I had wanted to get the output of diff to include
 >> blame information as well (to see when something was added/removed).
 >
 > This is something I've wanted, too. The trickiest part, though, is
 > blaming deletions, because git-blame only tracks the origin of content,
 > not the origin of a change.

 Hmph, this is a comment without looking at what difflame does
 internally, so you can ignore me if I am misunderstood what problem
 you are pointing out, but I am not sure how "tracks the origin of
 content" could be a problem.

 If output from "git show" says this:

   --- a/file
   +++ b/file
   @@ -1,5 +1,6 @@
a
b
   -c
   +C
   +D
d
e

 in order to annotate lines 'a', 'b', 'd', and 'e' for their origin,
 you would run 'blame' on the commit the above output was taken from
 (or its parent---they are in the context so either would be OK).

 You know where 'C' and 'D' came from already.  It's the commit you
 are feeding "git show".
>>>
>>> I think the point (or at least as I understand it) is that the diff is
>>> not "git show" for a particular commit. It could be part of a much
>>> larger diff that covers many commits.
>>>
>>> As a non-hypothetical instance, I have a fork of git.git that has
>>> various enhancements. I want to feed those enhancements upstream. I need
>>> to know which commits should be cherry-picked to get those various
>>> enhancements.
>>>
>>> Looking at "log origin..fork" tells me too many commits, because it
>>> includes ones which aren't useful anymore. Either because they already
>>> went upstream, or because they were cherry-picked to the fork and their
>>> upstream counterparts merged (or even equivalent commits made upstream
>>> that obsoleted the features).
>>>
>>> Looking at "git diff origin fork" tells me what the actual differences
>>> are, but it doesn't show me which commits are responsible for them.
>>>
>>> I can "git blame" each individual line of the diff (starting with "fork"
>>> as the tip), but that doesn't work for lines that no longer exist (i.e.,
>>> when the interesting change is a deletion).
>>>
 In order to run blame to find where 'c' came from, you need to start
 at the _parent_ of the commit the above output came from, and the
 hunk header shows which line range to find the final 'c'.
>>>
>>> So perhaps that explains my comment more. "blame" is not good for
>>> finding which commit took away a lin

Re: How to use git show's "%<([,trunc|ltrunc|mtrunc])"?

2017-02-02 Thread Hilco Wijbenga
On 2 February 2017 at 20:19, G. Sylvie Davies  wrote:
> On Thu, Feb 2, 2017 at 9:51 AM, Hilco Wijbenga  
> wrote:
>> Hi all,
>>
>> I'm trying to get the committer date printed in a custom fashion.
>> Using "%cI" gets me close:
>>
>> $ git show --format="%cI | %an" master | head -n 1
>> 2017-01-31T17:02:13-08:00 | Hilco Wijbenga
>>
>> I would like to get rid of the "-08:00" bit at the end of the
>> timestamp. According to the "git show" manual I should be able to use
>> "%<([,trunc|ltrunc|mtrunc])" to drop that last part.
>>
>> $ git show --format="%<(19,trunc)cI | %an" master | head -n 1
>> cI | Hilco Wijbenga
>>
>> Mmm, it seems to be recognized as a valid "something" but it's not
>> working as I had expected. :-) I tried several other versions of this
>> but no luck. Clearly, I'm misunderstanding the format description. How
>> do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?
>>
>
> Will this work for you?
>
> $ git show -s --pretty='%cd | %an' --date=format:%FT%R:%S
> 2017-02-02T10:01:36 | G. Sylvie Davies

Ah, that does indeed do exactly what I want. Thank you.

> I have no idea how portable this might be.  As "git help log" says:
>
>  --date=format:...  feeds the format ...  to your system
> strftime. Use --date=format:%c to show the date in your system
> locale’s preferred format. See the strftime manual for a complete list
> of format placeholders.

It should be fine for my purposes.

Any idea why "%<(19,trunc)cl" doesn't work? (Your solution solves my
original problem perfectly but I'd like to understand how I'm
misreading the spec.)


Re: How to use git show's "%<([,trunc|ltrunc|mtrunc])"?

2017-02-02 Thread G. Sylvie Davies
On Thu, Feb 2, 2017 at 9:51 AM, Hilco Wijbenga  wrote:
> Hi all,
>
> I'm trying to get the committer date printed in a custom fashion.
> Using "%cI" gets me close:
>
> $ git show --format="%cI | %an" master | head -n 1
> 2017-01-31T17:02:13-08:00 | Hilco Wijbenga
>
> I would like to get rid of the "-08:00" bit at the end of the
> timestamp. According to the "git show" manual I should be able to use
> "%<([,trunc|ltrunc|mtrunc])" to drop that last part.
>
> $ git show --format="%<(19,trunc)cI | %an" master | head -n 1
> cI | Hilco Wijbenga
>
> Mmm, it seems to be recognized as a valid "something" but it's not
> working as I had expected. :-) I tried several other versions of this
> but no luck. Clearly, I'm misunderstanding the format description. How
> do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?
>

Will this work for you?

$ git show -s --pretty='%cd | %an' --date=format:%FT%R:%S
2017-02-02T10:01:36 | G. Sylvie Davies


I have no idea how portable this might be.  As "git help log" says:

 --date=format:...  feeds the format ...  to your system
strftime. Use --date=format:%c to show the date in your system
locale’s preferred format. See the strftime manual for a complete list
of format placeholders.



- Sylvie


Re: [PATCH 00/12] completion: speed up refs completion

2017-02-02 Thread Jacob Keller
On Thu, Feb 2, 2017 at 6:53 PM, SZEDER Gábor  wrote:
> This series speeds up refs completion for large number of refs, partly
> by giving up disambiguating ambiguous refs (patch 6) and partly by
> eliminating most of the shell processing between 'git for-each-ref'
> and 'ls-remote' and Bash's completion facility.  The rest is a bit of
> preparatory reorganization, cleanup and bugfixes.
>
> The last patch touches the ZSH wrapper, too.  By a lucky educated
> guess I managed to get it work on the first try, but I don't really
> know what I've actually done, so...  ZSH users, please have a closer
> look.
>
> At the end of this series refs completion from a local repository is
> as fast as it can possibly get, at least as far as the completion
> script is concerned, because it basically does nothing anymore :)  All
> it does is run 'git for-each-ref' with assorted options to do all the
> work, and feed its output directly, without any processing into Bash's
> COMPREPLY array.  There is still room for improvements in the code
> paths using 'git ls-remote', but for that we would need enhancements
> to 'ls-remote'.
>
> It goes on top of the __gitdir() improvements series I just posted at:
>
>   http://public-inbox.org/git/20170203024829.8071-1-szeder@gmail.com/T/
>
> This series is also available at:
>
>   https://github.com/szeder/git completion-refs-speedup
>

Nice! This is something i've been bothered by in the past since
completion would take a rather long time!

Regards,
Jake


possible bug: inconsistent CLI behaviour for empty user.name

2017-02-02 Thread bs . x . ttp
The problem is that GIT accepts a user.name of " " for some operations (for 
example when doing a simple "git commit"), but does require a "non-empty" 
user.name for others (like git commit --amend and git rebase). In case of the 
latter commands GIT fails with the message "fatal: empty ident name (for 
) not allowed".

As people tend to do simple commits first, before amending or rebasing 
something, they may have to change their name after some dozen of commits which 
doesn't look nice.

This is certainly not a big issue, but it turns out to be quite annoying and 
I've already rewritten the history of a GIT repository once because of it, so 
that all my commits had the same author.

Proposed solution: GIT's requirements for user.name should not depend on the 
operation. Either user.name should be enforced to be non-empty everywhere or an 
empty user.name should be accepted everywhere. Perhaps filling out one of 
user.name and user.email could be sufficient.







[PATCH 04/12] completion: support excluding full refs

2017-02-02 Thread SZEDER Gábor
Commit 49416ad22 (completion: support excluding refs, 2016-08-24) made
possible to complete short refs with a '^' prefix.

Extend the support to full refs to make completing '^refs/...' work.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash |  8 
 t/t9902-completion.sh  | 31 +++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 67a03cfd4..63e803154 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -387,6 +387,10 @@ __git_refs ()
fi
 
if [ "$list_refs_from" = path ]; then
+   if [[ "$cur_" == ^* ]]; then
+   pfx="^"
+   cur_=${cur_#^}
+   fi
case "$cur_" in
refs|refs/*)
format="refname"
@@ -394,10 +398,6 @@ __git_refs ()
track=""
;;
*)
-   if [[ "$cur_" == ^* ]]; then
-   pfx="^"
-   cur_=${cur_#^}
-   fi
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
if [ -e "$dir/$i" ]; then echo $pfx$i; fi
done
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 8fe748839..7b42a686c 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -806,6 +806,37 @@ test_expect_success '__git_refs - after --opt= - full 
refs' '
test_cmp expected "$actual"
 '
 
+test_expect_success '__git refs - exluding refs' '
+   cat >expected <<-EOF &&
+   ^HEAD
+   ^master
+   ^matching-branch
+   ^other/branch-in-other
+   ^other/master-in-other
+   ^matching-tag
+   EOF
+   (
+   cur=^ &&
+   __git_refs >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success '__git refs - exluding full refs' '
+   cat >expected <<-EOF &&
+   ^refs/heads/master
+   ^refs/heads/matching-branch
+   ^refs/remotes/other/branch-in-other
+   ^refs/remotes/other/master-in-other
+   ^refs/tags/matching-tag
+   EOF
+   (
+   cur=^refs/ &&
+   __git_refs >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
 test_expect_success '__git_complete_refs - simple' '
sed -e "s/Z$//g" >expected <<-EOF &&
HEAD Z
-- 
2.11.0.555.g967c1bcb3



[PATCH 05/12] completion: don't disambiguate tags and branches

2017-02-02 Thread SZEDER Gábor
When the completion script has to list only tags or only branches, it
uses the 'git for-each-ref' format 'refname:short', which makes sure
that all listed tags and branches are unambiguous.  However,
disambiguating tags and branches in these cases is wrong, because:

  - __git_tags(), the helper function listing possible tagname
arguments for 'git tag', lists an ambiguous tag
'refs/tags/ambiguous' as 'tags/ambiguous'.  Its only consumer,
'git tag' expects its tagname argument to be under 'refs/tags/',
thus it interprets that abgiguous tag as
'refs/tags/tags/ambiguous'.  Clearly wrong.

  - __git_heads() lists possible branchname arguments for 'git branch'
and possible 'branch.' configuration subsections.
Both of these expect branchnames to be under 'refs/heads/' and
misinterpret a disambiguated branchname like 'heads/ambiguous'.

Furthermore, disambiguation involves several stat() syscalls for each
tag or branch, thus comes at a steep cost especially on Windows and/or
when there are a lot of tags or branches to be listed.

Use the 'git for-each-ref' format 'refname:strip=2' instead of
'refname:short' to avoid harmful disambiguation of tags and branches
in __git_tags() and __git_heads().

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 63e803154..19799e3ba 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -340,12 +340,12 @@ __git_index_files ()
 
 __git_heads ()
 {
-   __git for-each-ref --format='%(refname:short)' refs/heads
+   __git for-each-ref --format='%(refname:strip=2)' refs/heads
 }
 
 __git_tags ()
 {
-   __git for-each-ref --format='%(refname:short)' refs/tags
+   __git for-each-ref --format='%(refname:strip=2)' refs/tags
 }
 
 # Lists refs from the local (by default) or from a remote repository.
-- 
2.11.0.555.g967c1bcb3



[PATCH 08/12] completion: let 'for-each-ref' strip the remote name from remote branches

2017-02-02 Thread SZEDER Gábor
The code listing unique remote branches for 'git checkout's tracking
DWIMery uses a shell parameter expansion in a loop iterating over each
listed ref to remove the remote's name from the remote branches, i.e.
the leading path component from the short ref.  When listing refs from
a configured remote repository, '| sed s///' is used for the same
purpose.

Let 'git for-each-ref' strip one more leading path component from the
refs, i.e. use the format 'refname:strip=3' instead of '=2', making
that parameter expansion and 'sed' execution unnecessary.

This speeds up refs completion for 'git checkout'.  Uniquely
completing a branch for 'git checkout maste' in a repo with 100k
remote branches, all packed, best of five:

  On Linux, near the beginning of this series, for reference:

$ time __git_complete_refs --cur=maste --track

real0m8.185s
user0m6.896s
sys 0m1.616s

  Before this patch:

real0m2.714s
user0m2.344s
sys 0m0.436s

  After:

real0m1.993s
user0m1.740s
sys 0m0.304s

  On Windows, near the beginning:

real1m8.421s
user0m7.591s
sys 0m3.557s

  Before this patch:

real0m8.191s
user0m4.638s
sys 0m2.918s

  After:

real0m6.187s
user0m3.358s
sys 0m2.121s

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 615292f8b..8f1203025 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -414,11 +414,10 @@ __git_refs ()
# Try to find a remote branch that matches the 
completion word
# but only output if the branch name is unique
local ref entry
-   __git for-each-ref --shell 
--format="ref=%(refname:strip=2)" \
+   __git for-each-ref --shell 
--format="ref=%(refname:strip=3)" \
"refs/remotes/" | \
while read -r entry; do
eval "$entry"
-   ref="${ref#*/}"
if [[ "$ref" == "$cur_"* ]]; then
echo "$ref"
fi
@@ -439,9 +438,9 @@ __git_refs ()
*)
if [ "$list_refs_from" = remote ]; then
echo "HEAD"
-   __git for-each-ref --format="%(refname:strip=2)" \
+   __git for-each-ref --format="%(refname:strip=3)" \
"refs/remotes/$remote/$cur_*" \
-   "refs/remotes/$remote/$cur_*/**" | sed -e 
"s#^$remote/##"
+   "refs/remotes/$remote/$cur_*/**"
else
__git ls-remote "$remote" HEAD \
"refs/tags/$cur_*" "refs/heads/$cur_*" \
-- 
2.11.0.555.g967c1bcb3



[PATCH 06/12] completion: don't disambiguate short refs

2017-02-02 Thread SZEDER Gábor
When the completion script lists short refs it does so using the 'git
for-each-ref' format 'refname:short', which makes sure that all listed
refs are unambiguous.  While disambiguating refs is technically
correct in this case, as opposed to the cases discussed in the
previous patch, this disambiguation involves several stat() syscalls
for each ref, thus, unfortunately, comes at a steep cost especially on
Windows and/or when there are a lot of refs to be listed.  A user of
Git for Windows reported[1] 'git checkout ' taking ~11 seconds in
a repository with just about 4000 refs.

However, it's questionable whether ambiguous refs are really that bad
to justify that much extra cost:

  - Ambiguous refs are not that common,
  - even if a repository contains ambiguous refs, they only hurt when
the user actually happens to want to do something with one of the
ambiguous refs, and
  - the issue can be easily circumvented by renaming those ambiguous
refs.

  - On the other hand, apparently not that many refs are needed to
make refs completion unacceptably slow on Windows,
  - and this slowness bites each and every time the user attempts refs
completion, even when the repository doesn't contain any ambiguous
refs.
  - Furthermore, circumventing the issue might not be possible or
might be considerably more difficult and requires various
trade-offs (e.g. working in a repository with only a few selected
important refs while keeping a separate repository with all refs
for reference).

Arguably, in this case the benefits of technical correctness are
rather minor compared to the price we pay for it, and we are better
off opting for performance over correctness.

Use the 'git for-each-ref' format 'refname:strip=2' to list short refs
to spare the substantial cost of disambiguating.

This speeds up refs completion considerably.  Uniquely completing a
branch in a repository with 100k local branches, all packed, best of
five:

  On Linux, before:

$ time __git_complete_refs --cur=maste

real0m1.662s
user0m1.368s
sys 0m0.296s

  After:

real0m0.831s
user0m0.808s
sys 0m0.028s

  On Windows, before:

real0m12.457s
user0m1.016s
sys 0m0.092s

  After:

real0m1.480s
user0m1.031s
sys 0m0.060s

[1] - https://github.com/git-for-windows/git/issues/524

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 19799e3ba..d55eadfd1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -401,7 +401,7 @@ __git_refs ()
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
if [ -e "$dir/$i" ]; then echo $pfx$i; fi
done
-   format="refname:short"
+   format="refname:strip=2"
refs="refs/tags refs/heads refs/remotes"
;;
esac
@@ -412,7 +412,7 @@ __git_refs ()
# Try to find a remote branch that matches the 
completion word
# but only output if the branch name is unique
local ref entry
-   __git for-each-ref --shell 
--format="ref=%(refname:short)" \
+   __git for-each-ref --shell 
--format="ref=%(refname:strip=2)" \
"refs/remotes/" | \
while read -r entry; do
eval "$entry"
@@ -437,7 +437,7 @@ __git_refs ()
*)
if [ "$list_refs_from" = remote ]; then
echo "HEAD"
-   __git for-each-ref --format="%(refname:short)" \
+   __git for-each-ref --format="%(refname:strip=2)" \
"refs/remotes/$remote/" | sed -e "s#^$remote/##"
else
__git ls-remote "$remote" HEAD \
-- 
2.11.0.555.g967c1bcb3



[PATCH 12/12] completion: fill COMPREPLY directly when completing refs

2017-02-02 Thread SZEDER Gábor
__gitcomp_nl() iterates over all the possible completion words it gets
as argument

  - filtering matching words,
  - appending a trailing space to each matching word (in all but two
cases),
  - prepending a prefix to each matching word (when completing words
after e.g. '--option=' or 'master..'), and
  - adding each matching word to the COMPREPLY array.

This takes a while when a lot of refs are passed to __gitcomp_nl().

The previous changes in this series ensure that __git_refs() lists
only refs matching the current word to be completed, making a second
filtering in __gitcomp_nl() redundant.

Adding the necessary prefix and suffix could be done in __git_refs()
as well:

  - When refs come from 'git for-each-ref', then that prefix and
suffix could be added much more efficiently using a 'git
for-each-ref' format containing said prefix and suffix.
  - When refs come from 'git ls-remote', then that prefix and suffix
can be added in the shell loop that has to process 'git
ls-remote's output anyway.
  - Finally, the prefix and suffix can be added to that handful of
potentially matching symbolic and pseudo refs right away in the
shell loop listing them.

And then all what is still left to do is to assign a bunch of
newline-separated words to a shell array, which can be done without a
shell loop iterating over each word, basically making all of
__gitcomp_nl() unnecessary for refs completion.

Add the helper function __gitcomp_direct() to fill the COMPREPLY array
with prefiltered and preprocessed words without any additional
processing, without a shell loop, with just one single compound
assignment.  Modify __git_refs() to accept prefix and suffix
parameters and add them to each and every listed ref.  Modify
__git_complete_refs() to pass the prefix and suffix parameters to
__git_refs() and to feed __git_refs()'s output to __gitcomp_direct()
instead of __gitcomp_nl().

This speeds up refs completion when there are a lot of refs matching
the current word to be completed.  Listing all branches for completion
in a repo with 100k local branches, all packed, best of five:

  On Linux, near the beginning of this series, for reference:

$ time __git_complete_refs

real0m2.028s
user0m1.692s
sys 0m0.344s

  Before this patch:

real0m1.135s
user0m1.112s
sys 0m0.024s

  After:

real0m0.367s
user0m0.352s
sys 0m0.020s

  On Windows, near the beginning:

real0m13.078s
user0m1.609s
sys 0m0.060s

  Before this patch:

real0m2.093s
user0m1.641s
sys 0m0.060s

  After:

real0m0.683s
user0m0.203s
sys 0m0.076s

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 54 --
 contrib/completion/git-completion.zsh  |  9 ++
 t/t9902-completion.sh  | 16 ++
 3 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 0ad02cec6..dbbb62f5f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -213,6 +213,20 @@ _get_comp_words_by_ref ()
 }
 fi
 
+# Fills the COMPREPLY array with prefiltered words without any additional
+# processing.
+# Callers must take care of providing only words that match the current word
+# to be completed and adding any prefix and/or suffix (trailing space!), if
+# necessary.
+# 1: List of newline-separated matching completion words, complete with
+#prefix and suffix.
+__gitcomp_direct ()
+{
+   local IFS=$'\n'
+
+   COMPREPLY=($1)
+}
+
 __gitcompappend ()
 {
local x i=${#COMPREPLY[@]}
@@ -354,17 +368,19 @@ __git_tags ()
 #Can be the name of a configured remote, a path, or a URL.
 # 2: In addition to local refs, list unique branches from refs/remotes/ for
 #'git checkout's tracking DWIMery (optional; ignored, if set but empty).
-# 3: Currently ignored.
+# 3: A prefix to be added to each listed ref (optional).
 # 4: List only refs matching this word instead of the current word being
-#completed (optional).
+#completed (optional; NOT ignored, if empty, but lists all refs).
+# 5: A suffix to be appended to each listed ref (optional; ignored, if set
+#but empty).
 #
 # Use __git_complete_refs() instead.
 __git_refs ()
 {
local i hash dir track="${2-}"
local list_refs_from=path remote="${1-}"
-   local format refs pfx
-   local cur_="${4-$cur}"
+   local format refs
+   local pfx="${3-}" cur_="${4-$cur}" sfx="${5-}"
 
__git_find_repo_path
dir="$__git_repo_path"
@@ -389,7 +405,7 @@ __git_refs ()
 
if [ "$list_refs_from" = path ]; then
if [[ "$cur_" == ^* ]]; then
-   pfx="^"
+   pfx="$pfx^"
cur_=${cur_#^}
fi
case "$cur_" in
@@ -402,

[PATCH 03/12] completion: support completing full refs after '--option=refs/'

2017-02-02 Thread SZEDER Gábor
Completing full refs currently only works when the full ref stands on
in its own on the command line, but doesn't work when the current word
to be completed contains a prefix before the full ref, e.g.
'--option=refs/' or 'master..refs/bis'.

The reason is that __git_refs() looks at the current word to be
completed ($cur) as a whole to decide whether it has to list full (if
it starts with 'refs/') or short refs (otherwise).  However, $cur also
holds said '--option=' or 'master..' prefixes, which of course throw
off this decision.  Luckily, the default action is to list short refs,
that's why completing short refs happens to work even after a
'master..' prefix and similar cases.

Pass only the ref part of the current word to be completed to
__git_refs() as a new positional parameter, so it can make the right
decision even if the whole current word contains some kind of a
prefix.

Make this new parameter the 4. positional parameter and leave the 3.
as an ignored placeholder for now (it will be used later in this patch
series).

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 21 ++---
 t/t9902-completion.sh  | 31 +++
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 7f19e2a4f..67a03cfd4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -354,6 +354,8 @@ __git_tags ()
 #Can be the name of a configured remote, a path, or a URL.
 # 2: In addition to local refs, list unique branches from refs/remotes/ for
 #'git checkout's tracking DWIMery (optional; ignored, if set but empty).
+# 3: Currently ignored.
+# 4: The current ref to be completed (optional).
 #
 # Use __git_complete_refs() instead.
 __git_refs ()
@@ -361,6 +363,7 @@ __git_refs ()
local i hash dir track="${2-}"
local list_refs_from=path remote="${1-}"
local format refs pfx
+   local cur_="${4-$cur}"
 
__git_find_repo_path
dir="$__git_repo_path"
@@ -384,14 +387,17 @@ __git_refs ()
fi
 
if [ "$list_refs_from" = path ]; then
-   case "$cur" in
+   case "$cur_" in
refs|refs/*)
format="refname"
-   refs="${cur%/*}"
+   refs="${cur_%/*}"
track=""
;;
*)
-   [[ "$cur" == ^* ]] && pfx="^"
+   if [[ "$cur_" == ^* ]]; then
+   pfx="^"
+   cur_=${cur_#^}
+   fi
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
if [ -e "$dir/$i" ]; then echo $pfx$i; fi
done
@@ -411,16 +417,16 @@ __git_refs ()
while read -r entry; do
eval "$entry"
ref="${ref#*/}"
-   if [[ "$ref" == "$cur"* ]]; then
+   if [[ "$ref" == "$cur_"* ]]; then
echo "$ref"
fi
done | sort | uniq -u
fi
return
fi
-   case "$cur" in
+   case "$cur_" in
refs|refs/*)
-   __git ls-remote "$remote" "$cur*" | \
+   __git ls-remote "$remote" "$cur_*" | \
while read -r hash i; do
case "$i" in
*^{}) ;;
@@ -475,7 +481,8 @@ __git_complete_refs ()
shift
done
 
-   __gitcomp_nl "$(__git_refs "$remote" "$track")" "$pfx" "$cur_" "$sfx"
+   __gitcomp_nl "$(__git_refs "$remote" "$track" "" "$cur_")" \
+   "$pfx" "$cur_" "$sfx"
 }
 
 # __git_refs2 requires 1 argument (to pass to __git_refs)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 50c534072..8fe748839 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -775,6 +775,37 @@ test_expect_success '__git_refs - unique remote branches 
for git checkout DWIMer
test_cmp expected "$actual"
 '
 
+test_expect_success '__git_refs - after --opt=' '
+   cat >expected <<-EOF &&
+   HEAD
+   master
+   matching-branch
+   other/branch-in-other
+   other/master-in-other
+   matching-tag
+   EOF
+   (
+   cur="--opt=" &&
+   __git_refs "" "" "" "" >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - after --opt= - full refs' '
+   cat >expected <<-EOF &&
+   refs/heads/master
+   refs/heads/matching-branch
+   refs/remotes/other/branch-in-other
+   refs/remotes/other/master-in-other
+   refs/tags/matching-tag
+   EOF
+   (
+   cur=

[PATCH 11/12] completion: list only matching symbolic and pseudorefs when completing refs

2017-02-02 Thread SZEDER Gábor
The previous changes in this series ensure that __git_refs() lists
only refs that match the current word to be completed, but
non-matching symbolic and pseudo refs still show up in its output.

Filter out non-matching symbolic and pseudo refs as well, so anything
__git_refs() outputs matches the current word to be completed, as it
will allow us to optimize how refs are placed into the COMPREPLY
array.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 20 
 t/t9902-completion.sh  |  4 
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 9f5a6f44e..0ad02cec6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -355,7 +355,8 @@ __git_tags ()
 # 2: In addition to local refs, list unique branches from refs/remotes/ for
 #'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 # 3: Currently ignored.
-# 4: The current ref to be completed (optional).
+# 4: List only refs matching this word instead of the current word being
+#completed (optional).
 #
 # Use __git_complete_refs() instead.
 __git_refs ()
@@ -399,7 +400,12 @@ __git_refs ()
;;
*)
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
-   if [ -e "$dir/$i" ]; then echo $pfx$i; fi
+   case "$i" in
+   $cur_*) if [ -e "$dir/$i" ]; then
+   echo $pfx$i
+   fi
+   ;;
+   esac
done
format="refname:strip=2"
refs=("refs/tags/$cur_*" "refs/tags/$cur_*/**"
@@ -432,12 +438,18 @@ __git_refs ()
;;
*)
if [ "$list_refs_from" = remote ]; then
-   echo "HEAD"
+   case "HEAD" in
+   $cur_*) echo "HEAD" ;;
+   esac
__git for-each-ref --format="%(refname:strip=3)" \
"refs/remotes/$remote/$cur_*" \
"refs/remotes/$remote/$cur_*/**"
else
-   __git ls-remote "$remote" HEAD \
+   local query_symref
+   case "HEAD" in
+   $cur_*) query_symref="HEAD" ;;
+   esac
+   __git ls-remote "$remote" $query_symref \
"refs/tags/$cur_*" "refs/heads/$cur_*" \
"refs/remotes/$cur_*" |
while read -r hash i; do
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 499be5879..5e06acc17 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -847,7 +847,6 @@ test_expect_success 'setup for filtering matching refs' '
 
 test_expect_success '__git_refs - only matching refs' '
cat >expected <<-EOF &&
-   HEAD
matching-branch
matching/branch
matching-tag
@@ -874,7 +873,6 @@ test_expect_success '__git_refs - only matching refs - full 
refs' '
 
 test_expect_success '__git_refs - only matching refs - remote on local file 
system' '
cat >expected <<-EOF &&
-   HEAD
master-in-other
matching/branch-in-other
EOF
@@ -887,7 +885,6 @@ test_expect_success '__git_refs - only matching refs - 
remote on local file syst
 
 test_expect_success '__git_refs - only matching refs - configured remote' '
cat >expected <<-EOF &&
-   HEAD
master-in-other
matching/branch-in-other
EOF
@@ -912,7 +909,6 @@ test_expect_success '__git_refs - only matching refs - 
remote - full refs' '
 
 test_expect_success '__git_refs - only matching refs - checkout DWIMery' '
cat >expected <<-EOF &&
-   HEAD
matching-branch
matching/branch
matching-tag
-- 
2.11.0.555.g967c1bcb3



[PATCH 10/12] completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery

2017-02-02 Thread SZEDER Gábor
When listing unique remote branches for 'git checkout's tracking
DWIMery, __git_refs() runs the classic '... |sort |uniq -u' pattern to
filter out duplicate remote branches.

Let 'git for-each-ref' do the sorting, sparing the overhead of
fork()+exec()ing 'sort' and a stage in the pipeline where potentially
relatively large amount of data can be passed between two subsequent
pipeline stages.

This speeds up refs completion for 'git checkout' a bit when a lot of
remote branches match the current word to be completed.  Listing a
single local and 100k remote branches, all packed, best of five:

  On Linux, before:

$ time __git_complete_refs --track

real0m1.856s
user0m1.816s
sys 0m0.060s

  After:

real0m1.550s
user0m1.512s
sys 0m0.060s

  On Windows, before:

real0m3.128s
user0m2.155s
sys 0m0.183s

  After:

real0m2.781s
user0m1.826s
sys 0m0.136s

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e2c4794f3..9f5a6f44e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -414,8 +414,9 @@ __git_refs ()
# Try to find a remote branch that matches the 
completion word
# but only output if the branch name is unique
__git for-each-ref --format="%(refname:strip=3)" \
+   --sort="refname:strip=3" \
"refs/remotes/*/$cur_*" 
"refs/remotes/*/$cur_*/**" | \
-   sort | uniq -u
+   uniq -u
fi
return
fi
-- 
2.11.0.555.g967c1bcb3



[PATCH 09/12] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery

2017-02-02 Thread SZEDER Gábor
The code listing unique remote branches for 'git checkout's tracking
DWIMery outputs only remote branches that match the current word to be
completed, but the filtering is done in a shell loop iterating over
all remote refs.

Let 'git for-each-ref' do the filtering, as it can do so much more
efficiently and we can remove that shell loop entirely.

This speeds up refs completion for 'git checkout' considerably when
there are a lot of non-matching remote refs to be filtered out.
Uniquely completing a branch in a repository with 100k remote
branches, all packed, best of five:

  On Linux, before:

$ time __git_complete_refs --cur=maste --track

real0m1.993s
user0m1.740s
sys 0m0.304s

  After:

real0m0.266s
user0m0.248s
sys 0m0.012s

  On Windows, before:

real0m6.187s
user0m3.358s
sys 0m2.121s

  After:

real0m0.750s
user0m0.015s
sys 0m0.090s

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 8f1203025..e2c4794f3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -413,15 +413,9 @@ __git_refs ()
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the 
completion word
# but only output if the branch name is unique
-   local ref entry
-   __git for-each-ref --shell 
--format="ref=%(refname:strip=3)" \
-   "refs/remotes/" | \
-   while read -r entry; do
-   eval "$entry"
-   if [[ "$ref" == "$cur_"* ]]; then
-   echo "$ref"
-   fi
-   done | sort | uniq -u
+   __git for-each-ref --format="%(refname:strip=3)" \
+   "refs/remotes/*/$cur_*" 
"refs/remotes/*/$cur_*/**" | \
+   sort | uniq -u
fi
return
fi
-- 
2.11.0.555.g967c1bcb3



[PATCH 07/12] completion: let 'for-each-ref' and 'ls-remote' filter matching refs

2017-02-02 Thread SZEDER Gábor
When completing refs, several __git_refs() code paths list all the
refs from the refs/{heads,tags,remotes}/ hierarchy and then
__gitcomp_nl() iterates over those refs in a shell loop to filter out
refs not matching the current word to be completed.  This comes with a
considerable performance penalty when a repository contains a lot of
refs but the current word can be uniquely completed or when only a
handful of refs match the current word.

Specify appropriate globbing patterns to 'git for-each-ref' and 'git
ls-remote' to list only those refs that match the current word to be
completed.  This reduces the number of iterations in __gitcomp_nl()
from the number of refs to the number of matching refs.

This speeds up refs completion considerably when there are a lot of
non-matching refs to be filtered out.  Uniquely completing a branch in
a repository with 100k local branches, all packed, best of five:

  On Linux, before:

$ time __git_complete_refs --cur=maste

real0m0.831s
user0m0.808s
sys 0m0.028s

  After:

real0m0.119s
user0m0.104s
sys 0m0.008s

  On Windows, before:

real0m1.480s
user0m1.031s
sys 0m0.060s

  After:

real0m0.377s
user0m0.015s
sys 0m0.030s

Strictly speaking this is a fundamental behavior change in
__git_refs(), a helper function that is likely used in users' custom
completion scriptlets.  However, arguably this change should be
harmless, because:

  - __git_refs() was only ever intended to feed refs to Bash's
completion machinery, for which non-matching refs have to be
filtered out anyway.

  - There were already code paths that list only matching refs
(listing unique remote branches for 'git checkout's tracking
DWIMery and listing full remote refs via 'git ls-remote').

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash |  14 +++--
 t/t9902-completion.sh  | 102 +
 2 files changed, 111 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index d55eadfd1..615292f8b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -394,7 +394,7 @@ __git_refs ()
case "$cur_" in
refs|refs/*)
format="refname"
-   refs="${cur_%/*}"
+   refs=("$cur_*" "$cur_*/**")
track=""
;;
*)
@@ -402,11 +402,13 @@ __git_refs ()
if [ -e "$dir/$i" ]; then echo $pfx$i; fi
done
format="refname:strip=2"
-   refs="refs/tags refs/heads refs/remotes"
+   refs=("refs/tags/$cur_*" "refs/tags/$cur_*/**"
+   "refs/heads/$cur_*" "refs/heads/$cur_*/**"
+   "refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
;;
esac
__git_dir="$dir" __git for-each-ref --format="$pfx%($format)" \
-   $refs
+   "${refs[@]}"
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the 
completion word
@@ -438,10 +440,12 @@ __git_refs ()
if [ "$list_refs_from" = remote ]; then
echo "HEAD"
__git for-each-ref --format="%(refname:strip=2)" \
-   "refs/remotes/$remote/" | sed -e "s#^$remote/##"
+   "refs/remotes/$remote/$cur_*" \
+   "refs/remotes/$remote/$cur_*/**" | sed -e 
"s#^$remote/##"
else
__git ls-remote "$remote" HEAD \
-   "refs/tags/*" "refs/heads/*" "refs/remotes/*" |
+   "refs/tags/$cur_*" "refs/heads/$cur_*" \
+   "refs/remotes/$cur_*" |
while read -r hash i; do
case "$i" in
*^{})   ;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7b42a686c..499be5879 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -837,6 +837,108 @@ test_expect_success '__git refs - exluding full refs' '
test_cmp expected "$actual"
 '
 
+test_expect_success 'setup for filtering matching refs' '
+   git branch matching/branch &&
+   git tag matching/tag &&
+   git -C otherrepo branch matching/branch-in-other &&
+   git fetch --no-tags other &&
+   rm -f .git/FETCH_HEAD
+'
+
+test_expect_success '__git_refs - only matching refs' '
+   cat >expected <<-EOF &&
+   HEAD
+   matching-branch
+   matching/branch
+ 

[PATCH 02/12] completion: wrap __git_refs() for better option parsing

2017-02-02 Thread SZEDER Gábor
__git_refs() currently accepts two optional positional parameters: a
remote and a flag for 'git checkout's tracking DWIMery.  To fix a
minor bug, and, more importantly, for faster refs completion, this
series will add three more parameters: a prefix, the current word to
be completed and a suffix, i.e. the options accepted by __gitcomp() &
friends, and will change __git_refs() to list only refs matching that
given current word and to add that given prefix and suffix to the
listed refs.

However, __git_refs() is the helper function that is most likely used
in users' custom completion scriptlets for their own git commands, and
we don't want to break those, so

  - we can't change __git_refs()'s default output format, i.e. we
can't by default append a trailing space to every listed ref,
meaning that the suffix parameter containing the default trailing
space would have to be specified on every invocation, and

  - we can't change the position of existing positional parameters
either, so there would have to be plenty of set-but-empty
placeholder positional parameters all over the completion script.

Furthermore, with five positional parameters it would be really hard
to remember which position means what.

To keep callsites simple, add the new wrapper function
__git_complete_refs() around __git_refs(), which:

  - instead of positional parameters accepts real '--opt=val'-style
options and with minimalistic option parsing translates them to
__git_refs()'s and __gitcomp_nl()'s positional parameters, and

  - includes the '__gitcomp_nl "$(__git_refs ...)" ...' command
substitution to make its behavior match its name and the behavior
of other __git_complete_* functions, and to limit future changes
in this series to __git_refs() and this new wrapper function.

Call this wrapper function instead of __git_refs() wherever possible
throughout the completion script, i.e. when __git_refs()'s output is
fed to __gitcomp_nl() right away without further processing, which
means all callsites except a single one in the __git_refs2() helper.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 102 ---
 t/t9902-completion.sh  | 106 +
 2 files changed, 173 insertions(+), 35 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index f20d4600c..7f19e2a4f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -354,6 +354,8 @@ __git_tags ()
 #Can be the name of a configured remote, a path, or a URL.
 # 2: In addition to local refs, list unique branches from refs/remotes/ for
 #'git checkout's tracking DWIMery (optional; ignored, if set but empty).
+#
+# Use __git_complete_refs() instead.
 __git_refs ()
 {
local i hash dir track="${2-}"
@@ -446,6 +448,36 @@ __git_refs ()
esac
 }
 
+# Completes refs, short and long, local and remote, symbolic and pseudo.
+#
+# Usage: __git_complete_refs []...
+# --remote=: The remote to list refs from, can be the name of a
+#configured remote, a path, or a URL.
+# --track: List unique remote branches for 'git checkout's tracking DWIMery.
+# --pfx=: A prefix to be added to each ref.
+# --cur=: The current ref to be completed.  Defaults to the current
+#   word to be completed.
+# --sfx=: A suffix to be appended to each ref instead of the default
+# space.
+__git_complete_refs ()
+{
+   local remote track pfx cur_="$cur" sfx=" "
+
+   while test $# != 0; do
+   case "$1" in
+   --remote=*) remote="${1##--remote=}" ;;
+   --track)track="yes" ;;
+   --pfx=*)pfx="${1##--pfx=}" ;;
+   --cur=*)cur_="${1##--cur=}" ;;
+   --sfx=*)sfx="${1##--sfx=}" ;;
+   *)  return 1 ;;
+   esac
+   shift
+   done
+
+   __gitcomp_nl "$(__git_refs "$remote" "$track")" "$pfx" "$cur_" "$sfx"
+}
+
 # __git_refs2 requires 1 argument (to pass to __git_refs)
 __git_refs2 ()
 {
@@ -554,15 +586,15 @@ __git_complete_revlist_file ()
*...*)
pfx="${cur_%...*}..."
cur_="${cur_#*...}"
-   __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
+   __git_complete_refs --pfx="$pfx" --cur="$cur_"
;;
*..*)
pfx="${cur_%..*}.."
cur_="${cur_#*..}"
-   __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
+   __git_complete_refs --pfx="$pfx" --cur="$cur_"
;;
*)
-   __gitcomp_nl "$(__git_refs)"
+   __git_complete_refs
;;
esac
 }
@@ -649,21 +681,21 @@ __git_complete_remote_or_refspec ()
if [ $lhs = 1 ]; then
__gitcomp_nl "$(__git_refs2 "$remote")" "$

[PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit()

2017-02-02 Thread SZEDER Gábor
Those two options are specifying the default values that
__gitcomp_nl() would use anyway when invoked with no options at all.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index ed06cb621..f20d4600c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1216,7 +1216,7 @@ _git_commit ()
 {
case "$prev" in
-c|-C)
-   __gitcomp_nl "$(__git_refs)" "" "${cur}"
+   __gitcomp_nl "$(__git_refs)"
return
;;
esac
-- 
2.11.0.555.g967c1bcb3



[PATCH 00/12] completion: speed up refs completion

2017-02-02 Thread SZEDER Gábor
This series speeds up refs completion for large number of refs, partly
by giving up disambiguating ambiguous refs (patch 6) and partly by
eliminating most of the shell processing between 'git for-each-ref'
and 'ls-remote' and Bash's completion facility.  The rest is a bit of
preparatory reorganization, cleanup and bugfixes.

The last patch touches the ZSH wrapper, too.  By a lucky educated
guess I managed to get it work on the first try, but I don't really
know what I've actually done, so...  ZSH users, please have a closer
look.

At the end of this series refs completion from a local repository is
as fast as it can possibly get, at least as far as the completion
script is concerned, because it basically does nothing anymore :)  All
it does is run 'git for-each-ref' with assorted options to do all the
work, and feed its output directly, without any processing into Bash's
COMPREPLY array.  There is still room for improvements in the code
paths using 'git ls-remote', but for that we would need enhancements
to 'ls-remote'.

It goes on top of the __gitdir() improvements series I just posted at:

  http://public-inbox.org/git/20170203024829.8071-1-szeder@gmail.com/T/

This series is also available at:

  https://github.com/szeder/git completion-refs-speedup


SZEDER Gábor (12):
  completion: remove redundant __gitcomp_nl() options from _git_commit()
  completion: wrap __git_refs() for better option parsing
  completion: support completing full refs after '--option=refs/'
  completion: support excluding full refs
  completion: don't disambiguate tags and branches
  completion: don't disambiguate short refs
  completion: let 'for-each-ref' and 'ls-remote' filter matching refs
  completion: let 'for-each-ref' strip the remote name from remote
branches
  completion: let 'for-each-ref' filter remote branches for 'checkout'
DWIMery
  completion: let 'for-each-ref' sort remote branches for 'checkout'
DWIMery
  completion: list only matching symbolic and pseudorefs when completing
refs
  completion: fill COMPREPLY directly when completing refs

 contrib/completion/git-completion.bash | 205 
 contrib/completion/git-completion.zsh  |   9 ++
 t/t9902-completion.sh  | 282 +
 3 files changed, 430 insertions(+), 66 deletions(-)

-- 
2.11.0.555.g967c1bcb3



[PATCHv2 14/21] completion: fix completion after 'git -C '

2017-02-02 Thread SZEDER Gábor
The main completion function finds the name of the git command by
iterating through all the words on the command line in search for the
first non-option-looking word.  As it is not aware of 'git -C's
mandatory path argument, if the '-C ' option is present, 'path'
will be the first such word and it will be mistaken for a git command.
This breaks completion in various ways:

 - If 'path' happens to match one of the commands supported by the
   completion script, then options of that command will be offered.

 - If 'path' doesn't match a supported command and doesn't contain any
   characters not allowed in Bash identifier names, then the
   completion script does basically nothing and Bash in turn falls
   back to filename completion for all subsequent words.

 - Otherwise, if 'path' does contain such an unallowed character, then
   it leads to a more or less ugly error message in the middle of the
   command line.  The standard '/' directory separator is such a
   character, and it happens to trigger one of the uglier errors:

 $ git -C some/path sh.exe": declare: `_git_some/path': not a valid 
identifier
 error: invalid key: alias.some/path

Fix this by skipping 'git -C's mandatory path argument while iterating
over the words on the command line.  Extend the relevant test with
this case and, while at it, with cases that needed similar treatment
in the past ('--git-dir', '-c', '--work-tree' and '--namespace').

Additionally, silence the standard error of the 'declare' builtins
looking for the completion function associated with the git command
and of the 'git config' query for the aliased command.  So if git ever
learns a new option with a mandatory argument in the future, then,
though the completion script will again misbehave, at least the
command line will not be utterly disrupted by those error messages.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 8 
 t/t9902-completion.sh  | 7 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 7d25b33b8..ac5eb9ced 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -816,7 +816,7 @@ __git_aliases ()
 __git_aliased_command ()
 {
local word cmdline=$(git --git-dir="$(__gitdir)" \
-   config --get "alias.$1")
+   config --get "alias.$1" 2>/dev/null)
for word in $cmdline; do
case "$word" in
\!gitk|gitk)
@@ -2800,7 +2800,7 @@ __git_main ()
--git-dir)   ((c++)) ; __git_dir="${words[c]}" ;;
--bare)  __git_dir="." ;;
--help) command="help"; break ;;
-   -c|--work-tree|--namespace) ((c++)) ;;
+   -c|-C|--work-tree|--namespace) ((c++)) ;;
-*) ;;
*) command="$i"; break ;;
esac
@@ -2844,13 +2844,13 @@ __git_main ()
fi
 
local completion_func="_git_${command//-/_}"
-   declare -f $completion_func >/dev/null && $completion_func && return
+   declare -f $completion_func >/dev/null 2>/dev/null && $completion_func 
&& return
 
local expansion=$(__git_aliased_command "$command")
if [ -n "$expansion" ]; then
words[1]=$expansion
completion_func="_git_${expansion//-/_}"
-   declare -f $completion_func >/dev/null && $completion_func
+   declare -f $completion_func >/dev/null 2>/dev/null && 
$completion_func
fi
 }
 
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 500505dca..be2ed8704 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -755,7 +755,12 @@ test_expect_success 'general options plus command' '
test_completion "git --namespace=foo check" "checkout " &&
test_completion "git --paginate check" "checkout " &&
test_completion "git --info-path check" "checkout " &&
-   test_completion "git --no-replace-objects check" "checkout "
+   test_completion "git --no-replace-objects check" "checkout " &&
+   test_completion "git --git-dir some/path check" "checkout " &&
+   test_completion "git -c conf.var=value check" "checkout " &&
+   test_completion "git -C some/path check" "checkout " &&
+   test_completion "git --work-tree some/path check" "checkout " &&
+   test_completion "git --namespace name/space check" "checkout "
 '
 
 test_expect_success 'git --help completion' '
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 09/21] completion: respect 'git --git-dir=' when listing remote refs

2017-02-02 Thread SZEDER Gábor
In __git_refs() the git commands listing refs, both short and full,
from a given remote repository are run without giving them the path to
the git repository which might have been specified on the command line
via 'git --git-dir='.  This is bad, those git commands should
access the 'refs/remotes//' hierarchy or the remote and
credentials configuration in that specified repository.

Use the __gitdir() helper only to find the path to the .git directory
and pass the resulting path to the 'git ls-remote' and 'for-each-ref'
executions that list remote refs.  While modifying that 'for-each-ref'
line, remove the superfluous disambiguating doubledash.

Don't use __gitdir() to check that the given remote is on the file
system: basically it performs only a single if statement for us at the
considerable cost of fork()ing a subshell for a command substitution.
We are better off to perform all the necessary checks of the remote in
__git_refs().

Though __git_refs() was the last remaining callsite that passed a
remote to __gitdir(), don't delete __gitdir()'s remote-handling part
yet, just in case some users' custom completion scriptlets depend on
it.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 22 +-
 t/t9902-completion.sh  |  4 ++--
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 295f6de24..7d7e8b9d9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -342,9 +342,21 @@ __git_tags ()
 #'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 __git_refs ()
 {
-   local i hash dir="$(__gitdir "${1-}")" track="${2-}"
+   local i hash dir="$(__gitdir)" track="${2-}"
+   local list_refs_from=path remote="${1-}"
local format refs pfx
-   if [ -d "$dir" ]; then
+
+   if [ -n "$remote" ]; then
+   if [ -d "$remote/.git" ]; then
+   dir="$remote/.git"
+   elif [ -d "$remote" ]; then
+   dir="$remote"
+   else
+   list_refs_from=remote
+   fi
+   fi
+
+   if [ "$list_refs_from" = path ] && [ -d "$dir" ]; then
case "$cur" in
refs|refs/*)
format="refname"
@@ -381,7 +393,7 @@ __git_refs ()
fi
case "$cur" in
refs|refs/*)
-   git ls-remote "$dir" "$cur*" 2>/dev/null | \
+   git --git-dir="$dir" ls-remote "$remote" "$cur*" 2>/dev/null | \
while read -r hash i; do
case "$i" in
*^{}) ;;
@@ -391,8 +403,8 @@ __git_refs ()
;;
*)
echo "HEAD"
-   git for-each-ref --format="%(refname:short)" -- \
-   "refs/remotes/$dir/" 2>/dev/null | sed -e "s#^$dir/##"
+   git --git-dir="$dir" for-each-ref --format="%(refname:short)" \
+   "refs/remotes/$remote/" 2>/dev/null | sed -e 
"s#^$remote/##"
;;
esac
 }
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7667baabf..6e64cd6ba 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -486,7 +486,7 @@ test_expect_success '__git_refs - configured remote - full 
refs' '
test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - configured remote - repo given on the 
command line' '
+test_expect_success '__git_refs - configured remote - repo given on the 
command line' '
cat >expected <<-EOF &&
HEAD
branch-in-other
@@ -501,7 +501,7 @@ test_expect_failure '__git_refs - configured remote - repo 
given on the command
test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - configured remote - full refs - repo given 
on the command line' '
+test_expect_success '__git_refs - configured remote - full refs - repo given 
on the command line' '
cat >expected <<-EOF &&
refs/heads/branch-in-other
refs/heads/master-in-other
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 01/21] completion: improve __git_refs()'s in-code documentation

2017-02-02 Thread SZEDER Gábor
That "first argument is passed to __gitdir()" statement in particular
is not really helpful, and after this series it won't be the case
anyway.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 6721ff80f..ee6fb2259 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -332,9 +332,11 @@ __git_tags ()
fi
 }
 
-# __git_refs accepts 0, 1 (to pass to __gitdir), or 2 arguments
-# presence of 2nd argument means use the guess heuristic employed
-# by checkout for tracking branches
+# Lists refs from the local (by default) or from a remote repository.
+# It accepts 0, 1 or 2 arguments:
+# 1: The remote to list refs from (optional; ignored, if set but empty).
+# 2: In addition to local refs, list unique branches from refs/remotes/ for
+#'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 __git_refs ()
 {
local i hash dir="$(__gitdir "${1-}")" track="${2-}"
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 06/21] completion tests: add tests for the __git_refs() helper function

2017-02-02 Thread SZEDER Gábor
Check how __git_refs() lists refs in different scenarios, i.e.

  - short and full refs,
  - from a local or from a remote repository,
  - remote specified via path, name or URL,
  - with or without a repository specified on the command line,
  - non-existing remote,
  - unique remote branches for 'git checkout's tracking DWIMery,
  - not in a git repository, and
  - interesting combinations of the above.

Seven of these tests expect failure, mostly demonstrating bugs related
to listing refs from a remote repository:

  - ignoring the repository specified on the command line (2 tests),
  - listing refs from the wrong place when the name of a configured
remote happens to match a directory,
  - listing only 'HEAD' but no short refs from a remote given as URL,
  - listing 'HEAD' even from non-existing remotes (2 tests), and
  - listing 'HEAD' when not in a repository.

Signed-off-by: SZEDER Gábor 
---
 t/t9902-completion.sh | 265 +-
 1 file changed, 264 insertions(+), 1 deletion(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f7f7d49fb..7956cb9b1 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -365,6 +365,269 @@ test_expect_success '__git_remotes - list remotes from 
$GIT_DIR/remotes and from
test_cmp expect actual
 '
 
+test_expect_success 'setup for ref completion' '
+   git commit --allow-empty -m initial &&
+   git branch matching-branch &&
+   git tag matching-tag &&
+   (
+   cd otherrepo &&
+   git commit --allow-empty -m initial &&
+   git branch -m master master-in-other &&
+   git branch branch-in-other &&
+   git tag tag-in-other
+   ) &&
+   git remote add other "$ROOT/otherrepo/.git" &&
+   git fetch --no-tags other &&
+   rm -f .git/FETCH_HEAD &&
+   git init thirdrepo
+'
+
+test_expect_success '__git_refs - simple' '
+   cat >expected <<-EOF &&
+   HEAD
+   master
+   matching-branch
+   other/branch-in-other
+   other/master-in-other
+   matching-tag
+   EOF
+   (
+   cur= &&
+   __git_refs >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - full refs' '
+   cat >expected <<-EOF &&
+   refs/heads/master
+   refs/heads/matching-branch
+   EOF
+   (
+   cur=refs/heads/ &&
+   __git_refs >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - repo given on the command line' '
+   cat >expected <<-EOF &&
+   HEAD
+   branch-in-other
+   master-in-other
+   tag-in-other
+   EOF
+   (
+   __git_dir="$ROOT/otherrepo/.git" &&
+   cur= &&
+   __git_refs >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - remote on local file system' '
+   cat >expected <<-EOF &&
+   HEAD
+   branch-in-other
+   master-in-other
+   tag-in-other
+   EOF
+   (
+   cur= &&
+   __git_refs otherrepo >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - remote on local file system - full refs' '
+   cat >expected <<-EOF &&
+   refs/heads/branch-in-other
+   refs/heads/master-in-other
+   refs/tags/tag-in-other
+   EOF
+   (
+   cur=refs/ &&
+   __git_refs otherrepo >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - configured remote' '
+   cat >expected <<-EOF &&
+   HEAD
+   branch-in-other
+   master-in-other
+   EOF
+   (
+   cur= &&
+   __git_refs other >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - configured remote - full refs' '
+   cat >expected <<-EOF &&
+   refs/heads/branch-in-other
+   refs/heads/master-in-other
+   refs/tags/tag-in-other
+   EOF
+   (
+   cur=refs/ &&
+   __git_refs other >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_failure '__git_refs - configured remote - repo given on the 
command line' '
+   cat >expected <<-EOF &&
+   HEAD
+   branch-in-other
+   master-in-other
+   EOF
+   (
+   cd thirdrepo &&
+   __git_dir="$ROOT/.git" &&
+   cur= &&
+   __git_refs other >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_failure '__git_refs - configured remote - full refs - repo given 
on the command line' '
+   cat >expected <<-EOF &&
+   refs/heads/branch-in-other
+   refs/heads/master-in-other
+   refs/tags/tag-in-other
+   EOF
+   (
+   cd thirdrepo &&
+   __git_dir="$ROOT/.git" &&
+   

[PATCHv2 08/21] completion: fix most spots not respecting 'git --git-dir='

2017-02-02 Thread SZEDER Gábor
The completion script already respects the path to the repository
specified on the command line most of the time, here we add the
necessary '--git-dir=$(__gitdir)' options to most of the places where
git was executed without it.  The exceptions where said option is not
added are the git invocations:

  - in __git_refs() which are non-trivial and will be the subject of
the following patch,

  - getting the list of git commands, merge strategies and archive
formats, because these are independent from the repository and
thus don't need it, and

  - the 'git rev-parse --git-dir' in __gitdir() itself.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 5b2bd6721..295f6de24 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -283,11 +283,13 @@ __gitcomp_file ()
 # argument, and using the options specified in the second argument.
 __git_ls_files_helper ()
 {
+   local dir="$(__gitdir)"
+
if [ "$2" == "--committable" ]; then
-   git -C "$1" diff-index --name-only --relative HEAD
+   git --git-dir="$dir" -C "$1" diff-index --name-only --relative 
HEAD
else
# NOTE: $2 is not quoted in order to support multiple options
-   git -C "$1" ls-files --exclude-standard $2
+   git --git-dir="$dir" -C "$1" ls-files --exclude-standard $2
fi 2>/dev/null
 }
 
@@ -408,7 +410,7 @@ __git_refs2 ()
 __git_refs_remotes ()
 {
local i hash
-   git ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
+   git --git-dir="$(__gitdir)" ls-remote "$1" 'refs/heads/*' 2>/dev/null | 
\
while read -r hash i; do
echo "$i:refs/remotes/$1/${i#refs/heads/}"
done
@@ -1186,7 +1188,7 @@ _git_commit ()
return
esac
 
-   if git rev-parse --verify --quiet HEAD >/dev/null; then
+   if git --git-dir="$(__gitdir)" rev-parse --verify --quiet HEAD 
>/dev/null; then
__git_complete_index_file "--committable"
else
# This is the first commit
@@ -1486,7 +1488,7 @@ _git_log ()
 {
__git_has_doubledash && return
 
-   local g="$(git rev-parse --git-dir 2>/dev/null)"
+   local g="$(__gitdir)"
local merge=""
if [ -f "$g/MERGE_HEAD" ]; then
merge="--merge"
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 20/21] completion: extract repository discovery from __gitdir()

2017-02-02 Thread SZEDER Gábor
To prepare for caching the path to the repository in the following
commit, extract the repository discovering part of __gitdir() into the
__git_find_repo_path() helper function, which stores the found path in
the $__git_repo_path variable instead of printing it.  Make __gitdir()
a wrapper around this new function.  Declare $__git_repo_path local in
the toplevel completion functions __git_main() and __gitk_main() to
ensure that it never leaks into the environment and influences
subsequent completions (though this isn't necessary right now, as
__gitdir() is still only executed in subshells, but will matter for
the following commit).

Adjust tests checking __gitdir() or any other completion function
calling __gitdir() to perform those checks in a subshell to prevent
$__git_repo_path from leaking into the test environment.  Otherwise
leave the tests unchanged to demonstrate that this change doesn't
alter __gitdir()'s behavior.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 42 +-
 t/t9902-completion.sh  | 22 +-
 2 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 1c7493ff2..7775411cd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -34,26 +34,35 @@ case "$COMP_WORDBREAKS" in
 *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
 esac
 
+# Discovers the path to the git repository taking any '--git-dir=' and
+# '-C ' options into account and stores it in the $__git_repo_path
+# variable.
+__git_find_repo_path ()
+{
+   if [ -n "${__git_C_args-}" ]; then
+   __git_repo_path="$(git "${__git_C_args[@]}" \
+   ${__git_dir:+--git-dir="$__git_dir"} \
+   rev-parse --absolute-git-dir 2>/dev/null)"
+   elif [ -n "${__git_dir-}" ]; then
+   test -d "$__git_dir" &&
+   __git_repo_path="$__git_dir"
+   elif [ -n "${GIT_DIR-}" ]; then
+   test -d "${GIT_DIR-}" &&
+   __git_repo_path="$GIT_DIR"
+   elif [ -d .git ]; then
+   __git_repo_path=.git
+   else
+   __git_repo_path="$(git rev-parse --git-dir 2>/dev/null)"
+   fi
+}
+
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
 __gitdir ()
 {
if [ -z "${1-}" ]; then
-   if [ -n "${__git_C_args-}" ]; then
-   git "${__git_C_args[@]}" \
-   ${__git_dir:+--git-dir="$__git_dir"} \
-   rev-parse --absolute-git-dir 2>/dev/null
-   elif [ -n "${__git_dir-}" ]; then
-   test -d "$__git_dir" || return 1
-   echo "$__git_dir"
-   elif [ -n "${GIT_DIR-}" ]; then
-   test -d "${GIT_DIR-}" || return 1
-   echo "$GIT_DIR"
-   elif [ -d .git ]; then
-   echo .git
-   else
-   git rev-parse --git-dir 2>/dev/null
-   fi
+   __git_find_repo_path || return 1
+   echo "$__git_repo_path"
elif [ -d "$1/.git" ]; then
echo "$1/.git"
else
@@ -2783,7 +2792,7 @@ _git_worktree ()
 
 __git_main ()
 {
-   local i c=1 command __git_dir
+   local i c=1 command __git_dir __git_repo_path
local __git_C_args C_args_count=0
 
while [ $c -lt $cword ]; do
@@ -2855,6 +2864,7 @@ __gitk_main ()
 {
__git_has_doubledash && return
 
+   local __git_repo_path
local g="$(__gitdir)"
local merge=""
if [ -f "$g/MERGE_HEAD" ]; then
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 984df05b2..1816ed2e0 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -147,19 +147,25 @@ test_expect_success '__gitdir - from command line 
(through $__git_dir)' '
 
 test_expect_success '__gitdir - repo as argument' '
echo "otherrepo/.git" >expected &&
-   __gitdir "otherrepo" >"$actual" &&
+   (
+   __gitdir "otherrepo" >"$actual"
+   ) &&
test_cmp expected "$actual"
 '
 
 test_expect_success '__gitdir - remote as argument' '
echo "remote" >expected &&
-   __gitdir "remote" >"$actual" &&
+   (
+   __gitdir "remote" >"$actual"
+   ) &&
test_cmp expected "$actual"
 '
 
 test_expect_success '__gitdir - .git directory in cwd' '
echo ".git" >expected &&
-   __gitdir >"$actual" &&
+   (
+   __gitdir >"$actual"
+   ) &&
test_cmp expected "$actual"
 '
 
@@ -450,7 +456,9 @@ test_expect_success '__git_remotes - list remotes from 
$GIT_DIR/remotes and from
git remote add remote_in_config_1 git://remote_1 &&
test_when_finished "git remote remove remote_in_config_2" &&
git remote add remote_in_co

[PATCHv2 10/21] completion: list refs from remote when remote's name matches a directory

2017-02-02 Thread SZEDER Gábor
If the remote given to __git_refs() happens to match both the name of
a configured remote and the name of a directory in the current working
directory, then that directory is assumed to be a git repository, and
listing refs from that directory will be attempted.  This is wrong,
because in such a situation git commands (e.g. 'git fetch|pull|push
' whom these refs will eventually be passed to) give
precedence to the configured remote.  Therefore, __git_refs() should
list refs from the configured remote as well.

Add the helper function __git_is_configured_remote() that checks
whether its argument matches the name of a configured remote.  Use
this helper to decide how to handle the remote passed to __git_refs().

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 20 ++--
 t/t9902-completion.sh  | 11 ++-
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 7d7e8b9d9..fd0ba1f3b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -347,12 +347,16 @@ __git_refs ()
local format refs pfx
 
if [ -n "$remote" ]; then
-   if [ -d "$remote/.git" ]; then
+   if __git_is_configured_remote "$remote"; then
+   # configured remote takes precedence over a
+   # local directory with the same name
+   list_refs_from=remote
+   elif [ -d "$remote/.git" ]; then
dir="$remote/.git"
elif [ -d "$remote" ]; then
dir="$remote"
else
-   list_refs_from=remote
+   list_refs_from=url
fi
fi
 
@@ -435,6 +439,18 @@ __git_remotes ()
git --git-dir="$d" remote
 }
 
+# Returns true if $1 matches the name of a configured remote, false otherwise.
+__git_is_configured_remote ()
+{
+   local remote
+   for remote in $(__git_remotes); do
+   if [ "$remote" = "$1" ]; then
+   return 0
+   fi
+   done
+   return 1
+}
+
 __git_list_merge_strategies ()
 {
git merge -s help 2>&1 |
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 6e64cd6ba..a201b5212 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -373,6 +373,15 @@ test_expect_success '__git_remotes - list remotes from 
$GIT_DIR/remotes and from
test_cmp expect actual
 '
 
+test_expect_success '__git_is_configured_remote' '
+   test_when_finished "git remote remove remote_1" &&
+   git remote add remote_1 git://remote_1 &&
+   test_when_finished "git remote remove remote_2" &&
+   git remote add remote_2 git://remote_2 &&
+   verbose __git_is_configured_remote remote_2 &&
+   test_must_fail __git_is_configured_remote non-existent
+'
+
 test_expect_success 'setup for ref completion' '
git commit --allow-empty -m initial &&
git branch matching-branch &&
@@ -516,7 +525,7 @@ test_expect_success '__git_refs - configured remote - full 
refs - repo given on
test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - configured remote - remote name matches a 
directory' '
+test_expect_success '__git_refs - configured remote - remote name matches a 
directory' '
cat >expected <<-EOF &&
HEAD
branch-in-other
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 19/21] completion: don't guard git executions with __gitdir()

2017-02-02 Thread SZEDER Gábor
Three completion functions, namely __git_index_files(), __git_heads()
and __git_tags(), first run __gitdir() and check that the path it
outputs exists, i.e. that there is a git repository, and run a git
command only if there is one.

After the previous changes in this series there are no further uses of
__gitdir()'s output in these functions besides those checks.  And
those checks are unnecessary, because we can just execute those git
commands outside of a repository and let them error out.  We don't
perform such a check in other places either.

Remove this check and the __gitdir() call from these functions,
sparing the fork()+exec() overhead of the command substitution and the
potential 'git rev-parse' execution.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 32 +++-
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 1a849761f..1c7493ff2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -312,35 +312,25 @@ __git_ls_files_helper ()
 #slash.
 __git_index_files ()
 {
-   local dir="$(__gitdir)" root="${2-.}" file
-
-   if [ -d "$dir" ]; then
-   __git_ls_files_helper "$root" "$1" |
-   while read -r file; do
-   case "$file" in
-   ?*/*) echo "${file%%/*}" ;;
-   *) echo "$file" ;;
-   esac
-   done | sort | uniq
-   fi
+   local root="${2-.}" file
+
+   __git_ls_files_helper "$root" "$1" |
+   while read -r file; do
+   case "$file" in
+   ?*/*) echo "${file%%/*}" ;;
+   *) echo "$file" ;;
+   esac
+   done | sort | uniq
 }
 
 __git_heads ()
 {
-   local dir="$(__gitdir)"
-   if [ -d "$dir" ]; then
-   __git for-each-ref --format='%(refname:short)' refs/heads
-   return
-   fi
+   __git for-each-ref --format='%(refname:short)' refs/heads
 }
 
 __git_tags ()
 {
-   local dir="$(__gitdir)"
-   if [ -d "$dir" ]; then
-   __git for-each-ref --format='%(refname:short)' refs/tags
-   return
-   fi
+   __git for-each-ref --format='%(refname:short)' refs/tags
 }
 
 # Lists refs from the local (by default) or from a remote repository.
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 15/21] rev-parse: add '--absolute-git-dir' option

2017-02-02 Thread SZEDER Gábor
The output of 'git rev-parse --git-dir' can be either a relative or an
absolute path, depending on whether the current working directory is
at the top of the worktree or the .git directory or not, or how the
path to the repository is specified via the '--git-dir=' option
or the $GIT_DIR environment variable.  And if that output is a
relative path, then it is relative to the directory where any 'git
-C ' options might have led us.

This doesn't matter at all for regular scripts, because the git
wrapper automatically takes care of changing directories according to
the '-C ' options, and the scripts can then simply follow any
path returned by 'git rev-parse --git-dir', even if it's a relative
path.

Our Bash completion script, however, is unique in that it must run
directly in the user's interactive shell environment.  This means that
it's not executed through the git wrapper and would have to take care
of any '-C  options on its own, and it can't just change
directories as it pleases.  Consequently, adding support for taking
any '-C ' options on the command line into account during
completion turned out to be considerably more difficult, error prone
and required more subshells and git processes when it had to cope with
a relative path to the .git directory.

Help this rather special use case and teach 'git rev-parse' a new
'--absolute-git-dir' option which always outputs a canonicalized
absolute path to the .git directory, regardless of whether the path is
discovered automatically or is specified via $GIT_DIR or 'git
--git-dir='.

Signed-off-by: SZEDER Gábor 
---
 Documentation/git-rev-parse.txt |  4 
 builtin/rev-parse.c | 26 ++
 t/t1500-rev-parse.sh| 17 +
 3 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 7241e9689..91c02b8c8 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -217,6 +217,10 @@ If `$GIT_DIR` is not defined and the current directory
 is not detected to lie in a Git repository or work tree
 print a message to stderr and exit with nonzero status.
 
+--absolute-git-dir::
+   Like `--git-dir`, but its output is always the canonicalized
+   absolute path.
+
 --git-common-dir::
Show `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index ff13e59e1..1967bafba 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -802,17 +802,27 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
putchar('\n');
continue;
}
-   if (!strcmp(arg, "--git-dir")) {
+   if (!strcmp(arg, "--git-dir") ||
+   !strcmp(arg, "--absolute-git-dir")) {
const char *gitdir = 
getenv(GIT_DIR_ENVIRONMENT);
char *cwd;
int len;
-   if (gitdir) {
-   puts(gitdir);
-   continue;
-   }
-   if (!prefix) {
-   puts(".git");
-   continue;
+   if (arg[2] == 'g') {/* --git-dir */
+   if (gitdir) {
+   puts(gitdir);
+   continue;
+   }
+   if (!prefix) {
+   puts(".git");
+   continue;
+   }
+   } else {/* --absolute-git-dir */
+   if (!gitdir && !prefix)
+   gitdir = ".git";
+   if (gitdir) {
+   puts(real_path(gitdir));
+   continue;
+   }
}
cwd = xgetcwd();
len = strlen(cwd);
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 038e24c40..8b62ed85b 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -3,7 +3,7 @@
 test_description='test git rev-parse'
 . ./test-lib.sh
 
-# usage: [options] label is-bare is-inside-git is-inside-work prefix git-dir
+# usage: [options] label is-bare is-inside-git is-inside-work prefix git-dir 
absolute-git-dir
 test_rev_parse () {
d=
bare=
@@ -29,7 +29,8 @@ test_rev_parse () {
 --is-inside-git-d

[PATCHv2 13/21] completion: don't offer commands when 'git --opt' needs an argument

2017-02-02 Thread SZEDER Gábor
The main git options '--git-dir', '-c', '-C', '--worktree' and
'--namespace' require an argument, but attempting completion right
after them lists git commands.

Don't offer anything right after these options, thus let Bash fall
back to filename completion, because

  - the three options '--git-dir', '-C' and '--worktree' do actually
require a path argument, and

  - we don't complete the required argument of '-c' and '--namespace',
and in that case the "standard" behavior of our completion script
is to not offer anything, but fall back to filename completion.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 4ded44977..7d25b33b8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2808,6 +2808,17 @@ __git_main ()
done
 
if [ -z "$command" ]; then
+   case "$prev" in
+   --git-dir|-C|--work-tree)
+   # these need a path argument, let's fall back to
+   # Bash filename completion
+   return
+   ;;
+   -c|--namespace)
+   # we don't support completing these options' arguments
+   return
+   ;;
+   esac
case "$cur" in
--*)   __gitcomp "
--paginate
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 18/21] completion: consolidate silencing errors from git commands

2017-02-02 Thread SZEDER Gábor
Outputting error messages during completion is bad: they disrupt the
command line, can't be deleted, and the user is forced to Ctrl-C and
start over most of the time.  We already silence stderr of many git
commands in our Bash completion script, but there are still some in
there that can spew error messages when something goes wrong.

We could add the missing stderr redirections to all the remaining
places, but instead let's leverage that git commands are now executed
through the previously introduced __git() wrapper function, and
redirect standard error to /dev/null only in that function.  This way
we need only one redirection to take care of errors from almost all
git commands.  Redirecting standard error of the __git() wrapper
function thus became redundant, remove them.

The exceptions, i.e. the repo-independent git executions and those in
the __gitdir() function that don't go through __git() already have
their standard error silenced.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 28 
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e7c0b56ea..1a849761f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -66,7 +66,7 @@ __gitdir ()
 __git ()
 {
git ${__git_C_args:+"${__git_C_args[@]}"} \
-   ${__git_dir:+--git-dir="$__git_dir"} "$@"
+   ${__git_dir:+--git-dir="$__git_dir"} "$@" 2>/dev/null
 }
 
 # The following function is based on code from:
@@ -300,7 +300,7 @@ __git_ls_files_helper ()
else
# NOTE: $2 is not quoted in order to support multiple options
__git -C "$1" ls-files --exclude-standard $2
-   fi 2>/dev/null
+   fi
 }
 
 
@@ -410,7 +410,7 @@ __git_refs ()
fi
case "$cur" in
refs|refs/*)
-   __git ls-remote "$remote" "$cur*" 2>/dev/null | \
+   __git ls-remote "$remote" "$cur*" | \
while read -r hash i; do
case "$i" in
*^{}) ;;
@@ -422,10 +422,10 @@ __git_refs ()
if [ "$list_refs_from" = remote ]; then
echo "HEAD"
__git for-each-ref --format="%(refname:short)" \
-   "refs/remotes/$remote/" 2>/dev/null | sed -e 
"s#^$remote/##"
+   "refs/remotes/$remote/" | sed -e "s#^$remote/##"
else
__git ls-remote "$remote" HEAD \
-   "refs/tags/*" "refs/heads/*" "refs/remotes/*" 
2>/dev/null |
+   "refs/tags/*" "refs/heads/*" "refs/remotes/*" |
while read -r hash i; do
case "$i" in
*^{})   ;;
@@ -451,7 +451,7 @@ __git_refs2 ()
 __git_refs_remotes ()
 {
local i hash
-   __git ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
+   __git ls-remote "$1" 'refs/heads/*' | \
while read -r hash i; do
echo "$i:refs/remotes/$1/${i#refs/heads/}"
done
@@ -527,7 +527,7 @@ __git_complete_revlist_file ()
*)   pfx="$ref:$pfx" ;;
esac
 
-   __gitcomp_nl "$(__git ls-tree "$ls" 2>/dev/null \
+   __gitcomp_nl "$(__git ls-tree "$ls" \
| sed '/^100... blob /{
   s,^.*,,
   s,$, ,
@@ -805,7 +805,7 @@ __git_compute_porcelain_commands ()
 __git_get_config_variables ()
 {
local section="$1" i IFS=$'\n'
-   for i in $(__git config --name-only --get-regexp "^$section\..*" 
2>/dev/null); do
+   for i in $(__git config --name-only --get-regexp "^$section\..*"); do
echo "${i#$section.}"
done
 }
@@ -823,7 +823,7 @@ __git_aliases ()
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
-   local word cmdline=$(__git config --get "alias.$1" 2>/dev/null)
+   local word cmdline=$(__git config --get "alias.$1")
for word in $cmdline; do
case "$word" in
\!gitk|gitk)
@@ -1841,9 +1841,7 @@ _git_send_email ()
 {
case "$prev" in
--to|--cc|--bcc|--from)
-   __gitcomp "
-   $(__git send-email --dump-aliases 2>/dev/null)
-   "
+   __gitcomp "$(__git send-email --dump-aliases)"
return
;;
esac
@@ -1873,9 +1871,7 @@ _git_send_email ()
return
;;
--to=*|--cc=*|--bcc=*|--from=*)
-   __gitcomp "
-   $(__git send-email --dump-aliases 2>/dev/null)
-   " "" "${cur#--*=}"
+   __gitcomp "$(__git send-email --dump-aliases)" "" "${cur#--*=}"
return
  

[PATCHv2 21/21] completion: cache the path to the repository

2017-02-02 Thread SZEDER Gábor
After the previous changes in this series there are only a handful of
$(__gitdir) command substitutions left in the completion script, but
there is still a bit of room for improvements:

  1. The command substitution involves the forking of a subshell,
 which has considerable overhead on some platforms.

  2. There are a few cases, where this command substitution is
 executed more than once during a single completion, which means
 multiple subshells and possibly multiple 'git rev-parse'
 executions.  __gitdir() is invoked twice while completing refs
 for e.g. 'git log', 'git rebase', 'gitk', or while completing
 remote refs for 'git fetch' or 'git push'.

Both of these points can be addressed by using the
__git_find_repo_path() helper function introduced in the previous
commit:

  1. __git_find_repo_path() stores the path to the repository in a
 variable instead of printing it, so the command substitution
 around the function can be avoided.  Or rather: the command
 substitution should be avoided to make the new value of the
 variable set inside the function visible to the callers.
 (Yes, there is now a command substitution inside
 __git_find_repo_path() around each 'git rev-parse', but that's
 executed only if necessary, and only once per completion, see
 point 2. below.)

  2. $__git_repo_path, the variable holding the path to the
 repository, is declared local in the toplevel completion
 functions __git_main() and __gitk_main().  Thus, once set, the
 path is visible in all completion functions, including all
 subsequent calls to __git_find_repo_path(), meaning that they
 wouldn't have to re-discover the path to the repository.

So call __git_find_repo_path() and use $__git_repo_path instead of the
$(__gitdir) command substitution to access paths in the .git
directory.  Turn tests checking __gitdir()'s repository discovery into
tests of __git_find_repo_path() such that only the tested function
changes but the expected results don't, ensuring that repo discovery
keeps working as it did before.

As __gitdir() is not used anymore in the completion script, mark it as
deprecated and direct users' attention to __git_find_repo_path() and
$__git_repo_path.  Yet keep four __gitdir() tests to ensure that it
handles success and failure of __git_find_repo_path() and that it
still handles its optional remote argument, because users' custom
completion scriptlets might depend on it.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash |  46 ++
 t/t9902-completion.sh  | 161 +
 2 files changed, 132 insertions(+), 75 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 7775411cd..ed06cb621 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -39,6 +39,11 @@ esac
 # variable.
 __git_find_repo_path ()
 {
+   if [ -n "$__git_repo_path" ]; then
+   # we already know where it is
+   return
+   fi
+
if [ -n "${__git_C_args-}" ]; then
__git_repo_path="$(git "${__git_C_args[@]}" \
${__git_dir:+--git-dir="$__git_dir"} \
@@ -56,6 +61,7 @@ __git_find_repo_path ()
fi
 }
 
+# Deprecated: use __git_find_repo_path() and $__git_repo_path instead
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
 __gitdir ()
@@ -350,10 +356,13 @@ __git_tags ()
 #'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 __git_refs ()
 {
-   local i hash dir="$(__gitdir)" track="${2-}"
+   local i hash dir track="${2-}"
local list_refs_from=path remote="${1-}"
local format refs pfx
 
+   __git_find_repo_path
+   dir="$__git_repo_path"
+
if [ -z "$remote" ]; then
if [ -z "$dir" ]; then
return
@@ -458,8 +467,8 @@ __git_refs_remotes ()
 
 __git_remotes ()
 {
-   local d="$(__gitdir)"
-   test -d "$d/remotes" && ls -1 "$d/remotes"
+   __git_find_repo_path
+   test -d "$__git_repo_path/remotes" && ls -1 "$__git_repo_path/remotes"
__git remote
 }
 
@@ -957,8 +966,8 @@ __git_whitespacelist="nowarn warn error error-all fix"
 
 _git_am ()
 {
-   local dir="$(__gitdir)"
-   if [ -d "$dir"/rebase-apply ]; then
+   __git_find_repo_path
+   if [ -d "$__git_repo_path"/rebase-apply ]; then
__gitcomp "--skip --continue --resolved --abort"
return
fi
@@ -1041,7 +1050,8 @@ _git_bisect ()
local subcommands="start bad good skip reset visualize replay log run"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
-   if [ -f "$(__gitdir)"/BISECT_START ]; then
+   __git_find_repo_path
+   if [ -f "$__git_repo_path"/BISECT_START ]; t

[PATCHv2 17/21] completion: don't use __gitdir() for git commands

2017-02-02 Thread SZEDER Gábor
Several completion functions contain the following pattern to run git
commands respecting the path to the repository specified on the
command line:

  git --git-dir="$(__gitdir)"  

This imposes the overhead of fork()ing a subshell for the command
substitution and potentially fork()+exec()ing 'git rev-parse' inside
__gitdir().

Now, if neither '--gitdir=' nor '-C ' options are
specified on the command line, then those git commands are perfectly
capable to discover the repository on their own.  If either one or
both of those options are specified on the command line, then, again,
the git commands could discover the repository, if we pass them all of
those options from the command line.

This means we don't have to run __gitdir() at all for git commands and
can spare its fork()+exec() overhead.

Use Bash parameter expansions to check the $__git_dir variable and
$__git_C_args array and to assemble the appropriate '--git-dir='
and '-C ' options if either one or both are present on the
command line.  These parameter expansions are, however, rather long,
so instead of changing all git executions and make already long lines
even longer, encapsulate running git with '--git-dir= -C '
options into the new __git() wrapper function.  Furthermore, this
wrapper function will also enable us to silence error messages from
git commands uniformly in one place in a later commit.

There's one tricky case, though: in __git_refs() local refs are listed
with 'git for-each-ref', where "local" is not necessarily the
repository we are currently in, but it might mean a remote repository
in the filesystem (e.g. listing refs for 'git fetch /some/other/repo
').  Use one-shot variable assignment to override $__git_dir with
the path of the repository where the refs should come from.  Although
one-shot variable assignments in front of shell functions are to be
avoided in our scripts in general, in the Bash completion script we
can do that safely.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 60 ++
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e003afd54..e7c0b56ea 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -61,6 +61,14 @@ __gitdir ()
fi
 }
 
+# Runs git with all the options given as argument, respecting any
+# '--git-dir=' and '-C ' options present on the command line
+__git ()
+{
+   git ${__git_C_args:+"${__git_C_args[@]}"} \
+   ${__git_dir:+--git-dir="$__git_dir"} "$@"
+}
+
 # The following function is based on code from:
 #
 #   bash_completion - programmable completion functions for bash 3.2+
@@ -287,13 +295,11 @@ __gitcomp_file ()
 # argument, and using the options specified in the second argument.
 __git_ls_files_helper ()
 {
-   local dir="$(__gitdir)"
-
if [ "$2" == "--committable" ]; then
-   git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C 
"$1" diff-index --name-only --relative HEAD
+   __git -C "$1" diff-index --name-only --relative HEAD
else
# NOTE: $2 is not quoted in order to support multiple options
-   git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C 
"$1" ls-files --exclude-standard $2
+   __git -C "$1" ls-files --exclude-standard $2
fi 2>/dev/null
 }
 
@@ -323,8 +329,7 @@ __git_heads ()
 {
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
-   git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
-   refs/heads
+   __git for-each-ref --format='%(refname:short)' refs/heads
return
fi
 }
@@ -333,8 +338,7 @@ __git_tags ()
 {
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
-   git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
-   refs/tags
+   __git for-each-ref --format='%(refname:short)' refs/tags
return
fi
 }
@@ -385,14 +389,14 @@ __git_refs ()
refs="refs/tags refs/heads refs/remotes"
;;
esac
-   git --git-dir="$dir" for-each-ref --format="$pfx%($format)" \
+   __git_dir="$dir" __git for-each-ref --format="$pfx%($format)" \
$refs
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the 
completion word
# but only output if the branch name is unique
local ref entry
-   git --git-dir="$dir" for-each-ref --shell 
--format="ref=%(refname:short)" \
+   __git for-each-ref --shell 
--format="ref=%(refname:short)" \
"refs/remotes/" | \
 

[PATCHv2 04/21] completion tests: consolidate getting path of current working directory

2017-02-02 Thread SZEDER Gábor
Some tests of the __gitdir() helper function use the $TRASH_DIRECTORY
variable in direct path comparisons.  In general this should be
avoided, because it might contain symbolic links.  There happens to be
no issues with this here, however, because those tests use
$TRASH_DIRECTORY both for specifying the expected result and for
specifying input which in turn is just 'echo'ed verbatim.

Other __gitdir() tests ask for the path of the trash directory by
running $(pwd -P) in each test, sometimes even twice in a single test.

Run $(pwd) only once at the beginning of the test script to store the
path of the trash directory in a variable, and use that variable in
all __gitdir() tests.

Signed-off-by: SZEDER Gábor 
---
 t/t9902-completion.sh | 44 +---
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 294a08cfe..030a16e77 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -124,15 +124,22 @@ invalid_variable_name='${foo.bar}'
 
 actual="$TRASH_DIRECTORY/actual"
 
+if test_have_prereq MINGW
+then
+   ROOT="$(pwd -W)"
+else
+   ROOT="$(pwd)"
+fi
+
 test_expect_success 'setup for __gitdir tests' '
mkdir -p subdir/subsubdir &&
git init otherrepo
 '
 
 test_expect_success '__gitdir - from command line (through $__git_dir)' '
-   echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+   echo "$ROOT/otherrepo/.git" >expected &&
(
-   __git_dir="$TRASH_DIRECTORY/otherrepo/.git" &&
+   __git_dir="$ROOT/otherrepo/.git" &&
__gitdir >"$actual"
) &&
test_cmp expected "$actual"
@@ -157,7 +164,7 @@ test_expect_success '__gitdir - .git directory in cwd' '
 '
 
 test_expect_success '__gitdir - .git directory in parent' '
-   echo "$(pwd -P)/.git" >expected &&
+   echo "$ROOT/.git" >expected &&
(
cd subdir/subsubdir &&
__gitdir >"$actual"
@@ -175,7 +182,7 @@ test_expect_success '__gitdir - cwd is a .git directory' '
 '
 
 test_expect_success '__gitdir - parent is a .git directory' '
-   echo "$(pwd -P)/.git" >expected &&
+   echo "$ROOT/.git" >expected &&
(
cd .git/refs/heads &&
__gitdir >"$actual"
@@ -184,9 +191,9 @@ test_expect_success '__gitdir - parent is a .git directory' 
'
 '
 
 test_expect_success '__gitdir - $GIT_DIR set while .git directory in cwd' '
-   echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+   echo "$ROOT/otherrepo/.git" >expected &&
(
-   GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+   GIT_DIR="$ROOT/otherrepo/.git" &&
export GIT_DIR &&
__gitdir >"$actual"
) &&
@@ -194,9 +201,9 @@ test_expect_success '__gitdir - $GIT_DIR set while .git 
directory in cwd' '
 '
 
 test_expect_success '__gitdir - $GIT_DIR set while .git directory in parent' '
-   echo "$TRASH_DIRECTORY/otherrepo/.git" >expected &&
+   echo "$ROOT/otherrepo/.git" >expected &&
(
-   GIT_DIR="$TRASH_DIRECTORY/otherrepo/.git" &&
+   GIT_DIR="$ROOT/otherrepo/.git" &&
export GIT_DIR &&
cd subdir &&
__gitdir >"$actual"
@@ -206,24 +213,15 @@ test_expect_success '__gitdir - $GIT_DIR set while .git 
directory in parent' '
 
 test_expect_success '__gitdir - non-existing $GIT_DIR' '
(
-   GIT_DIR="$TRASH_DIRECTORY/non-existing" &&
+   GIT_DIR="$ROOT/non-existing" &&
export GIT_DIR &&
test_must_fail __gitdir
)
 '
 
-function pwd_P_W () {
-   if test_have_prereq MINGW
-   then
-   pwd -W
-   else
-   pwd -P
-   fi
-}
-
 test_expect_success '__gitdir - gitfile in cwd' '
-   echo "$(pwd_P_W)/otherrepo/.git" >expected &&
-   echo "gitdir: $(pwd_P_W)/otherrepo/.git" >subdir/.git &&
+   echo "$ROOT/otherrepo/.git" >expected &&
+   echo "gitdir: $ROOT/otherrepo/.git" >subdir/.git &&
test_when_finished "rm -f subdir/.git" &&
(
cd subdir &&
@@ -233,8 +231,8 @@ test_expect_success '__gitdir - gitfile in cwd' '
 '
 
 test_expect_success '__gitdir - gitfile in parent' '
-   echo "$(pwd_P_W)/otherrepo/.git" >expected &&
-   echo "gitdir: $(pwd_P_W)/otherrepo/.git" >subdir/.git &&
+   echo "$ROOT/otherrepo/.git" >expected &&
+   echo "gitdir: $ROOT/otherrepo/.git" >subdir/.git &&
test_when_finished "rm -f subdir/.git" &&
(
cd subdir/subsubdir &&
@@ -244,7 +242,7 @@ test_expect_success '__gitdir - gitfile in parent' '
 '
 
 test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' '
-   echo "$(pwd -P)/otherrepo/.git" >expected &&
+   echo "$ROOT/otherrepo/.git" >expected &&
mkdir otherrepo/dir &&
test_when_finished "rm -rf otherrepo/dir" &&
ln -s ot

[PATCHv2 02/21] completion tests: don't add test cruft to the test repository

2017-02-02 Thread SZEDER Gábor
While preparing commits, three tests added newly created files to the
index using 'git add .', which added not only the files in question
but leftover test cruft from previous tests like the files 'expected'
and 'actual' as well.  Luckily, this had no effect on the tests'
correctness.

Add only the files we are actually interested in.

Signed-off-by: SZEDER Gábor 
---
 t/t9902-completion.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a34e55f87..f490c1d05 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -486,7 +486,7 @@ test_expect_success 'git --help completion' '
 test_expect_success 'setup for ref completion' '
echo content >file1 &&
echo more >file2 &&
-   git add . &&
+   git add file1 file2 &&
git commit -m one &&
git branch mybranch &&
git tag mytag
@@ -517,7 +517,7 @@ test_expect_success ': completes paths' '
 
 test_expect_success 'complete tree filename with spaces' '
echo content >"name with spaces" &&
-   git add . &&
+   git add "name with spaces" &&
git commit -m spaces &&
test_completion "git show HEAD:nam" <<-\EOF
name with spaces Z
@@ -526,7 +526,7 @@ test_expect_success 'complete tree filename with spaces' '
 
 test_expect_success 'complete tree filename with metacharacters' '
echo content >"name with \${meta}" &&
-   git add . &&
+   git add "name with \${meta}" &&
git commit -m meta &&
test_completion "git show HEAD:nam" <<-\EOF
name with ${meta} Z
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 00/21] completion: various __gitdir()-related improvements

2017-02-02 Thread SZEDER Gábor
This is a long overdue reroll of a bunch of bugfixes, cleanups and
optimizations related to how the completion script finds the path to
the repository and how it uses that path.  Most importantly this
series adds support for completion following 'git -C path', and it
eliminates a few subshells and git processes, for the sake of
fork()+exec() challenged OSes.

The first round is at [1].  It made its way to pu back then, but since
the reroll didn't come it was eventually discarded.

What did NOT change since v1 is that the new option added to 'git
rev-parse' is still called '--absolute-git-dir'.  There was a
suggestion [2] to turn it into an orthogonal '--absolute-dir' option
that works with other path-querying options, too, but I really doubt
it's worth it.  In short, regular scripts don't care, because a
relative path doesn't make any difference for them, and before we do
this orthogonal thing we have to decide a bunch of questions first,
see [3].

Changes since v1:

 - Use our real_path() instead of system realpath() to implement 'git
   rev-parse --absolute-git-dir'.
 - Refactored a bit how __git_refs() determines where it should list
   refs from.
 - Renamed a few refnames and remote in the tests (this accounts for
   the bulk of the interdiff).
 - Misc small adjustments: a few more comments, removed unnecessary
   disambiguating '--', typofix and more consistent quoting.
 - Improved commit messages.
 - Rebased to current master.

The interdiff below is compared to v1 rebased on top of current master.

This series is also available at

  https://github.com/szeder/git completion-gitdir-improvements


[1] - 
http://public-inbox.org/git/1456440650-32623-1-git-send-email-sze...@ira.uka.de/T/

[2] - 
http://public-inbox.org/git/CANoM8SXO_Rz_CVOz9ptsaVCzcQ2D1FQrSuFFW4vZ4SdRYMzD=w...@mail.gmail.com/

[3] - 
http://public-inbox.org/git/20160518185825.horde.epd2njnvqew_vx4b01yw...@webmail.informatik.kit.edu/

SZEDER Gábor (21):
  completion: improve __git_refs()'s in-code documentation
  completion tests: don't add test cruft to the test repository
  completion tests: make the $cur variable local to the test helper
functions
  completion tests: consolidate getting path of current working
directory
  completion tests: check __gitdir()'s output in the error cases
  completion tests: add tests for the __git_refs() helper function
  completion: ensure that the repository path given on the command line
exists
  completion: fix most spots not respecting 'git --git-dir='
  completion: respect 'git --git-dir=' when listing remote refs
  completion: list refs from remote when remote's name matches a
directory
  completion: don't list 'HEAD' when trying refs completion outside of a
repo
  completion: list short refs from a remote given as a URL
  completion: don't offer commands when 'git --opt' needs an argument
  completion: fix completion after 'git -C '
  rev-parse: add '--absolute-git-dir' option
  completion: respect 'git -C '
  completion: don't use __gitdir() for git commands
  completion: consolidate silencing errors from git commands
  completion: don't guard git executions with __gitdir()
  completion: extract repository discovery from __gitdir()
  completion: cache the path to the repository

 Documentation/git-rev-parse.txt|   4 +
 builtin/rev-parse.c|  26 +-
 contrib/completion/git-completion.bash | 250 ++-
 t/t1500-rev-parse.sh   |  17 +-
 t/t9902-completion.sh  | 561 +
 5 files changed, 690 insertions(+), 168 deletions(-)

-- 
2.11.0.555.g967c1bcb3

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 4040b3c86..1967bafba 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -820,10 +820,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
if (!gitdir && !prefix)
gitdir = ".git";
if (gitdir) {
-   char absolute_path[PATH_MAX];
-   if (!realpath(gitdir, 
absolute_path))
-   die_errno(_("unable to 
get absolute path"));
-   puts(absolute_path);
+   puts(real_path(gitdir));
continue;
}
}
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 0184d0ebc..ed06cb621 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -350,41 +350,38 @@ __git_tags ()
 
 # Lists refs from the local (by default) or from a remote repository.
 # It accepts 0, 1 or 2 arguments:
-# 1: The remote to lists refs from (option

[PATCHv2 05/21] completion tests: check __gitdir()'s output in the error cases

2017-02-02 Thread SZEDER Gábor
The __gitdir() helper function shouldn't output anything if not in a
git repository.  The relevant tests only checked its error code, so
extend them to ensure that there's no output.

Signed-off-by: SZEDER Gábor 
---
 t/t9902-completion.sh | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 030a16e77..f7f7d49fb 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -215,8 +215,9 @@ test_expect_success '__gitdir - non-existing $GIT_DIR' '
(
GIT_DIR="$ROOT/non-existing" &&
export GIT_DIR &&
-   test_must_fail __gitdir
-   )
+   test_must_fail __gitdir >"$actual"
+   ) &&
+   test_must_be_empty "$actual"
 '
 
 test_expect_success '__gitdir - gitfile in cwd' '
@@ -255,7 +256,8 @@ test_expect_success SYMLINKS '__gitdir - resulting path 
avoids symlinks' '
 '
 
 test_expect_success '__gitdir - not a git repository' '
-   nongit test_must_fail __gitdir
+   nongit test_must_fail __gitdir >"$actual" &&
+   test_must_be_empty "$actual"
 '
 
 test_expect_success '__gitcomp - trailing space - options' '
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 03/21] completion tests: make the $cur variable local to the test helper functions

2017-02-02 Thread SZEDER Gábor
The test helper functions test_gitcomp() and test_gitcomp_nl() leak
the $cur variable into the test environment.  Since this variable has
a special role in the Bash completion script (it holds the word
currently being completed) it influences the behavior of most
completion functions and thus this leakage could interfere with
subsequent tests.  Although there are no such issues in the current
tests, early versions of the new tests that will be added later in
this series suffered because of this.

It's better to play safe and declare $cur local in those test helper
functions.  'local' is bashism, of course, but the tests of the Bash
completion script are run under Bash anyway, and there are already
other variables declared local in this test script.

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

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f490c1d05..294a08cfe 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -98,7 +98,7 @@ test_gitcomp ()
 {
local -a COMPREPLY &&
sed -e 's/Z$//' >expected &&
-   cur="$1" &&
+   local cur="$1" &&
shift &&
__gitcomp "$@" &&
print_comp &&
@@ -113,7 +113,7 @@ test_gitcomp_nl ()
 {
local -a COMPREPLY &&
sed -e 's/Z$//' >expected &&
-   cur="$1" &&
+   local cur="$1" &&
shift &&
__gitcomp_nl "$@" &&
print_comp &&
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 07/21] completion: ensure that the repository path given on the command line exists

2017-02-02 Thread SZEDER Gábor
The __gitdir() helper function prints the path to the git repository
to its stdout or stays silent and returns with error when it can't
find a repository or when the repository given via $GIT_DIR doesn't
exist.

This is not the case, however, when the path in $__git_dir, i.e. the
path to the repository specified on the command line via 'git
--git-dir=', doesn't exist: __gitdir() still outputs it as if it
were a real existing repository, making some completion functions
believe that they operate on an existing repository.

Check that the path in $__git_dir exists and return with error without
printing anything to stdout if it doesn't.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 1 +
 t/t9902-completion.sh  | 8 
 2 files changed, 9 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index ee6fb2259..5b2bd6721 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -40,6 +40,7 @@ __gitdir ()
 {
if [ -z "${1-}" ]; then
if [ -n "${__git_dir-}" ]; then
+   test -d "$__git_dir" || return 1
echo "$__git_dir"
elif [ -n "${GIT_DIR-}" ]; then
test -d "${GIT_DIR-}" || return 1
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7956cb9b1..7667baabf 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -211,6 +211,14 @@ test_expect_success '__gitdir - $GIT_DIR set while .git 
directory in parent' '
test_cmp expected "$actual"
 '
 
+test_expect_success '__gitdir - non-existing path in $__git_dir' '
+   (
+   __git_dir="non-existing" &&
+   test_must_fail __gitdir >"$actual"
+   ) &&
+   test_must_be_empty "$actual"
+'
+
 test_expect_success '__gitdir - non-existing $GIT_DIR' '
(
GIT_DIR="$ROOT/non-existing" &&
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 16/21] completion: respect 'git -C '

2017-02-02 Thread SZEDER Gábor
'git -C ' option(s) on the command line should be taken into
account during completion, because

  - like '--git-dir=', it can lead us to a different repository,

  - a few git commands executed in the completion script do care about
in which directory they are executed, and

  - the command for which we are providing completion might care about
in which directory it will be executed.

However, unlike '--git-dir=', the '-C ' option can be
specified multiple times and their effect is cumulative, so we can't
just store a single '' in a variable.  Nor can we simply
concatenate a path from '-C  -C  ...', because e.g. (in
an arguably pathological corner case) a relative path might be
followed by an absolute path.

Instead, store all '-C ' options word by word in the
$__git_C_args array in the main git completion function, and pass this
array, if present, to 'git rev-parse --absolute-git-dir' when
discovering the repository in __gitdir(), and let it take care of
multiple options, relative paths, absolute paths and everything.

Also pass all '-C  options via the $__git_C_args array to those
git executions which require a worktree and for which it matters from
which directory they are executed from.  There are only three such
cases:

  - 'git diff-index' and 'git ls-files' in __git_ls_files_helper()
used for git-aware filename completion, and

  - the 'git ls-tree' used for completing the 'ref:path' notation.

The other git commands executed in the completion script don't need
these '-C ' options, because __gitdir() already took those
options into account.  It would not hurt them, either, but let's not
induce unnecessary code churn.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 19 ++--
 t/t9902-completion.sh  | 87 ++
 2 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index ac5eb9ced..e003afd54 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -39,7 +39,11 @@ esac
 __gitdir ()
 {
if [ -z "${1-}" ]; then
-   if [ -n "${__git_dir-}" ]; then
+   if [ -n "${__git_C_args-}" ]; then
+   git "${__git_C_args[@]}" \
+   ${__git_dir:+--git-dir="$__git_dir"} \
+   rev-parse --absolute-git-dir 2>/dev/null
+   elif [ -n "${__git_dir-}" ]; then
test -d "$__git_dir" || return 1
echo "$__git_dir"
elif [ -n "${GIT_DIR-}" ]; then
@@ -286,10 +290,10 @@ __git_ls_files_helper ()
local dir="$(__gitdir)"
 
if [ "$2" == "--committable" ]; then
-   git --git-dir="$dir" -C "$1" diff-index --name-only --relative 
HEAD
+   git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C 
"$1" diff-index --name-only --relative HEAD
else
# NOTE: $2 is not quoted in order to support multiple options
-   git --git-dir="$dir" -C "$1" ls-files --exclude-standard $2
+   git ${__git_C_args:+"${__git_C_args[@]}"} --git-dir="$dir" -C 
"$1" ls-files --exclude-standard $2
fi 2>/dev/null
 }
 
@@ -519,7 +523,7 @@ __git_complete_revlist_file ()
*)   pfx="$ref:$pfx" ;;
esac
 
-   __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" 
2>/dev/null \
+   __gitcomp_nl "$(git ${__git_C_args:+"${__git_C_args[@]}"} 
--git-dir="$(__gitdir)" ls-tree "$ls" 2>/dev/null \
| sed '/^100... blob /{
   s,^.*,,
   s,$, ,
@@ -2792,6 +2796,7 @@ _git_worktree ()
 __git_main ()
 {
local i c=1 command __git_dir
+   local __git_C_args C_args_count=0
 
while [ $c -lt $cword ]; do
i="${words[c]}"
@@ -2800,7 +2805,11 @@ __git_main ()
--git-dir)   ((c++)) ; __git_dir="${words[c]}" ;;
--bare)  __git_dir="." ;;
--help) command="help"; break ;;
-   -c|-C|--work-tree|--namespace) ((c++)) ;;
+   -c|--work-tree|--namespace) ((c++)) ;;
+   -C) __git_C_args[C_args_count++]=-C
+   ((c++))
+   __git_C_args[C_args_count++]="${words[c]}"
+   ;;
-*) ;;
*) command="$i"; break ;;
esac
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index be2ed8704..984df05b2 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -211,6 +211,87 @@ test_expect_success '__gitdir - $GIT_DIR set while .git 
directory in parent' '
test_cmp expected "$actual"
 '
 
+test_expect_success '__gitdir - from command line while "git -C"' '
+   echo "$ROOT/.git" >expected &&
+   

[PATCHv2 11/21] completion: don't list 'HEAD' when trying refs completion outside of a repo

2017-02-02 Thread SZEDER Gábor
When refs completion is attempted while not in a git repository, the
completion script offers 'HEAD' erroneously.

Check early in __git_refs() that there is either a repository or a
remote to work on, and return early if neither is given.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 8 ++--
 t/t9902-completion.sh  | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index fd0ba1f3b..6b489b7c8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -346,7 +346,11 @@ __git_refs ()
local list_refs_from=path remote="${1-}"
local format refs pfx
 
-   if [ -n "$remote" ]; then
+   if [ -z "$remote" ]; then
+   if [ -z "$dir" ]; then
+   return
+   fi
+   else
if __git_is_configured_remote "$remote"; then
# configured remote takes precedence over a
# local directory with the same name
@@ -360,7 +364,7 @@ __git_refs ()
fi
fi
 
-   if [ "$list_refs_from" = path ] && [ -d "$dir" ]; then
+   if [ "$list_refs_from" = path ]; then
case "$cur" in
refs|refs/*)
format="refname"
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a201b5212..5b4defaa5 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -599,7 +599,7 @@ test_expect_success '__git_refs - non-existing URL remote - 
full refs' '
test_must_be_empty "$actual"
 '
 
-test_expect_failure '__git_refs - not in a git repository' '
+test_expect_success '__git_refs - not in a git repository' '
(
GIT_CEILING_DIRECTORIES="$ROOT" &&
export GIT_CEILING_DIRECTORIES &&
-- 
2.11.0.555.g967c1bcb3



[PATCHv2 12/21] completion: list short refs from a remote given as a URL

2017-02-02 Thread SZEDER Gábor
e832f5c09680 (completion: avoid ls-remote in certain scenarios,
2013-05-28) turned a 'git ls-remote ' query into a 'git
for-each-ref refs/remotes//' to improve responsiveness of
remote refs completion by avoiding potential network communication.
However, it inadvertently made impossible to complete short refs from
a remote given as a URL, e.g. 'git fetch git://server.com/repo.git
', because there is, of course, no such thing as
'refs/remotes/git://server.com/repo.git'.

Since the previous commit we tell apart configured remotes, i.e. those
that can have a hierarchy under 'refs/remotes/', from others that
don't, including remotes given as URL, so we know when we can't use
the faster 'git for-each-ref'-based approach.

Resurrect the old, pre-e832f5c09680 'git ls-remote'-based code for the
latter case to support listing short refs from remotes given as a URL.
The code is slightly updated from the original to

  - take into account the path to the repository given on the command
line (if any), and
  - omit 'ORIG_HEAD' from the query, as 'git ls-remote' will never
list it anyway.

When the remote given to __git_refs() doesn't exist, then it will be
handled by this resurrected 'git ls-remote' query.  This code path
doesn't list 'HEAD' unconditionally, which has the nice side effect of
fixing two more expected test failures.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 19 ---
 t/t9902-completion.sh  |  6 +++---
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 6b489b7c8..4ded44977 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -338,6 +338,7 @@ __git_tags ()
 # Lists refs from the local (by default) or from a remote repository.
 # It accepts 0, 1 or 2 arguments:
 # 1: The remote to list refs from (optional; ignored, if set but empty).
+#Can be the name of a configured remote, a path, or a URL.
 # 2: In addition to local refs, list unique branches from refs/remotes/ for
 #'git checkout's tracking DWIMery (optional; ignored, if set but empty).
 __git_refs ()
@@ -410,9 +411,21 @@ __git_refs ()
done
;;
*)
-   echo "HEAD"
-   git --git-dir="$dir" for-each-ref --format="%(refname:short)" \
-   "refs/remotes/$remote/" 2>/dev/null | sed -e 
"s#^$remote/##"
+   if [ "$list_refs_from" = remote ]; then
+   echo "HEAD"
+   git --git-dir="$dir" for-each-ref 
--format="%(refname:short)" \
+   "refs/remotes/$remote/" 2>/dev/null | sed -e 
"s#^$remote/##"
+   else
+   git --git-dir="$dir" ls-remote "$remote" HEAD \
+   "refs/tags/*" "refs/heads/*" "refs/remotes/*" 
2>/dev/null |
+   while read -r hash i; do
+   case "$i" in
+   *^{})   ;;
+   refs/*) echo "${i#refs/*/}" ;;
+   *)  echo "$i" ;;  # symbolic refs
+   esac
+   done
+   fi
;;
esac
 }
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 5b4defaa5..500505dca 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -540,7 +540,7 @@ test_expect_success '__git_refs - configured remote - 
remote name matches a dire
test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - URL remote' '
+test_expect_success '__git_refs - URL remote' '
cat >expected <<-EOF &&
HEAD
branch-in-other
@@ -567,7 +567,7 @@ test_expect_success '__git_refs - URL remote - full refs' '
test_cmp expected "$actual"
 '
 
-test_expect_failure '__git_refs - non-existing remote' '
+test_expect_success '__git_refs - non-existing remote' '
(
cur= &&
__git_refs non-existing >"$actual"
@@ -583,7 +583,7 @@ test_expect_success '__git_refs - non-existing remote - 
full refs' '
test_must_be_empty "$actual"
 '
 
-test_expect_failure '__git_refs - non-existing URL remote' '
+test_expect_success '__git_refs - non-existing URL remote' '
(
cur= &&
__git_refs "file://$ROOT/non-existing" >"$actual"
-- 
2.11.0.555.g967c1bcb3



RE

2017-02-02 Thread Aleena Aasim Abdulaziz




Assalam Alaikum, how are you doing my friend? my name is Madam Aleena  
Aasim Abdulaziz from Turkey and i have something very important to  
discuss with you please contact me now on my private email:  
aleen@hotmail.com




[PATCH] reset: add an example of how to split a commit into two

2017-02-02 Thread Jacob Keller
From: Jacob Keller 

It is sometimes useful to break a commit into parts to more logically
show how the code changes. There are many possible ways to achieve this
result, but one simple and powerful one is to use git reset -p.

Add an example to the documentation showing how this can be done so that
users are more likely to discover this, instead of inventing more
painful methods such as re-writing code from scratch, or duplicating git
add -p more times than necessary.

Signed-off-by: Jacob Keller 
---
 Documentation/git-reset.txt | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 25432d9257f9..4adac7a25bf9 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -292,6 +292,29 @@ $ git reset --keep start<3>
 <3> But you can use "reset --keep" to remove the unwanted commit after
 you switched to "branch2".
 
+Split a commit into two::
++
+Suppose that you have created a commit, but later decide that you want to split
+the changes into two separate commits, including only part of the old commit
+into the first commit, and including the rest as a separate commit on top. You
+can use git reset in patch mode to interactively select which hunks to split
+out into a separate commit.
++
+
+$ git reset -p HEAD^<1>
+$ git commit --amend<2>
+$ git commit ...<3>
+
++
+<1> This lets you interactively undo changes between HEAD^ and HEAD, so you can
+select which parts to remove from the initial commit. The changes are
+placed into the index, leaving the working tree untouched.
+<2> Now, you ammend the initial commit with the modifications that you just
+made in the index.
+<3> Finally, you can add and then commit the final original unmodified files
+back as the second commit, enabling you to logically separate a commit
+into a sequence of two commits instead.
+
 
 DISCUSSION
 --
-- 
2.11.0.864.ge7592a54611d



What's cooking in git.git (Feb 2017, #01; Thu, 2)

2017-02-02 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

The tip of 'master' has most of the topics (and all the big ones)
that should be in the upcoming release.  I'll tag 2.12-rc0 sometime
tomorrow.  On the 'maint' front, Git 2.11.1 has been tagged with
bugfixes that are already in 'master'.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* bc/use-asciidoctor-opt (2017-01-31) 8 commits
  (merged to 'next' on 2017-01-31 at f2a641f6f3)
 + Documentation: implement linkgit macro for Asciidoctor
 + Makefile: add a knob to enable the use of Asciidoctor
 + Documentation: move dblatex arguments into variable
 + Documentation: add XSLT to fix DocBook for Texinfo
 + Documentation: sort sources for gitman.texi
 + Documentation: remove unneeded argument in cat-texi.perl
 + Documentation: modernize cat-texi.perl
 + Documentation: fix warning in cat-texi.perl

 Asciidoctor, an alternative reimplementation of AsciiDoc, still
 needs some changes to work with documents meant to be formatted
 with AsciiDoc.  "make USE_ASCIIDOCTOR=YesPlease" to use it out of
 the box to document our pages is getting closer to reality.


* cw/doc-sign-off (2017-01-27) 1 commit
  (merged to 'next' on 2017-01-31 at 133cc2886d)
 + doc: clarify distinction between sign-off and pgp-signing

 Doc update.


* ep/commit-static-buf-cleanup (2017-01-31) 2 commits
  (merged to 'next' on 2017-01-31 at 02d3c25219)
 + builtin/commit.c: switch to strbuf, instead of snprintf()
 + builtin/commit.c: remove the PATH_MAX limitation via dynamic allocation

 Code clean-up.


* gv/mingw-p4-mapuser (2017-01-30) 1 commit
  (merged to 'next' on 2017-01-31 at 5a9f2c96f6)
 + git-p4: fix git-p4.mapUser on Windows

 "git p4" did not work well with multiple git-p4.mapUser entries on
 Windows.


* hv/mingw-help-is-executable (2017-01-30) 1 commit
  (merged to 'next' on 2017-01-31 at 89aae8d018)
 + help: improve is_executable() on Windows

 "git help" enumerates executable files in $PATH; the implementation
 of "is this file executable?" on Windows has been optimized.


* js/mingw-hooks-with-exe-suffix (2017-01-30) 1 commit
  (merged to 'next' on 2017-01-31 at 3b7863c578)
 + mingw: allow hooks to be .exe files

 Names of the various hook scripts must be spelled exactly, but on
 Windows, an .exe binary must be named with .exe suffix; notice
 $GIT_DIR/hooks/.exe as a valid  hook.


* js/retire-relink (2017-01-25) 2 commits
  (merged to 'next' on 2017-01-31 at c6c6f9b902)
 + relink: really remove the command
 + relink: retire the command

 Cruft removal.


* js/status-pre-rebase-i (2017-01-26) 1 commit
  (merged to 'next' on 2017-01-31 at 09e51b2e39)
 + status: be prepared for not-yet-started interactive rebase

 After starting "git rebase -i", which first opens the user's editor
 to edit the series of patches to apply, but before saving the
 contents of that file, "git status" failed to show the current
 state (i.e. you are in an interactive rebase session, but you have
 applied no steps yet) correctly.


* js/unzip-in-usr-bin-workaround (2017-01-27) 1 commit
  (merged to 'next' on 2017-01-31 at 515d1d1f90)
 + test-lib: on FreeBSD, look for unzip(1) in /usr/local/bin/

 Test tweak for FreeBSD where /usr/bin/unzip is unsuitable to run
 our tests but /usr/local/bin/unzip is usable.


* mm/reset-facl-before-umask-test (2017-01-30) 1 commit
  (merged to 'next' on 2017-01-31 at 4a2031e49c)
 + t0001: don't let a default ACL interfere with the umask test

 Test tweaks for those who have default ACL in their git source tree
 that interfere with the umask test.


* nd/log-graph-configurable-colors (2017-01-31) 4 commits
  (merged to 'next' on 2017-01-31 at 36df9e2376)
 + color_parse_mem: allow empty color spec
  (merged to 'next' on 2017-01-23 at c369982ad8)
 + log --graph: customize the graph lines with config log.graphColors
 + color.c: trim leading spaces in color_parse_mem()
 + color.c: fix color_parse_mem() with value_len == 0

 Some people feel the default set of colors used by "git log --graph"
 rather limiting.  A mechanism to customize the set of colors has
 been introduced.


* rs/absolute-pathdup (2017-01-27) 2 commits
  (merged to 'next' on 2017-01-31 at f751f64876)
 + use absolute_pathdup()
 + abspath: add absolute_pathdup()

 Code cleanup.


* rs/receive-pack-cleanup (2017-01-30) 1 commit
  (merged to 'next' on 2017-01-31 at d660881f69)
 + receive-pack: call string_list_clear() unconditionally

 Code clean-up.


* sb/submodule-add-force (2016-11-29) 1 commit
 + submodule add: extend force flag to add existing repos
 (this branch is used by sb/push-make-submodule-check-the-def

[ANNOUNCE] Git v2.11.1

2017-02-02 Thread Junio C Hamano
The latest maintenance release Git v2.11.1 is now available at
the usual places.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.11.1'
tag and the 'maint' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git



Git v2.11.1 Release Notes
=

Fixes since v2.11
-

 * The default Travis-CI configuration specifies newer P4 and GitLFS.

 * The character width table has been updated to match Unicode 9.0

 * Update the isatty() emulation for Windows by updating the previous
   hack that depended on internals of (older) MSVC runtime.

 * "git rev-parse --symbolic" failed with a more recent notation like
   "HEAD^-1" and "HEAD^!".

 * An empty directory in a working tree that can simply be nuked used
   to interfere while merging or cherry-picking a change to create a
   submodule directory there, which has been fixed..

 * The code in "git push" to compute if any commit being pushed in the
   superproject binds a commit in a submodule that hasn't been pushed
   out was overly inefficient, making it unusable even for a small
   project that does not have any submodule but have a reasonable
   number of refs.

 * "git push --dry-run --recurse-submodule=on-demand" wasn't
   "--dry-run" in the submodules.

 * The output from "git worktree list" was made in readdir() order,
   and was unstable.

 * mergetool..trustExitCode configuration variable did not apply
   to built-in tools, but now it does.

 * "git p4" LFS support was broken when LFS stores an empty blob.

 * Fix a corner case in merge-recursive regression that crept in
   during 2.10 development cycle.

 * Update the error messages from the dumb-http client when it fails
   to obtain loose objects; we used to give sensible error message
   only upon 404 but we now forbid unexpected redirects that needs to
   be reported with something sensible.

 * When diff.renames configuration is on (and with Git 2.9 and later,
   it is enabled by default, which made it worse), "git stash"
   misbehaved if a file is removed and another file with a very
   similar content is added.

 * "git diff --no-index" did not take "--no-abbrev" option.

 * "git difftool --dir-diff" had a minor regression when started from
   a subdirectory, which has been fixed.

 * "git commit --allow-empty --only" (no pathspec) with dirty index
   ought to be an acceptable way to create a new commit that does not
   change any paths, but it was forbidden, perhaps because nobody
   needed it so far.

 * A pathname that begins with "//" or "\\" on Windows is special but
   path normalization logic was unaware of it.

 * "git pull --rebase", when there is no new commits on our side since
   we forked from the upstream, should be able to fast-forward without
   invoking "git rebase", but it didn't.

 * The way to specify hotkeys to "xxdiff" that is used by "git
   mergetool" has been modernized to match recent versions of xxdiff.

 * Unlike "git am --abort", "git cherry-pick --abort" moved HEAD back
   to where cherry-pick started while picking multiple changes, when
   the cherry-pick stopped to ask for help from the user, and the user
   did "git reset --hard" to a different commit in order to re-attempt
   the operation.

 * Code cleanup in shallow boundary computation.

 * A recent update to receive-pack to make it easier to drop garbage
   objects made it clear that GIT_ALTERNATE_OBJECT_DIRECTORIES cannot
   have a pathname with a colon in it (no surprise!), and this in turn
   made it impossible to push into a repository at such a path.  This
   has been fixed by introducing a quoting mechanism used when
   appending such a path to the colon-separated list.

 * The function usage_msg_opt() has been updated to say "fatal:"
   before the custom message programs give, when they want to die
   with a message about wrong command line options followed by the
   standard usage string.

 * "git index-pack --stdin" needs an access to an existing repository,
   but "git index-pack file.pack" to generate an .idx file that
   corresponds to a packfile does not.

 * Fix for NDEBUG builds.

 * A lazy "git push" without refspec did not internally use a fully
   specified refspec to perform 'current', 'simple', or 'upstream'
   push, causing unnecessary "ambiguous ref" errors.

 * "git p4" misbehaved when swapping a directory and a symbolic link.

 * Even though an fix was attempted in Git 2.9.3 days, but running
   "git difftool --dir-diff" from a subdirectory never worked. This
   has been fixed.

 * "git p4" that tracks multile p4 paths imported a single changelist
   that touches files 

Re: git-scm.com status report

2017-02-02 Thread Samuel Lijin
For anyone interested, this thread is on the HN front page right now[0].

There's one suggestion in particular that stands out to me - shifting
to Digital Ocean[1], which for $240/mo offers wa more than what it
sounds like the current Heroku costs are.

[0] https://news.ycombinator.com/item?id=13554065
[1] https://news.ycombinator.com/item?id=13554632

On Thu, Feb 2, 2017 at 4:01 PM, pedro rijo  wrote:
> Hey,
>
> While I’m not experienced with Rails apps, I would like to give my
> contribution to the Git project. I could help doing some kind of triage,
> removing abusing PRs/issues (like
> https://github.com/git/git-scm.com/pull/557), looking for typos and other
> tasks that wouldn’t require a lot of RoR knowledge to get start. Also,
> completely free and available to start digging into the RoR stuff of course!
>
> If you are interested, just let me know :)
>
> Thanks,
> Pedro Rijo


[PATCH v2] git-prompt.sh: add submodule indicator

2017-02-02 Thread Benjamin Fuchs
Hi everyone again,
I guess this time I'm rerolling my patch the right way.
Thanks again Gábor for your feedback.
And thanks to Junio for being patient and explaining the reroll workflow 
Greeting,
Benjamin

Benjamin Fuchs (1):
  git-prompt.sh: add submodule indicator

 contrib/completion/git-prompt.sh | 40 ++-
 t/t9903-bash-prompt.sh   | 59 
 2 files changed, 98 insertions(+), 1 deletion(-)

-- 
2.7.4



[PATCH v2] git-prompt.sh: add submodule indicator

2017-02-02 Thread Benjamin Fuchs
I expirienced that working with submodules can be confusing. A submodule can be
anywhere in your parent git repository. While walking through the parent
repository it would be good to have a reminder to know when you entered a
submodule.

This new indicator will show if you are in a submodule. The new prompt will look
like this: (sub:master). If the currently checked out submodule commit does not
match the SHA-1 found in the index of the containing repository a "+" will be
prepended (+sub:master). Adding a new optional env variable for the new feature.

Signed-off-by: Benjamin Fuchs 
---
 contrib/completion/git-prompt.sh | 40 ++-
 t/t9903-bash-prompt.sh   | 59 
 2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 97eacd7..9a0c4af 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -93,6 +93,13 @@
 # directory is set up to be ignored by git, then set
 # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
 # repository level by setting bash.hideIfPwdIgnored to "false".
+#
+# If you would like __git_ps1 to indicate that you are in a submodule,
+# set GIT_PS1_SHOWSUBMODULE to a nonempty value. In this case the name
+# of the submodule will be prepended to the branch name (e.g. module:master).
+# If you set GIT_PS1_SHOWDIRTYSTATE to a nonempty value, the name will be
+# prepended by "+" if the currently checked out submodule commit does not
+# match the SHA-1 found in the index of the containing repository.
 
 # check whether printf supports -v
 __git_printf_supports_v=
@@ -284,6 +291,32 @@ __git_eread ()
test -r "$f" && read "$@" <"$f"
 }
 
+# __git_ps1_submodule
+# Requires the git dir set in $g by the caller.
+# Returns the name of the submodule in $sub if we are currently inside one.
+# The name will be prepended by "+" if the currently checked out submodule 
commit
+# does not match the SHA-1 found in the index of the containing repository.
+# NOTE: git_dir looks like in a ...
+# - submodule: "GIT_PARENT/.git/modules/PATH_TO_SUBMODULE"
+# - parent:"GIT_PARENT/.git" (exception "." in .git)
+__git_ps1_submodule ()
+{
+   local git_dir="$g"
+   local submodule_name="$(basename "$git_dir")"
+   if [ "$submodule_name" != ".git" ] && [ "$submodule_name" != "." ]; then
+   local status=""
+   if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&
+  [ "$(git config --bool bash.showDirtyState)" != "false" ]
+   then
+   local parent_top="${git_dir%.git*}"
+   local submodule_top="${git_dir#*modules/}"
+   git -C "$parent_top" diff --no-ext-diff 
--ignore-submodules=dirty --quiet -- "$submodule_top" 2>/dev/null || status="+"
+   fi
+
+   sub="$status$submodule_name:"
+   fi
+}
+
 # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
 # when called from PS1 using command substitution
 # in this mode it prints text to add to bash PS1 prompt (includes branch name)
@@ -513,8 +546,13 @@ __git_ps1 ()
b="\${__git_ps1_branch_name}"
fi
 
+   local sub=""
+   if [ -n "${GIT_PS1_SHOWSUBMODULE-}" ]; then
+   __git_ps1_submodule
+   fi
+
local f="$w$i$s$u"
-   local gitstring="$c$b${f:+$z$f}$r$p"
+   local gitstring="$c$sub$b${f:+$z$f}$r$p"
 
if [ $pcmode = yes ]; then
if [ "${__git_printf_supports_v-}" != yes ]; then
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 97c9b32..ac82c99 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -16,9 +16,22 @@ c_lblue='\\[\\e[1;34m\\]'
 c_clear='\\[\\e[0m\\]'
 
 test_expect_success 'setup for prompt tests' '
+   mkdir .subrepo &&
+   (cd .subrepo &&
+   git init &&
+   echo 1 >file &&
+   git add file &&
+   git commit -m initial &&
+   git checkout -b dirty &&
+   echo 2 >file &&
+   git commit -m "dirty branch" file
+   ) &&
git init otherrepo &&
echo 1 >file &&
git add file &&
+   git submodule add ./.subrepo sub &&
+   git -C sub checkout master &&
+   git add sub &&
test_tick &&
git commit -m initial &&
git tag -a -m msg1 t1 &&
@@ -755,4 +768,50 @@ test_expect_success 'prompt - hide if pwd ignored - inside 
gitdir (stderr)' '
test_cmp expected "$actual"
 '
 
+test_expect_success 'prompt - submodule indicator' '
+   printf " (sub:master)" >expected &&
+   (
+   cd sub &&
+   GIT_PS1_SHOWSUBMODULE=y &&
+   __git_ps1 >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - submodule indicator - disabled' '
+   printf " (master)" >expected &&
+   (
+

Re: [PATCH] ls-remote: add "--diff" option to show only refs that differ

2017-02-02 Thread Linus Torvalds
On Thu, Feb 2, 2017 at 1:05 PM, Junio C Hamano  wrote:
>
> Another case I can think of that "--diff" would help is when you are
> inspecting your own mirror (but that can be seen as a special case
> of the "they have copies of yours plus their own", if you think of
> your mirror as "them" and the difference is "being stale").

Yeah, that's actually what I did for some testing (not having stale
branches, but just to check the expected differences for my upstream
kernel repo with my local pulls that I haven't pushed out yet).

The actual real use-case is something that only happens for me only
very occasionally. I end up sending out "did you forget to push"
emails perhaps a couple of times every release, but every time I do I
will have gone and done a ls-remote on their repo..

 Linus


Re: [PATCH] ls-remote: add "--diff" option to show only refs that differ

2017-02-02 Thread Junio C Hamano
Linus Torvalds  writes:

> I basically don't see downstream contributor doing ls-remote, it's a
> upstream maintainer command.
>
> But that may be a lack of imagination on my part.

I actually share that perception.  For the "downstream wonders about
the state of the origin" usecase, I would rather recommend "fetch",
either without "-n" (when the downstream does not value the current
state of refs/remotes/*) or with "-n" (when it does for whatever
reason).

>> When one contributor asks you to pull refs/heads/master you want to
>> go and see if it is different from refs/heads/master you have?
>
> No. What happens is that people ask me to do something like
>
> git pull ..some-target.. tags/for-linus-3
>
> and the pull fails because there is no such tag. That's when I go "ok,
> they screwed up, let's see what they *meant* for me to pull", and I go
> "git ls-remote".

In that context, I fully agree that "--diff --tags" would help.  The
copies of your tags they have there would overwhelm what you are
really looking for in the output from the command.

And if they asked you to pull "for-linus-3" branch, which is buried
in many other branches (perhaps their publishing repository they ask
you to pull from is also serving as their back-up place, and the
local branches they use before coming up with something pull-able
are all there), then "--diff --refs" would still help by culling
tags that originated from you.

> I agree that it's a specialized case, but I also think it's the _main_
> case for ls-remote in the first place (apart from some scripting to
> check for updates or whatever).
>
> But maybe more people use ls-remote than I think they do (and in
> different ways than what I envision).

You and I are not the only folks in the world, but I agree with you
in thinking that "ls-remote" is not something you would use on
'origin' as a downstream contributor or a consumer.  

Another case I can think of that "--diff" would help is when you are
inspecting your own mirror (but that can be seen as a special case
of the "they have copies of yours plus their own", if you think of
your mirror as "them" and the difference is "being stale").


Re: [PATCH] ls-remote: add "--diff" option to show only refs that differ

2017-02-02 Thread Linus Torvalds
On Thu, Feb 2, 2017 at 12:03 PM, Junio C Hamano  wrote:
>
> Most downstream folks seem to care about refs/remotes/origin/$branch
> and I think in that context "git ls-remote --diff [origin]" that
> compares their refs/heads/* and refs/remotes/origin/* would make
> sense.

Hmm. Maybe. The main target for noise reduction for me was actually
all the shared tags.

Which doesn't have that issue.

Also, I've never ever used "git ls-remote" on origin. Do people
actually do that? Why would a regular user ever use ls-remote in the
first place?

The only reason I've ever had for using ls-remote is exactly because
the remote is somehow "odd", and the _normal_ flow didn't work, so you
want to start investigating. So by definition (at least for me),
ls-remote is not part of a good normal flow.

So I kind of see where you are coming from, but I don't really see
that as being a normal workflow for me - or really anybody.

What I think *your* use case is would be more for a workflow along the lines of

   # update the remote data
   git fetch [origin]

   # have some way to just see what branches are not the same
   git status --all

or something ("git status" already talks about the status of the
current branch vs the origin branch).

> Your has_ref_locally() seems to return true by comparing
> their value with the value of the local ref without any the fetch
> refspec mapping.

Right. Because I see the use of "ls-remote" being mostly for
maintainer pulls, and the "origin" in many ways would be the other way
around (and you wouldn't even know what the name of said origin would
be locally).

I basically don't see downstream contributor doing ls-remote, it's a
upstream maintainer command.

But that may be a lack of imagination on my part.

> When one contributor asks you to pull refs/heads/master you want to
> go and see if it is different from refs/heads/master you have?

No. What happens is that people ask me to do something like

git pull ..some-target.. tags/for-linus-3

and the pull fails because there is no such tag. That's when I go "ok,
they screwed up, let's see what they *meant* for me to pull", and I go
"git ls-remote".

In other words, I don't see anybody ever using git ls-remote if they
already know what the remote is. That's why I don't see "origin" to be
an issue - origin is by definition somethinig you trust, and you just
fetch and pull from.

So the only reason I've ever had for using ls-remote is literally "ok,
what the hell is going on at that remote repo".

And then it is generally a bare repository, and it generally does
*not* have remote branches in it at all.  But it *does* generally end
up having all the basic branches and tags (not always, but it's very
common).

Which is why I as a maintainer then want to just weed out anything
that is already my usual branches that everybody downstream already
has.

I agree that it's a specialized case, but I also think it's the _main_
case for ls-remote in the first place (apart from some scripting to
check for updates or whatever).

But maybe more people use ls-remote than I think they do (and in
different ways than what I envision).

 Linus


Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Junio C Hamano
Johannes Schindelin  writes:

> Also, the more important reply was Peff's reply that suggested that the
> proposed fix was incomplete, as it misses --unpack-unreachable:
> https://public-inbox.org/git/20160601160143.ga9...@sigill.intra.peff.net/

While I think that --unpack-unreachable thing is a symptom of the
basic approach of patching things up at the wrong level, if you are
hinting that fix to the issue that gc does not pay attention to
various anchoring point in other worktrees is more important than
adding new commands to "worktree", I fully agree with that.  

We have a basic structure of "worktree" working well enough to allow
adventurous folks to experiment (there is a reason why we keep
calling it experimental).  mv and other additions are primarily to
make things _easier_ to use, but we shouldn't be encouraging its use
to general public by making it easier for them to hurt themselves,
if we know there still are sharp edges.

Thanks for bringing it up.


Re: [PATCH] ls-remote: add "--diff" option to show only refs that differ

2017-02-02 Thread Junio C Hamano
Linus Torvalds  writes:

> My main use of "git ls-remote" tends to be to check what the other end
> has when some pull request goes wrong (they forgot to push, or they used
> the wrong ref name or whatever), and it ends up being hard to see all
> the relevant data from the noise of people just having the same basic
> tags etc from upstream.
>
> So this adds a "--diff" option that shows only the refs that are
> different from the local repository.  So when somebody asks me to pull,
> I can now just trivially look at what they have that isn't already my
> basic branches and tags.

Most downstream folks seem to care about refs/remotes/origin/$branch
and I think in that context "git ls-remote --diff [origin]" that
compares their refs/heads/* and refs/remotes/origin/* would make
sense.  Your has_ref_locally() seems to return true by comparing
their value with the value of the local ref without any the fetch
refspec mapping.

When one contributor asks you to pull refs/heads/master you want to
go and see if it is different from refs/heads/master you have?

> Comments?
>
> +static int has_ref_locally(const struct ref *ref)
> +{
> + unsigned char sha1[20];
> +
> + if (!resolve_ref_unsafe(ref->name, RESOLVE_REF_READING, sha1, NULL))
> + return 0;
> +
> + return !hashcmp(ref->old_oid.hash, sha1);
> +}
> +
> @@ -105,6 +121,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
>   continue;
>   if (!tail_match(pattern, ref->name))
>   continue;
> + if (diff && has_ref_locally(ref))
> + continue;
>   if (show_symref_target && ref->symref)
>   printf("ref: %s\t%s\n", ref->symref, ref->name);
>   printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);


[PATCH] ls-remote: add "--diff" option to show only refs that differ

2017-02-02 Thread Linus Torvalds

From: Linus Torvalds 
Date: Thu, 2 Feb 2017 11:37:49 -0800
Subject: [PATCH] ls-remote: add "--diff" option to show only refs that differ

My main use of "git ls-remote" tends to be to check what the other end
has when some pull request goes wrong (they forgot to push, or they used
the wrong ref name or whatever), and it ends up being hard to see all
the relevant data from the noise of people just having the same basic
tags etc from upstream.

So this adds a "--diff" option that shows only the refs that are
different from the local repository.  So when somebody asks me to pull,
I can now just trivially look at what they have that isn't already my
basic branches and tags.

Note that "--diff" implies "--refs" (ie it also disables showing peeled
tags).

Signed-off-by: Linus Torvalds 
---

This is not a big deal, but maybe others have the same issues I've had. 
And maybe nobody else ever uses "git ls-remote". I dunno.

Also, I considered adding this feature as a more generic flag to 
"check_ref_type()" (ie add a REF_NONLOCAL option to complete the existing 
REF_NORMAL/REF_HEAD/etc flags), but that would have been a more involved 
patch and I'm not convinced it makes much sense for any other use, so I 
made it a specific local hack to ls-remote instead.

Comments?

 builtin/ls-remote.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 66cdd45cc..23469c3a6 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "transport.h"
 #include "remote.h"
+#include "refs.h"
 
 static const char * const ls_remote_usage[] = {
N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=]\n"
@@ -31,6 +32,16 @@ static int tail_match(const char **pattern, const char *path)
return 0;
 }
 
+static int has_ref_locally(const struct ref *ref)
+{
+   unsigned char sha1[20];
+
+   if (!resolve_ref_unsafe(ref->name, RESOLVE_REF_READING, sha1, NULL))
+   return 0;
+
+   return !hashcmp(ref->old_oid.hash, sha1);
+}
+
 int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
const char *dest = NULL;
@@ -39,6 +50,7 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
int quiet = 0;
int status = 0;
int show_symref_target = 0;
+   int diff = 0;
const char *uploadpack = NULL;
const char **pattern = NULL;
 
@@ -62,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
N_("exit with exit code 2 if no matching refs are 
found"), 2),
OPT_BOOL(0, "symref", &show_symref_target,
 N_("show underlying ref in addition to the object 
pointed by it")),
+   OPT_BOOL(0, "diff", &diff,
+N_("show only refs that differ from local refs")),
OPT_END()
};
 
@@ -98,6 +112,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
if (transport_disconnect(transport))
return 1;
 
+   if (diff)
+   flags |= REF_NORMAL;
if (!dest && !quiet)
fprintf(stderr, "From %s\n", *remote->url);
for ( ; ref; ref = ref->next) {
@@ -105,6 +121,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
continue;
if (!tail_match(pattern, ref->name))
continue;
+   if (diff && has_ref_locally(ref))
+   continue;
if (show_symref_target && ref->symref)
printf("ref: %s\t%s\n", ref->symref, ref->name);
printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);


Re: [PATCH v3 00/27] Revamp the attribute system; another round

2017-02-02 Thread Junio C Hamano
Brandon Williams  writes:

> Per some of the discussion online and off I locally broke up up the question
> and answer and I wasn't very thrilled with the outcome for a number of 
> reasons.
>
> 1. The API is more complex
> 2. Performance hit
> ...
> Given the above, v3 is a reroll of the same design as in v2.  This is a good
> milestone in improving the attribute system as it achieves the goal of making
> the attribute subsystem thread-safe (ie multiple callers can be executing
> inside the attribute system at the same time) and will enable a future series
> to allow pathspec code to call into the attribute system.
>
> Most of the changes in this revision are cosmetic (variable renames, code
> movement, etc) but there was a memory leak that was also fixed.

I am OK with the patches presented in this round, but let me explain
why I still expect that we would eventually end up spliting the
question & answer into separate data structure before we can truly
go multi-threaded.

A typical application would do

for path taken from some set:
do_something(path)

and "do something with path" would be another helper function, which
may do

do_something(path):
ask 'text' attribute for the path
switch on the attribute and do different things

With the original API, the latter would statically allocate an array
of  pairs, with an optimization to populate
 which is immutable (because the codepath is always and
only interested in 'text' attribute, and you need a hash lookup to
intern the string "text" which costs cycles) only once, and make a
call to git_check_attr() function with the "path".  This obviously
will not work when two threads are calling this helper function, as
the threads both want their git_check_attr() to return their answers
to the array, but the  part are shared between the threads.

A naive and inefficient way to split questions and answers is to
have two arrays, allocating the former statically (protected under a
mutex, of course) to avoid repeated cost of interning, while
allocating the latter (and some working area per invocation, like
the check_all_attr[]) dynamically and place it on stack or heap.
Because do_something() will be called number of times, the cost for
allocation and initialization of the  part that is paid per
invocation will of course become very high.

We could in theory keep the original arrangement of having an
array of  pairs and restructure the code to do:

prepare the  array
for path taken from some set:
do_something(the array, path)

That way, do_something() do not have to keep allocating,
initializing and destroying the array.

But after looking at the current set of codepaths, before coming to
the conclusion that we need to split the static part that is
specific to the callsite for git_check_attr() and the dynamic part
that is specific to the  pair, I noticed that
typically the callers that can prepare the array before going into
the loop (which will eventually be spread across multiple threads)
are many levels away in the callchain, and they are not even aware
of what attributes are going to be requested in the leaf level
helper functions.  In other words, the approach to hoist "the
 array" up in the callchain would not scale.  A
caller that loops over paths in the index and check them out does
not want to know (and we do not want to tell it) what exact
attributes are involved in the decision convert_to_working_tree()
makes for each path, for example.

So how would we split questions and answers in a way that is not
naive and inefficient?  

I envision that we would allow the attribute subsystem to keep track
of the dynamic part, which will receive the answers, holds working
area like check_all_attr[], and has the equivalent to the "attr
stack", indexed by  pair (and the
identification of "callsite" can be done by using the address of the
static part, i.e. the array of questions that we initialize just
once when interning the list of attribute names for the first time).

The API to prepare and ask for attributes may look like:

static struct attr_static_part Q;
struct attr_dynamic_part *D;

attr_check_init(&Q, "text", ...);
D = git_attr_check(&Q, path);

where Q contains an array of interned attributes (i.e. questions)
and other immutable things that is unique to this callsite, but can
be shared across multiple threads asking the same question from
here.  As an internal implementation detail, it probably will have a
mutex to make sure that init will run only once.

Then the implementation of git_attr_check(&Q, path) would be:

- see if there is already the "dynaic part" allocated for the
  current thread asking the question Q.  If there is not,
  allocate one and remember it, so that it can be reused in
  later calls by the same thread; if there is, use that existing
  one.

- reinitialize the "dynamic part" as need

How to use git show's "%<([,trunc|ltrunc|mtrunc])"?

2017-02-02 Thread Hilco Wijbenga
Hi all,

I'm trying to get the committer date printed in a custom fashion.
Using "%cI" gets me close:

$ git show --format="%cI | %an" master | head -n 1
2017-01-31T17:02:13-08:00 | Hilco Wijbenga

I would like to get rid of the "-08:00" bit at the end of the
timestamp. According to the "git show" manual I should be able to use
"%<([,trunc|ltrunc|mtrunc])" to drop that last part.

$ git show --format="%<(19,trunc)cI | %an" master | head -n 1
cI | Hilco Wijbenga

Mmm, it seems to be recognized as a valid "something" but it's not
working as I had expected. :-) I tried several other versions of this
but no luck. Clearly, I'm misunderstanding the format description. How
do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?

Cheers,
Hilco


Re: [PATCH 4/7] completion: teach ls-remote to complete options

2017-02-02 Thread Junio C Hamano
Cornelius Weig  writes:

> On 02/02/2017 02:40 AM, SZEDER Gábor wrote:
>> 
>>> ls-remote needs to complete remote names and its own options.
>> 
>> And refnames, too.
>
> Yes, right. However, do you think it is reasonable to complete remote
> refnames? I don't think so, because it would mean we would have to run
> ls-remote during completion -- and waiting for ls-remote could be quite
> lengthy.

... and by the time the completion script knew what they are, we
have all information necessary without actually having the user run
the command ;-)  That does sound backwards.

I am however not sure what Szeder really meant by "refnames".  For
example, you may want to see that this

$ git ls-remote origin mast

peek into refs/remotes/origin/* and find those matching, i.e. most
likely "master", and that can be done without talking to the remote
side.  It won't catch the case where the remote end added a new
branch that also match, e.g. "mastiff", and it will actively harm
the users by giving the impression that Git (collectively, even
though from technical point of view, what the completion script does
is not part of Git) told them that there is no such new branch they
need to worry about.

So probably even with the "peek local" optimization, I have a feeling
that completion of refnames may not be such a good idea.


[PATCH] document behavior of empty color name

2017-02-02 Thread Jeff King
On Thu, Feb 02, 2017 at 04:16:15PM +0700, Duy Nguyen wrote:

> > I hadn't heard anything back,
> 
> Sorry I was accidentally busy during Luna new year holiday.

No problem. That sounds much more fun than working on Git. :)

> > -   if (!len)
> > -   return -1;
> > +   if (!len) {
> > +   dst[0] = '\0';
> > +   return 0;
> > +   }
> >  
> > if (!strncasecmp(ptr, "reset", len)) {
> > xsnprintf(dst, end - dst, GIT_COLOR_RESET);
> 
> I wonder if it makes more sense to do this in builtin/config.c. The
> "default value" business is strictly git-config command's. The parsing
> function does not need to know. Maybe something like this?

I don't think so. The default value is a git-config thing, but you would
want to be able to do the same thing in a config file. For example, to
disable coloring entirely for part of the diff, you could do:

  [color "diff"]
  meta = ""

> This is also a good opportunity to document this behavior in
> git-config.txt, I think.

Yeah. Maybe:

-- >8 --
Subject: [PATCH] document behavior of empty color name

Commit 55cccf4bb (color_parse_mem: allow empty color spec,
2017-02-01) clearly defined the behavior of an empty color
config variable. Let's document that, and give a hint about
why it might be useful.

It's important not to say that it makes the item uncolored,
because it doesn't. It just sets no attributes, which means
that any previous attributes continue to take effect.

Signed-off-by: Jeff King 
---
 Documentation/config.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 33a007b52..49b264566 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -170,6 +170,9 @@ The position of any attributes with respect to the colors
 be turned off by prefixing them with `no` or `no-` (e.g., `noreverse`,
 `no-ul`, etc).
 +
+An empty color string produces no color effect at all. This can be used
+to avoid coloring specific elements without disabling color entirely.
++
 For git's pre-defined color slots, the attributes are meant to be reset
 at the beginning of each item in the colored output. So setting
 `color.decorate.branch` to `black` will paint that branch name in a
-- 
2.11.0.948.gca97da533



Re: init --separate-git-dir does not set core.worktree

2017-02-02 Thread Kyle Meyer
Duy Nguyen  writes:

> On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer  wrote:
>>
>> As of 6311cfaf9 (init: do not set unnecessary core.worktree,
>> 2016-09-25), "git init --separate-git-dir" no longer sets core.worktree
>> (test below).  Based on the commit message and the corresponding thread
>> [1], I don't think this change in behavior was intentional, but I wasn't
>> able to understand things well enough to attempt a patch.
>
> I'm missing some context. Why does --separate-git-dir have to set
> core.worktree? What fails for you exactly?

Sorry for not providing enough information.  I haven't run into a
failure.

In Magit, we need to determine the top-level of the working tree from
COMMIT_EDITMSG.  Right now that logic [*1*] looks something like this:

 * COMMIT_EDITMSG in .git/modules//: set working tree to the
   output of "git rev-parse --show-toplevel"

 * COMMIT_EDITMSG in .git/worktrees//: set working tree to the
   path in .git/worktrees//gitdir, minus the trailing "/.git"

 * COMMIT_EDITMSG in .git: set working tree to the parent directory

This fails for a repo set up with --separate-git-dir [*2*], where the
last step will go out into an unrelated repo.  If core.worktree was set
and "git rev-parse --show-toplevel" returned the working tree like it
did for submodules, things would work.

Of course, the issue above isn't a reason that --separate-git-dir should
set core.worktree, but the submodule behavior is why we were wondering
if it should.  And so I was going to ask here whether core.worktree
should be set, but then I confused myself into thinking 6311cfaf9
unintentionally changed this behavior.


[*1*] This is done by magit-toplevel:
  
https://github.com/magit/magit/blob/e34f4e8eb00f292e8c475489fa7caa286857a421/lisp/magit-git.el#L400

[*2*] https://github.com/magit/magit/issues/2955
  https://github.com/magit/magit/issues/2981


Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Johannes Schindelin
Hi Duy,

On Thu, 2 Feb 2017, Johannes Schindelin wrote:

> Hi Duy,
> 
> On Thu, 2 Feb 2017, Duy Nguyen wrote:
> 
> > On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin
> >  wrote:
> > >
> > > On Thu, 2 Feb 2017, Duy Nguyen wrote:
> > >
> > >> You meant this one [1]? There is nothing substantial since then.
> > >>
> > >> [1] 
> > >> https://public-inbox.org/git/%3c20160601104519.16563-1-pclo...@gmail.com%3E/
> > 
> > I  could rebase and clean it up a bit if you need it, but I don't
> > think it'll end up in 'pu' or anywhere near since Junio wanted a
> > cleaner approach [1]. That means (as far as I can see) a lot more work
> > around refs store and backend area before it's ready to handle "get
> > refs from this worktree store" (or "get refs from every reachable
> > stores").
> > 
> > [1] 
> > https://public-inbox.org/git/xmqqshwwzyee@gitster.mtv.corp.google.com/

Given that
https://public-inbox.org/git/xmqqy46ntrhk@gitster.mtv.corp.google.com/
seems to have expected something to happen within a reasonable time frame,
and that 8 months is substantially longer than a reasonable time frame, I
am not sure that that position can still be defended.

Also, the more important reply was Peff's reply that suggested that the
proposed fix was incomplete, as it misses --unpack-unreachable:
https://public-inbox.org/git/20160601160143.ga9...@sigill.intra.peff.net/

Ciao,
Johannes


Business Proposal

2017-02-02 Thread QUATIF GROUP OF COMPANIES
Dear Friend,

I would like to discuss a very important issue with you. I am writing to
find out if this is your valid email. Please, let me know if this email is
valid

Kind regards
Adrien Saif
Attorney to Quatif Group of Companies



Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Johannes Schindelin
Hi Duy,

On Thu, 2 Feb 2017, Duy Nguyen wrote:

> On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin
>  wrote:
> >
> > On Thu, 2 Feb 2017, Duy Nguyen wrote:
> >
> >> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin
> >>  wrote:
> >> >
> >> > On Thu, 2 Feb 2017, Duy Nguyen wrote:
> >> >
> >> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
> >> >>  wrote:
> >> >> >
> >> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> >> >> >
> >> >> >> This squashes two changes from Johannes and Ramsay: [...]
> >> >> >
> >> >> > Sorry, I lost track of the worktree discussions... Could you
> >> >> > remind me which patch is supposed to fix my continuous reflog
> >> >> > corruption?
> >> >>
> >> >> The corruption caused by git-gc? It's not fixed. All the changes
> >> >> in this series is shown here.
> >> >
> >> > Oh sorry, I meant to ask "and if it is not in this patch series,
> >> > would you mind pointing me at the patch series that has that fix?"
> >>
> >> You meant this one [1]? There is nothing substantial since then.
> >>
> >> [1] 
> >> https://public-inbox.org/git/%3c20160601104519.16563-1-pclo...@gmail.com%3E/
> >
> > I guess I mean that.
> >
> > Given that this results in real data loss, it is surprising that this
> > has not made it even into `pu` yet!
> 
> I  could rebase and clean it up a bit if you need it, but I don't think
> it'll end up in 'pu' or anywhere near since Junio wanted a cleaner
> approach [1]. That means (as far as I can see) a lot more work around
> refs store and backend area before it's ready to handle "get refs from
> this worktree store" (or "get refs from every reachable stores").
> 
> [1] https://public-inbox.org/git/xmqqshwwzyee@gitster.mtv.corp.google.com/

That is a big, big bummer.

We are talking about a data corrupting bug here, yes? It should be
possible to do that redesign work while having a small workaround in place
that unbreaks, say, me?

Ciao,
Johannes

Please get back to me

2017-02-02 Thread Qatif Group of Companies
Dear Friend,

I would like to discuss a very important issue with you. I am writing to
find out if this is your valid email. Please, let me know if this email is
valid

Kind regards
Adrien Saif
Attorney to Qatif Group of Companies



Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Duy Nguyen
On Thu, Feb 2, 2017 at 5:37 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Thu, 2 Feb 2017, Duy Nguyen wrote:
>
>> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin
>>  wrote:
>> >
>> > On Thu, 2 Feb 2017, Duy Nguyen wrote:
>> >
>> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
>> >>  wrote:
>> >> >
>> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
>> >> >
>> >> >> This squashes two changes from Johannes and Ramsay: [...]
>> >> >
>> >> > Sorry, I lost track of the worktree discussions... Could you remind
>> >> > me which patch is supposed to fix my continuous reflog corruption?
>> >>
>> >> The corruption caused by git-gc? It's not fixed. All the changes in
>> >> this series is shown here.
>> >
>> > Oh sorry, I meant to ask "and if it is not in this patch series, would you
>> > mind pointing me at the patch series that has that fix?"
>>
>> You meant this one [1]? There is nothing substantial since then.
>>
>> [1] 
>> https://public-inbox.org/git/%3c20160601104519.16563-1-pclo...@gmail.com%3E/
>
> I guess I mean that.
>
> Given that this results in real data loss, it is surprising that this has
> not made it even into `pu` yet!

I  could rebase and clean it up a bit if you need it, but I don't
think it'll end up in 'pu' or anywhere near since Junio wanted a
cleaner approach [1]. That means (as far as I can see) a lot more work
around refs store and backend area before it's ready to handle "get
refs from this worktree store" (or "get refs from every reachable
stores").

[1] https://public-inbox.org/git/xmqqshwwzyee@gitster.mtv.corp.google.com/

-- 
Duy


Re: [PATCH v2 7/7] completion: recognize more long-options

2017-02-02 Thread Cornelius Weig
On 02/02/2017 03:00 AM, SZEDER Gábor wrote:
>> Personally, I agree with you that
>>> Adding more long options that git commands learn along the way is
>>> always an improvement.
>> However, people may start complaining that their terminal becomes too
>> cluttered when doing a double-Tab. In my cover letter, I go to length
>> about this. My assumption was that all options that are mentioned in the
>> introduction of the command man-page should be important enough to have
>> them in the completion list.
> 
> But that doesn't mean that the ones not mentioned in the synopsis
> section are not worth completing.

Absolutely. What I meant is that at least the options from the synopsis
should be contained in the set of completable options.

>> Btw, I haven't found that non-destructive options should not be eligible
>> for completion. To avoid confusion about this in the future, I suggest
>> to also change the documentation:
>>
>> index 933bb6e..96f1c7f 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -13,7 +13,7 @@
>>  #*) git email aliases for git-send-email
>>  #*) tree paths within 'ref:path/to/file' expressions
>>  #*) file paths within current working directory and index
>> -#*) common --long-options
>> +#*) common non-destructive --long-options
> 
> I don't mind such a change, but I don't think that list was ever meant
> to be comprehensive or decisive.  It is definitely not the former, as
> it's missing several things that the completion script does support.
> OTOH, it talks about .git/remotes, which has been considered legacy
> for quite some years (though it's right, because the completion script
> still supports it).

Then let's not do that change, because for some commands destructive
long-options have been in the list of completed options for quite a
while. Given that, the above change of the documentation, might stir up
more confusion than it settles.


Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Johannes Schindelin
Hi Duy,

On Thu, 2 Feb 2017, Duy Nguyen wrote:

> On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin
>  wrote:
> >
> > On Thu, 2 Feb 2017, Duy Nguyen wrote:
> >
> >> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
> >>  wrote:
> >> >
> >> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> >> >
> >> >> This squashes two changes from Johannes and Ramsay: [...]
> >> >
> >> > Sorry, I lost track of the worktree discussions... Could you remind
> >> > me which patch is supposed to fix my continuous reflog corruption?
> >>
> >> The corruption caused by git-gc? It's not fixed. All the changes in
> >> this series is shown here.
> >
> > Oh sorry, I meant to ask "and if it is not in this patch series, would you
> > mind pointing me at the patch series that has that fix?"
> 
> You meant this one [1]? There is nothing substantial since then.
> 
> [1] 
> https://public-inbox.org/git/%3c20160601104519.16563-1-pclo...@gmail.com%3E/

I guess I mean that.

Given that this results in real data loss, it is surprising that this has
not made it even into `pu` yet!

Would you mind rebasing and re-submitting?

Thanks,
Johannes

Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Duy Nguyen
On Thu, Feb 2, 2017 at 4:43 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Thu, 2 Feb 2017, Duy Nguyen wrote:
>
>> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
>>  wrote:
>> > Hi Duy,
>> >
>> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
>> >
>> >> This squashes two changes from Johannes and Ramsay: [...]
>> >
>> > Sorry, I lost track of the worktree discussions... Could you remind me
>> > which patch is supposed to fix my continuous reflog corruption?
>>
>> The corruption caused by git-gc? It's not fixed. All the changes in
>> this series is shown here.
>
> Oh sorry, I meant to ask "and if it is not in this patch series, would you
> mind pointing me at the patch series that has that fix?"

You meant this one [1]? There is nothing substantial since then.

[1] https://public-inbox.org/git/%3c20160601104519.16563-1-pclo...@gmail.com%3E/

-- 
Duy


Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Johannes Schindelin
Hi Duy,

On Thu, 2 Feb 2017, Duy Nguyen wrote:

> On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
>  wrote:
> > Hi Duy,
> >
> > On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
> >
> >> This squashes two changes from Johannes and Ramsay: [...]
> >
> > Sorry, I lost track of the worktree discussions... Could you remind me
> > which patch is supposed to fix my continuous reflog corruption?
> 
> The corruption caused by git-gc? It's not fixed. All the changes in
> this series is shown here.

Oh sorry, I meant to ask "and if it is not in this patch series, would you
mind pointing me at the patch series that has that fix?"

Thanks,
Johannes

Re: [PATCH 4/7] completion: teach ls-remote to complete options

2017-02-02 Thread Cornelius Weig


On 02/02/2017 02:40 AM, SZEDER Gábor wrote:
> 
>> ls-remote needs to complete remote names and its own options.
> 
> And refnames, too.

Yes, right. However, do you think it is reasonable to complete remote
refnames? I don't think so, because it would mean we would have to run
ls-remote during completion -- and waiting for ls-remote could be quite
lengthy.

>> In
>> addition to the existing remote name completions, do also complete
>> the options --heads, --tags, --refs, and --get-url.
> 
> Why only these four options and not the other four?
> 
> There are eight options in total here, so there is really no chance
> for cluttered terminals, and all eight are mentioned in the synopsis.

My line of thought is the following:

--quiet: does not print anything and is therefore only useful for
scripting. Thus, there is no need to have it on the command line completion.

--exit-code: has no visible effect and is only useful for scripting.

--upload-pack: is really exotic. Nobody will ever use it without digging
deep in the manuals. Therefore, I think it's unnecessary to have the
option completable.


--symref: Should probably be added, thanks.

However, if you don't find my reasoning for omitting the three options
above conclusive, I have no problem including them.


Fwd: [PATCH 1/2] doc: add doc for git-push --recurse-submodules=only

2017-02-02 Thread Brandon Williams
Looks good to me!  Thanks for writing the documentation.  I really
need to be better about getting documentation done at the same time
I'm adding features :)

-Brandon

On Wed, Feb 1, 2017 at 3:16 PM, Junio C Hamano  wrote:
>
> cornelius.w...@tngtech.com writes:
>
> > From: Cornelius Weig 
> >
> > Add documentation for the `--recurse-submodules=only` option of
> > git-push. The feature was added in commit 225e8bf (add option to
> > push only submodules).
> >
> > Signed-off-by: Cornelius Weig 
> > ---
> >
> > Notes:
> > This feature is already in 'next' but was undocumented. Unless somebody 
> > reads
> > the release notes, there is no way of knowing about it.
>
> Good eyes; the topic bw/push-submodule-only is already in 'master'.
>
> Looks good to me; Brandon?
>
> >
> >  Documentation/git-push.txt | 13 +++--
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> > index 8eefabd..1624a35 100644
> > --- a/Documentation/git-push.txt
> > +++ b/Documentation/git-push.txt
> > @@ -272,7 +272,7 @@ origin +master` to force a push to the `master` 
> > branch). See the
> >   standard error stream is not directed to a terminal.
> >
> >  --no-recurse-submodules::
> > ---recurse-submodules=check|on-demand|no::
> > +--recurse-submodules=check|on-demand|only|no::
> >   May be used to make sure all submodule commits used by the
> >   revisions to be pushed are available on a remote-tracking branch.
> >   If 'check' is used Git will verify that all submodule commits that
> > @@ -280,11 +280,12 @@ origin +master` to force a push to the `master` 
> > branch). See the
> >   remote of the submodule. If any commits are missing the push will
> >   be aborted and exit with non-zero status. If 'on-demand' is used
> >   all submodules that changed in the revisions to be pushed will be
> > - pushed. If on-demand was not able to push all necessary revisions
> > - it will also be aborted and exit with non-zero status. A value of
> > - 'no' or using `--no-recurse-submodules` can be used to override the
> > - push.recurseSubmodules configuration variable when no submodule
> > - recursion is required.
> > + pushed. If on-demand was not able to push all necessary revisions it 
> > will
> > + also be aborted and exit with non-zero status. If 'only' is used all
> > + submodules will be recursively pushed while the superproject is left
> > + unpushed. A value of 'no' or using `--no-recurse-submodules` can be 
> > used
> > + to override the push.recurseSubmodules configuration variable when no
> > + submodule recursion is required.
> >
> >  --[no-]verify::
> >   Toggle the pre-push hook (see linkgit:githooks[5]).  The


Re: init --separate-git-dir does not set core.worktree

2017-02-02 Thread Duy Nguyen
On Thu, Feb 2, 2017 at 10:55 AM, Kyle Meyer  wrote:
> Hello,
>
> As of 6311cfaf9 (init: do not set unnecessary core.worktree,
> 2016-09-25), "git init --separate-git-dir" no longer sets core.worktree
> (test below).  Based on the commit message and the corresponding thread
> [1], I don't think this change in behavior was intentional, but I wasn't
> able to understand things well enough to attempt a patch.

I'm missing some context. Why does --separate-git-dir have to set
core.worktree? What fails for you exactly?

>
> Thanks.
>
> [1] 
> https://public-inbox.org/git/calqjkkzo_y0dncrjjooyz7eso7ybmghvz6fe92oo4su7jec...@mail.gmail.com/T/#u
>
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index b8fc588b1..444e75865 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -309,6 +309,7 @@ test_expect_success 'init with separate gitdir' '
> git init --separate-git-dir realgitdir newdir &&
> echo "gitdir: $(pwd)/realgitdir" >expected &&
> test_cmp expected newdir/.git &&
> +   check_config realgitdir false "$(pwd)/newdir" &&
> test_path_is_dir realgitdir/refs
>  '
>



-- 
Duy


Re: [PATCH 2/2] completion: add completion for --recurse-submodules=only

2017-02-02 Thread Stefan Beller
On Wed, Feb 1, 2017 at 3:07 PM,   wrote:
> From: Cornelius Weig 
>
> Command completion for 'git-push --recurse-submodules' already knows to
> complete some modes. However, the recently added mode 'only' is missing.
>
> Adding 'only' to the recognized modes completes the list of non-trivial
> modes.
>
> Signed-off-by: Cornelius Weig 
> ---

Looks good,

Thanks,
Stefan

>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index ff7072a..fe3b0f8 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1675,7 +1675,7 @@ _git_pull ()
> __git_complete_remote_or_refspec
>  }
>
> -__git_push_recurse_submodules="check on-demand"
> +__git_push_recurse_submodules="check on-demand only"
>
>  __git_complete_force_with_lease ()
>  {
> --
> 2.10.2
>


Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Duy Nguyen
On Thu, Feb 2, 2017 at 4:16 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:
>
>> This squashes two changes from Johannes and Ramsay: [...]
>
> Sorry, I lost track of the worktree discussions... Could you remind me
> which patch is supposed to fix my continuous reflog corruption?

The corruption caused by git-gc? It's not fixed. All the changes in
this series is shown here.
-- 
Duy


Re: git-daemon shallow checkout fail

2017-02-02 Thread Duy Nguyen
On Tue, Jan 31, 2017 at 12:27 AM, Jeff King  wrote:
> On Sat, Jan 28, 2017 at 05:29:32PM -0700, Bob Proulx wrote:
>
>> However the problem driving me crazy is that this only fails this way
>> on one machine.  Unfortunately failing on the machine I need to use.
>> If I try this same setup on any other machine I try then there is no
>> failure and it works okay.  Therefore I conclude that in the failing
>> case it is trying to write a shallow_XX file in the repository but
>> in all of the passing cases it does not.  I browsed through the
>> git-daemon source but couldn't deduce the flow yet.
>>
>> Does anyone know why one system would try to create shallow_XX
>> files in the repository while another one would not?
>
> It depends on the git version on the server. The interesting code is in
> upload-pack.c, which is spawned by git-daemon to serve a fetch or clone
> request.
>
> See commit b790e0f67 (upload-pack: send shallow info over stdin to
> pack-objects, 2014-03-11), which lays out the history. Since that commit
> (in git v2.0.0), there should be no tmpfile needed.

It must be it. There's nowhere else that upload-pack can create
shallow_XX. (receive-pack and fetch-pack can).

>> Of course git-daemon running as nobody can't create a temporary file
>> shallow_XX in the /srv/git/test-project.git because it has no
>> permissions by design.  But why does this work on other systems and
>> not work on my target system?
>>
>>   git --version  # from today's git clone build
>>   git version 2.11.0.485.g4e59582
>
> This shouldn't be happening with git v2.11. Are you sure that the "git
> daemon" invocation is running that same version? I notice you set up a
> restricted PATH. Is it possible that /usr/local/bin or /usr/bin has an
> older version of git?

One easy way to verify is clone or fetch again with GIT_TRACE_PACKET=1
since we send the server's version as a capability since 1.8.0
-- 
Duy


Re: [PATCH 2/7] completion: add subcommand completion for rerere

2017-02-02 Thread Cornelius Weig


On 02/02/2017 01:57 AM, SZEDER Gábor wrote:
> You didn't add 'rerere' to the list of porcelain commands, i.e. it
> won't be listed after 'git '.  I'm not saying it should be
> listed there, because I can't decide whether 'rerere' is considered
> porcelain or plumbing...  Just wanted to make sure that this omission
> was intentional.

Yes this is intentional. There is a number of plumbing commands that
have command completion, but are not listed in 'git ' (e.g.
ls-tree, ls-files, ls-remote, ...). Given that rerere will not be
frequently invoked, I would not add it to the porcelain commands.


Re: [PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Johannes Schindelin
Hi Duy,

On Thu, 2 Feb 2017, Nguyễn Thái Ngọc Duy wrote:

> This squashes two changes from Johannes and Ramsay: [...]

Sorry, I lost track of the worktree discussions... Could you remind me
which patch is supposed to fix my continuous reflog corruption?

Thanks,
Dscho

Re: [PATCH] color_parse_mem: allow empty color spec

2017-02-02 Thread Duy Nguyen
On Wed, Feb 01, 2017 at 01:21:29AM +0100, Jeff King wrote:
> On Tue, Jan 31, 2017 at 02:45:40PM -0800, Junio C Hamano wrote:
> 
> > * nd/log-graph-configurable-colors (2017-01-23) 3 commits
> >   (merged to 'next' on 2017-01-23 at c369982ad8)
> >  + log --graph: customize the graph lines with config log.graphColors
> >  + color.c: trim leading spaces in color_parse_mem()
> >  + color.c: fix color_parse_mem() with value_len == 0
> > 
> >  Some people feel the default set of colors used by "git log --graph"
> >  rather limiting.  A mechanism to customize the set of colors has
> >  been introduced.
> > 
> >  Reported to break "add -p".
> >  cf. <20170128040709.tqx4u45ktgpkb...@sigill.intra.peff.net>
> 
> I hadn't heard anything back,

Sorry I was accidentally busy during Luna new year holiday.

> so I wrapped up my fix with a commit
> message and tests (it needs to go on top anyway, since the breakage is
> in 'next').
> 
> -- >8 --
> Subject: [PATCH] color_parse_mem: allow empty color spec
> 
> Prior to c2f41bf52 (color.c: fix color_parse_mem() with
> value_len == 0, 2017-01-19), the empty string was
> interpreted as a color "reset". This was an accidental
> outcome, and that commit turned it into an error.
> 
> However, scripts may pass the empty string as a default
> value to "git config --get-color" to disable color when the
> value is not defined. The git-add--interactive script does
> this. As a result, the script is unusable since c2f41bf52
> unless you have color.diff.plain defined (if it is defined,
> then we don't parse the empty default at all).

..

> --- a/color.c
> +++ b/color.c
> @@ -212,8 +212,10 @@ int color_parse_mem(const char *value, int value_len, 
> char *dst)
>   len--;
>   }
>  
> - if (!len)
> - return -1;
> + if (!len) {
> + dst[0] = '\0';
> + return 0;
> + }
>  
>   if (!strncasecmp(ptr, "reset", len)) {
>   xsnprintf(dst, end - dst, GIT_COLOR_RESET);

I wonder if it makes more sense to do this in builtin/config.c. The
"default value" business is strictly git-config command's. The parsing
function does not need to know. Maybe something like this?

diff --git a/builtin/config.c b/builtin/config.c
index 05843a0f96..ec1f4f0cf4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -322,8 +322,10 @@ static void get_color(const char *var, const char 
*def_color)
git_config_with_options(git_get_color_config, NULL,
&given_config_source, respect_includes);
 
-   if (!get_color_found && def_color) {
-   if (color_parse(def_color, parsed_color) < 0)
+   if (!get_color_found) {
+   if (!def_color)
+   strcpy(parsed_color, GIT_COLOR_RESET);
+   else if (color_parse(def_color, parsed_color) < 0)
die(_("unable to parse default color value"));
}
 
This is also a good opportunity to document this behavior in
git-config.txt, I think.
--
Duy


Google Doc about the Contributors' Summit

2017-02-02 Thread Johannes Schindelin
Hi team,

I just started typing stuff up in a Google Doc, and made it editable to
everyone, feel free to help and add things:

https://docs.google.com/document/d/1KDoSn4btbK5VJCVld32he29U0pUeFGhpFxyx9ZJDDB0/edit?usp=sharing

Ciao,
Johannes



[PATCH 09/11] worktree move: accept destination as directory

2017-02-02 Thread Nguyễn Thái Ngọc Duy
Similar to "mv a b/", which is actually "mv a b/a", we extract basename
of source worktree and create a directory of the same name at
destination if dst path is a directory.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 builtin/worktree.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 0d8b57ceb3..900b68bb5d 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -541,7 +541,13 @@ static int move_worktree(int ac, const char **av, const 
char *prefix)
strbuf_addstr(&dst, prefix_filename(prefix,
strlen(prefix),
av[1]));
-   if (file_exists(dst.buf))
+   if (is_directory(dst.buf))
+   /*
+* keep going, dst will be appended after we get the
+* source's absolute path
+*/
+   ;
+   else if (file_exists(dst.buf))
die(_("target '%s' already exists"), av[1]);
 
worktrees = get_worktrees(0);
@@ -559,6 +565,17 @@ static int move_worktree(int ac, const char **av, const 
char *prefix)
if (validate_worktree(wt, 0))
return -1;
 
+   if (is_directory(dst.buf)) {
+   const char *sep = find_last_dir_sep(wt->path);
+
+   if (!sep)
+   die(_("could not figure out destination name from 
'%s'"),
+   wt->path);
+   strbuf_addstr(&dst, sep);
+   if (file_exists(dst.buf))
+   die(_("target '%s' already exists"), dst.buf);
+   }
+
if (rename(wt->path, dst.buf) == -1)
die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
 
-- 
2.11.0.157.gd943d85



[PATCH 11/11] worktree remove: new command

2017-02-02 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-worktree.txt | 21 +
 builtin/worktree.c | 79 ++
 contrib/completion/git-completion.bash |  5 ++-
 t/t2028-worktree-move.sh   | 26 +++
 4 files changed, 121 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 13105138a7..bbde6b8110 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -14,6 +14,7 @@ SYNOPSIS
 'git worktree lock' [--reason ] 
 'git worktree move'  
 'git worktree prune' [-n] [-v] [--expire ]
+'git worktree remove' [--force] 
 'git worktree unlock' 
 
 DESCRIPTION
@@ -81,6 +82,13 @@ prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
 
+remove::
+
+Remove a working tree. Only clean working trees (no untracked files
+and no modification in tracked files) can be removed. Unclean working
+trees can be removed with `--force`. The main working tree cannot be
+removed.
+
 unlock::
 
 Unlock a working tree, allowing it to be pruned, moved or deleted.
@@ -90,9 +98,10 @@ OPTIONS
 
 -f::
 --force::
-   By default, `add` refuses to create a new working tree when ``
-   is already checked out by another working tree. This option overrides
-   that safeguard.
+   By default, `add` refuses to create a new working tree when
+   `` is already checked out by another working tree and
+   `remove` refuses to remove an unclean working tree. This option
+   overrides that safeguard.
 
 -b ::
 -B ::
@@ -253,12 +262,6 @@ Multiple checkout in general is still experimental, and 
the support
 for submodules is incomplete. It is NOT recommended to make multiple
 checkouts of a superproject.
 
-git-worktree could provide more automation for tasks currently
-performed manually, such as:
-
-- `remove` to remove a linked working tree and its administrative files (and
-  warn if the working tree is dirty)
-
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6c58d620ce..a1c91f1542 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -17,6 +17,7 @@ static const char * const worktree_usage[] = {
N_("git worktree lock [] "),
N_("git worktree move  "),
N_("git worktree prune []"),
+   N_("git worktree remove [] "),
N_("git worktree unlock "),
NULL
 };
@@ -605,6 +606,82 @@ static int move_worktree(int ac, const char **av, const 
char *prefix)
return update_worktree_location(wt, dst.buf);
 }
 
+static int remove_worktree(int ac, const char **av, const char *prefix)
+{
+   int force = 0;
+   struct option options[] = {
+   OPT_BOOL(0, "force", &force,
+N_("force removing even if the worktree is dirty")),
+   OPT_END()
+   };
+   struct worktree **worktrees, *wt;
+   struct strbuf sb = STRBUF_INIT;
+   const char *reason;
+   int ret = 0;
+
+   ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+   if (ac != 1)
+   usage_with_options(worktree_usage, options);
+
+   worktrees = get_worktrees(0);
+   wt = find_worktree(worktrees, prefix, av[0]);
+   if (!wt)
+   die(_("'%s' is not a working directory"), av[0]);
+   if (is_main_worktree(wt))
+   die(_("'%s' is a main working directory"), av[0]);
+   reason = is_worktree_locked(wt);
+   if (reason) {
+   if (*reason)
+   die(_("already locked, reason: %s"), reason);
+   die(_("already locked, no reason"));
+   }
+   if (validate_worktree(wt, 0))
+   return -1;
+
+   if (!force) {
+   struct argv_array child_env = ARGV_ARRAY_INIT;
+   struct child_process cp;
+   char buf[1];
+
+   argv_array_pushf(&child_env, "%s=%s/.git",
+GIT_DIR_ENVIRONMENT, wt->path);
+   argv_array_pushf(&child_env, "%s=%s",
+GIT_WORK_TREE_ENVIRONMENT, wt->path);
+   memset(&cp, 0, sizeof(cp));
+   argv_array_pushl(&cp.args, "status", "--porcelain", NULL);
+   cp.env = child_env.argv;
+   cp.git_cmd = 1;
+   cp.dir = wt->path;
+   cp.out = -1;
+   ret = start_command(&cp);
+   if (ret)
+   die_errno(_("failed to run git-status on '%s', code 
%d"),
+ av[0], ret);
+   ret = xread(cp.out, buf, sizeof(buf));
+   if (ret)
+   die(_("'%s' is dirty, use --force to delete it"), 
av[0]);
+   close(cp.out);
+   ret = finish_command(&cp);
+   if (ret)
+   die_errno(_("failed to run git-status on '%s', code 
%d"),
+  

[PATCH 10/11] worktree move: refuse to move worktrees with submodules

2017-02-02 Thread Nguyễn Thái Ngọc Duy
Submodules contains .git files with relative paths. After a worktree
move, these files need to be updated or they may point to nowhere.

This is a bandage patch to make sure "worktree move" don't break
people's worktrees by accident. When .git file update code is in
place, this validate_no_submodules() could be removed.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/worktree.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 900b68bb5d..6c58d620ce 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -525,6 +525,27 @@ static int unlock_worktree(int ac, const char **av, const 
char *prefix)
return ret;
 }
 
+static void validate_no_submodules(const struct worktree *wt)
+{
+   struct index_state istate = { NULL };
+   int i, found_submodules = 0;
+
+   if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
+   for (i = 0; i < istate.cache_nr; i++) {
+   struct cache_entry *ce = istate.cache[i];
+
+   if (S_ISGITLINK(ce->ce_mode)) {
+   found_submodules = 1;
+   break;
+   }
+   }
+   }
+   discard_index(&istate);
+
+   if (found_submodules)
+   die(_("This working tree contains submodules and cannot be 
moved yet"));
+}
+
 static int move_worktree(int ac, const char **av, const char *prefix)
 {
struct option options[] = {
@@ -565,6 +586,8 @@ static int move_worktree(int ac, const char **av, const 
char *prefix)
if (validate_worktree(wt, 0))
return -1;
 
+   validate_no_submodules(wt);
+
if (is_directory(dst.buf)) {
const char *sep = find_last_dir_sep(wt->path);
 
-- 
2.11.0.157.gd943d85



[PATCH 08/11] worktree move: new command

2017-02-02 Thread Nguyễn Thái Ngọc Duy
There are two options to move the main worktree, but both have
complications, so it's not implemented yet. Anyway the options are:

 - convert the main worktree to a linked one and move it away, leave the
   git repository where it is. The repo essentially becomes bare after
   this move.

 - move the repository with the main worktree. The tricky part is make
   sure all file descriptors to the repository are closed, or it may
   fail on Windows.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-worktree.txt |  7 +-
 builtin/worktree.c | 43 ++
 contrib/completion/git-completion.bash |  2 +-
 t/t2028-worktree-move.sh   | 31 
 4 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e257c19ebe..13105138a7 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -12,6 +12,7 @@ SYNOPSIS
 'git worktree add' [-f] [--detach] [--checkout] [-b ]  
[]
 'git worktree list' [--porcelain]
 'git worktree lock' [--reason ] 
+'git worktree move'  
 'git worktree prune' [-n] [-v] [--expire ]
 'git worktree unlock' 
 
@@ -71,6 +72,11 @@ files from being pruned automatically. This also prevents it 
from
 being moved or deleted. Optionally, specify a reason for the lock
 with `--reason`.
 
+move::
+
+Move a working tree to a new location. Note that the main working tree
+cannot be moved yet.
+
 prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
@@ -252,7 +258,6 @@ performed manually, such as:
 
 - `remove` to remove a linked working tree and its administrative files (and
   warn if the working tree is dirty)
-- `mv` to move or rename a working tree and update its administrative files
 
 GIT
 ---
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 9a97e37a3f..0d8b57ceb3 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -15,6 +15,7 @@ static const char * const worktree_usage[] = {
N_("git worktree add []  []"),
N_("git worktree list []"),
N_("git worktree lock [] "),
+   N_("git worktree move  "),
N_("git worktree prune []"),
N_("git worktree unlock "),
NULL
@@ -524,6 +525,46 @@ static int unlock_worktree(int ac, const char **av, const 
char *prefix)
return ret;
 }
 
+static int move_worktree(int ac, const char **av, const char *prefix)
+{
+   struct option options[] = {
+   OPT_END()
+   };
+   struct worktree **worktrees, *wt;
+   struct strbuf dst = STRBUF_INIT;
+   const char *reason;
+
+   ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+   if (ac != 2)
+   usage_with_options(worktree_usage, options);
+
+   strbuf_addstr(&dst, prefix_filename(prefix,
+   strlen(prefix),
+   av[1]));
+   if (file_exists(dst.buf))
+   die(_("target '%s' already exists"), av[1]);
+
+   worktrees = get_worktrees(0);
+   wt = find_worktree(worktrees, prefix, av[0]);
+   if (!wt)
+   die(_("'%s' is not a working directory"), av[0]);
+   if (is_main_worktree(wt))
+   die(_("'%s' is a main working directory"), av[0]);
+   reason = is_worktree_locked(wt);
+   if (reason) {
+   if (*reason)
+   die(_("already locked, reason: %s"), reason);
+   die(_("already locked, no reason"));
+   }
+   if (validate_worktree(wt, 0))
+   return -1;
+
+   if (rename(wt->path, dst.buf) == -1)
+   die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
+
+   return update_worktree_location(wt, dst.buf);
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
struct option options[] = {
@@ -546,5 +587,7 @@ int cmd_worktree(int ac, const char **av, const char 
*prefix)
return lock_worktree(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "unlock"))
return unlock_worktree(ac - 1, av + 1, prefix);
+   if (!strcmp(av[1], "move"))
+   return move_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
 }
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 21016bf8df..613e03b879 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2715,7 +2715,7 @@ _git_whatchanged ()
 
 _git_worktree ()
 {
-   local subcommands="add list lock prune unlock"
+   local subcommands="add list lock move prune unlock"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 8298aaf97f..bef420a086 100755
--- a/t/t2028-worktree-move.s

[PATCH 07/11] worktree.c: add update_worktree_location()

2017-02-02 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 worktree.c | 21 +
 worktree.h |  6 ++
 2 files changed, 27 insertions(+)

diff --git a/worktree.c b/worktree.c
index 929072ad89..7684951da5 100644
--- a/worktree.c
+++ b/worktree.c
@@ -354,6 +354,27 @@ int validate_worktree(const struct worktree *wt, int quiet)
return 0;
 }
 
+int update_worktree_location(struct worktree *wt, const char *path_)
+{
+   struct strbuf path = STRBUF_INIT;
+   int ret = 0;
+
+   if (is_main_worktree(wt))
+   return 0;
+
+   strbuf_add_absolute_path(&path, path_);
+   if (fspathcmp(wt->path, path.buf)) {
+   write_file(git_common_path("worktrees/%s/gitdir",
+  wt->id),
+  "%s/.git", real_path(path.buf));
+   free(wt->path);
+   wt->path = strbuf_detach(&path, NULL);
+   ret = 0;
+   }
+   strbuf_release(&path);
+   return ret;
+}
+
 int is_worktree_being_rebased(const struct worktree *wt,
  const char *target)
 {
diff --git a/worktree.h b/worktree.h
index 4433db2eb3..1ee03f4d06 100644
--- a/worktree.h
+++ b/worktree.h
@@ -58,6 +58,12 @@ extern const char *is_worktree_locked(struct worktree *wt);
 extern int validate_worktree(const struct worktree *wt, int quiet);
 
 /*
+ * Update worktrees/xxx/gitdir with the new path.
+ */
+extern int update_worktree_location(struct worktree *wt,
+   const char *path_);
+
+/*
  * Free up the memory for worktree(s)
  */
 extern void free_worktrees(struct worktree **);
-- 
2.11.0.157.gd943d85



[PATCH 06/11] worktree.c: add validate_worktree()

2017-02-02 Thread Nguyễn Thái Ngọc Duy
This function is later used by "worktree move" and "worktree remove"
to ensure that we have a good connection between the repository and
the worktree. For example, if a worktree is moved manually, the
worktree location recorded in $GIT_DIR/worktrees/.../gitdir is
incorrect and we should not move that one.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 worktree.c | 63 ++
 worktree.h |  5 +
 2 files changed, 68 insertions(+)

diff --git a/worktree.c b/worktree.c
index eb6121263b..929072ad89 100644
--- a/worktree.c
+++ b/worktree.c
@@ -291,6 +291,69 @@ const char *is_worktree_locked(struct worktree *wt)
return wt->lock_reason;
 }
 
+static int report(int quiet, const char *fmt, ...)
+{
+   va_list params;
+
+   if (quiet)
+   return -1;
+
+   va_start(params, fmt);
+   vfprintf(stderr, fmt, params);
+   fputc('\n', stderr);
+   va_end(params);
+   return -1;
+}
+
+int validate_worktree(const struct worktree *wt, int quiet)
+{
+   struct strbuf sb = STRBUF_INIT;
+   const char *path;
+   int err;
+
+   if (is_main_worktree(wt)) {
+   /*
+* Main worktree using .git file to point to the
+* repository would make it impossible to know where
+* the actual worktree is if this function is executed
+* from another worktree. No .git file support for now.
+*/
+   strbuf_addf(&sb, "%s/.git", wt->path);
+   if (!is_directory(sb.buf)) {
+   strbuf_release(&sb);
+   return report(quiet, _("'%s/.git' at main worktree is 
not the repository directory"),
+ wt->path);
+   }
+   return 0;
+   }
+
+   /*
+* Make sure "gitdir" file points to a real .git file and that
+* file points back here.
+*/
+   if (!is_absolute_path(wt->path))
+   return report(quiet, _("'%s' file does not contain absolute 
path to the worktree location"),
+ git_common_path("worktrees/%s/gitdir", wt->id));
+
+   strbuf_addf(&sb, "%s/.git", wt->path);
+   if (!file_exists(sb.buf)) {
+   strbuf_release(&sb);
+   return report(quiet, _("'%s/.git' does not exist"), wt->path);
+   }
+
+   path = read_gitfile_gently(sb.buf, &err);
+   strbuf_release(&sb);
+   if (!path)
+   return report(quiet, _("'%s/.git' is not a .git file, error 
code %d"),
+ wt->path, err);
+
+   if (fspathcmp(path, real_path(git_common_path("worktrees/%s", wt->id
+   return report(quiet, _("'%s' does not point back to"),
+ wt->path, git_common_path("worktrees/%s", 
wt->id));
+
+   return 0;
+}
+
 int is_worktree_being_rebased(const struct worktree *wt,
  const char *target)
 {
diff --git a/worktree.h b/worktree.h
index d59ce1fee8..4433db2eb3 100644
--- a/worktree.h
+++ b/worktree.h
@@ -53,6 +53,11 @@ extern int is_main_worktree(const struct worktree *wt);
 extern const char *is_worktree_locked(struct worktree *wt);
 
 /*
+ * Return zero if the worktree is in good condition.
+ */
+extern int validate_worktree(const struct worktree *wt, int quiet);
+
+/*
  * Free up the memory for worktree(s)
  */
 extern void free_worktrees(struct worktree **);
-- 
2.11.0.157.gd943d85



[PATCH 04/11] worktree.c: get_worktrees() takes a new flag argument

2017-02-02 Thread Nguyễn Thái Ngọc Duy
This is another no-op patch, in preparation for get_worktrees() to do
optional things, like sorting.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 branch.c   | 2 +-
 builtin/branch.c   | 2 +-
 builtin/worktree.c | 6 +++---
 worktree.c | 4 ++--
 worktree.h | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/branch.c b/branch.c
index 0d459b3cfe..c431cbf6a9 100644
--- a/branch.c
+++ b/branch.c
@@ -348,7 +348,7 @@ void die_if_checked_out(const char *branch, int 
ignore_current_worktree)
 int replace_each_worktree_head_symref(const char *oldref, const char *newref)
 {
int ret = 0;
-   struct worktree **worktrees = get_worktrees();
+   struct worktree **worktrees = get_worktrees(0);
int i;
 
for (i = 0; worktrees[i]; i++) {
diff --git a/builtin/branch.c b/builtin/branch.c
index 60cc5c8e8d..475707528a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -531,7 +531,7 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
 
 static void reject_rebase_or_bisect_branch(const char *target)
 {
-   struct worktree **worktrees = get_worktrees();
+   struct worktree **worktrees = get_worktrees(0);
int i;
 
for (i = 0; worktrees[i]; i++) {
diff --git a/builtin/worktree.c b/builtin/worktree.c
index b835b91f63..d7d195cd95 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix)
if (ac)
usage_with_options(worktree_usage, options);
else {
-   struct worktree **worktrees = get_worktrees();
+   struct worktree **worktrees = get_worktrees(0);
int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
 
if (!porcelain)
@@ -478,7 +478,7 @@ static int lock_worktree(int ac, const char **av, const 
char *prefix)
if (ac != 1)
usage_with_options(worktree_usage, options);
 
-   worktrees = get_worktrees();
+   worktrees = get_worktrees(0);
wt = find_worktree(worktrees, prefix, av[0]);
if (!wt)
die(_("'%s' is not a working tree"), av[0]);
@@ -511,7 +511,7 @@ static int unlock_worktree(int ac, const char **av, const 
char *prefix)
if (ac != 1)
usage_with_options(worktree_usage, options);
 
-   worktrees = get_worktrees();
+   worktrees = get_worktrees(0);
wt = find_worktree(worktrees, prefix, av[0]);
if (!wt)
die(_("'%s' is not a working tree"), av[0]);
diff --git a/worktree.c b/worktree.c
index 3145522536..ead088e43c 100644
--- a/worktree.c
+++ b/worktree.c
@@ -160,7 +160,7 @@ static void mark_current_worktree(struct worktree 
**worktrees)
free(git_dir);
 }
 
-struct worktree **get_worktrees(void)
+struct worktree **get_worktrees(unsigned flags)
 {
struct worktree **list = NULL;
struct strbuf path = STRBUF_INIT;
@@ -327,7 +327,7 @@ const struct worktree *find_shared_symref(const char 
*symref,
 
if (worktrees)
free_worktrees(worktrees);
-   worktrees = get_worktrees();
+   worktrees = get_worktrees(0);
 
for (i = 0; worktrees[i]; i++) {
struct worktree *wt = worktrees[i];
diff --git a/worktree.h b/worktree.h
index 90e1311fa7..2e68d4ad86 100644
--- a/worktree.h
+++ b/worktree.h
@@ -23,7 +23,7 @@ struct worktree {
  * The caller is responsible for freeing the memory from the returned
  * worktree(s).
  */
-extern struct worktree **get_worktrees(void);
+extern struct worktree **get_worktrees(unsigned flags);
 
 /*
  * Return git dir of the worktree. Note that the path may be relative.
-- 
2.11.0.157.gd943d85



[PATCH 02/11] worktree: reorder an if statement

2017-02-02 Thread Nguyễn Thái Ngọc Duy
This is no-op. But it helps reduce diff noise in the next patch.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 builtin/worktree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 5c4854d3e4..8a654e4ad3 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -406,10 +406,10 @@ static void show_worktree(struct worktree *wt, int 
path_maxlen, int abbrev_len)
else {
strbuf_addf(&sb, "%-*s ", abbrev_len,
find_unique_abbrev(wt->head_sha1, 
DEFAULT_ABBREV));
-   if (!wt->is_detached)
-   strbuf_addf(&sb, "[%s]", 
shorten_unambiguous_ref(wt->head_ref, 0));
-   else
+   if (wt->is_detached)
strbuf_addstr(&sb, "(detached HEAD)");
+   else
+   strbuf_addf(&sb, "[%s]", 
shorten_unambiguous_ref(wt->head_ref, 0));
}
printf("%s\n", sb.buf);
 
-- 
2.11.0.157.gd943d85



[PATCH 05/11] worktree list: keep the list sorted

2017-02-02 Thread Nguyễn Thái Ngọc Duy
It makes it easier to write tests for. But it should also be good for
the user since locating a worktree by eye would be easier once they
notice this.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 builtin/worktree.c   |  2 +-
 t/t2027-worktree-list.sh | 19 +++
 worktree.c   | 14 ++
 worktree.h   |  2 ++
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index d7d195cd95..9a97e37a3f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix)
if (ac)
usage_with_options(worktree_usage, options);
else {
-   struct worktree **worktrees = get_worktrees(0);
+   struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED);
int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
 
if (!porcelain)
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 98b5f340e5..465eeeacd3 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -117,4 +117,23 @@ test_expect_success 'broken main worktree still at the 
top' '
)
 '
 
+test_expect_success 'linked worktrees are sorted' '
+   mkdir sorted &&
+   git init sorted/main &&
+   (
+   cd sorted/main &&
+   test_tick &&
+   test_commit new &&
+   git worktree add ../first &&
+   git worktree add ../second &&
+   git worktree list --porcelain | grep ^worktree >actual
+   ) &&
+   cat >expected <<-EOF &&
+   worktree $(pwd)/sorted/main
+   worktree $(pwd)/sorted/first
+   worktree $(pwd)/sorted/second
+   EOF
+   test_cmp expected sorted/main/actual
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index ead088e43c..eb6121263b 100644
--- a/worktree.c
+++ b/worktree.c
@@ -160,6 +160,13 @@ static void mark_current_worktree(struct worktree 
**worktrees)
free(git_dir);
 }
 
+static int compare_worktree(const void *a_, const void *b_)
+{
+   const struct worktree *const *a = a_;
+   const struct worktree *const *b = b_;
+   return fspathcmp((*a)->path, (*b)->path);
+}
+
 struct worktree **get_worktrees(unsigned flags)
 {
struct worktree **list = NULL;
@@ -191,6 +198,13 @@ struct worktree **get_worktrees(unsigned flags)
ALLOC_GROW(list, counter + 1, alloc);
list[counter] = NULL;
 
+   if (flags & GWT_SORT_LINKED)
+   /*
+* don't sort the first item (main worktree), which will
+* always be the first
+*/
+   QSORT(list + 1, counter - 1, compare_worktree);
+
mark_current_worktree(list);
return list;
 }
diff --git a/worktree.h b/worktree.h
index 2e68d4ad86..d59ce1fee8 100644
--- a/worktree.h
+++ b/worktree.h
@@ -15,6 +15,8 @@ struct worktree {
 
 /* Functions for acting on the information about worktrees. */
 
+#define GWT_SORT_LINKED (1 << 0) /* keeps linked worktrees sorted */
+
 /*
  * Get the worktrees.  The primary worktree will always be the first returned,
  * and linked worktrees will be pointed to by 'next' in each subsequent
-- 
2.11.0.157.gd943d85



[PATCH 03/11] get_worktrees() must return main worktree as first item even on error

2017-02-02 Thread Nguyễn Thái Ngọc Duy
This is required by git-worktree.txt, stating that the main worktree is
the first line (especially in --porcelain mode when we can't just change
behavior at will).

There's only one case when get_worktrees() may skip main worktree, when
parse_ref() fails. Update the code so that we keep first item as main
worktree and return something sensible in this case:

 - In user-friendly mode, since we're not constraint by anything,
   returning "(error)" should do the job (we already show "(detached
   HEAD)" which is not machine-friendly). Actually errors should be
   printed on stderr by parse_ref() (*)

 - In plumbing mode, we do not show neither 'bare', 'detached' or
   'branch ...', which is possible by the format description if I read
   it right.

Careful readers may realize that when the local variable "head_ref" in
get_main_worktree() is emptied, add_head_info() will do nothing to
wt->head_sha1. But that's ok because head_sha1 is zero-ized in the
previous patch.

(*) Well, it does not. But it's supposed to be a stop gap implementation
until we can reuse refs code to parse "ref: " stuff in HEAD, from
resolve_refs_unsafe(). Now may be the time since refs refactoring is
mostly done.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 builtin/worktree.c   |  6 --
 t/t2027-worktree-list.sh | 21 +
 worktree.c   | 10 +++---
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 8a654e4ad3..b835b91f63 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -388,7 +388,7 @@ static void show_worktree_porcelain(struct worktree *wt)
printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
if (wt->is_detached)
printf("detached\n");
-   else
+   else if (wt->head_ref)
printf("branch %s\n", wt->head_ref);
}
printf("\n");
@@ -408,8 +408,10 @@ static void show_worktree(struct worktree *wt, int 
path_maxlen, int abbrev_len)
find_unique_abbrev(wt->head_sha1, 
DEFAULT_ABBREV));
if (wt->is_detached)
strbuf_addstr(&sb, "(detached HEAD)");
-   else
+   else if (wt->head_ref)
strbuf_addf(&sb, "[%s]", 
shorten_unambiguous_ref(wt->head_ref, 0));
+   else
+   strbuf_addstr(&sb, "(error)");
}
printf("%s\n", sb.buf);
 
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 1b1b65a6b0..98b5f340e5 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -96,4 +96,25 @@ test_expect_success 'bare repo cleanup' '
rm -rf bare1
 '
 
+test_expect_success 'broken main worktree still at the top' '
+   git init broken-main &&
+   (
+   cd broken-main &&
+   test_commit new &&
+   git worktree add linked &&
+   cat >expected <<-EOF &&
+   worktree $(pwd)
+   HEAD $_z40
+
+   EOF
+   cd linked &&
+   echo "worktree $(pwd)" >expected &&
+   echo "ref: .broken" >../.git/HEAD &&
+   git worktree list --porcelain | head -n 3 >actual &&
+   test_cmp ../expected actual &&
+   git worktree list | head -n 1 >actual.2 &&
+   grep -F "(error)" actual.2
+   )
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index f7c1b5e24d..3145522536 100644
--- a/worktree.c
+++ b/worktree.c
@@ -88,16 +88,13 @@ static struct worktree *get_main_worktree(void)
 
strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
 
-   if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
-   goto done;
-
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
worktree->is_bare = is_bare;
worktree->is_detached = is_detached;
-   add_head_info(&head_ref, worktree);
+   if (!parse_ref(path.buf, &head_ref, &is_detached))
+   add_head_info(&head_ref, worktree);
 
-done:
strbuf_release(&path);
strbuf_release(&worktree_path);
strbuf_release(&head_ref);
@@ -173,8 +170,7 @@ struct worktree **get_worktrees(void)
 
list = xmalloc(alloc * sizeof(struct worktree *));
 
-   if ((list[counter] = get_main_worktree()))
-   counter++;
+   list[counter++] = get_main_worktree();
 
strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
dir = opendir(path.buf);
-- 
2.11.0.157.gd943d85



[PATCH 01/11] worktree.c: zero new 'struct worktree' on allocation

2017-02-02 Thread Nguyễn Thái Ngọc Duy
This keeps things a bit simpler when we add more fields, knowing that
default values are always zero.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 worktree.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/worktree.c b/worktree.c
index f7869f8d60..f7c1b5e24d 100644
--- a/worktree.c
+++ b/worktree.c
@@ -91,16 +91,11 @@ static struct worktree *get_main_worktree(void)
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
 
-   worktree = xmalloc(sizeof(struct worktree));
+   worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
-   worktree->id = NULL;
worktree->is_bare = is_bare;
-   worktree->head_ref = NULL;
worktree->is_detached = is_detached;
-   worktree->is_current = 0;
add_head_info(&head_ref, worktree);
-   worktree->lock_reason = NULL;
-   worktree->lock_reason_valid = 0;
 
 done:
strbuf_release(&path);
@@ -138,16 +133,11 @@ static struct worktree *get_linked_worktree(const char 
*id)
if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
goto done;
 
-   worktree = xmalloc(sizeof(struct worktree));
+   worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
worktree->id = xstrdup(id);
-   worktree->is_bare = 0;
-   worktree->head_ref = NULL;
worktree->is_detached = is_detached;
-   worktree->is_current = 0;
add_head_info(&head_ref, worktree);
-   worktree->lock_reason = NULL;
-   worktree->lock_reason_valid = 0;
 
 done:
strbuf_release(&path);
-- 
2.11.0.157.gd943d85



[PATCH 00/11] nd/worktree-move update

2017-02-02 Thread Nguyễn Thái Ngọc Duy
This squashes two changes from Johannes and Ramsay:

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 339c622e20..a1c91f1542 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -528,7 +528,7 @@ static int unlock_worktree(int ac, const char **av, const 
char *prefix)
 
 static void validate_no_submodules(const struct worktree *wt)
 {
-   struct index_state istate = {0};
+   struct index_state istate = { NULL };
int i, found_submodules = 0;
 
if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 084acc6c6d..b3105eaaed 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -71,13 +71,14 @@ test_expect_success 'move locked worktree' '
 '
 
 test_expect_success 'move worktree' '
+   toplevel="$(pwd)" &&
git worktree move source destination &&
test_path_is_missing source &&
git worktree list --porcelain | grep "^worktree" >actual &&
cat <<-EOF >expected &&
-   worktree $TRASH_DIRECTORY
-   worktree $TRASH_DIRECTORY/destination
-   worktree $TRASH_DIRECTORY/elsewhere
+   worktree $toplevel
+   worktree $toplevel/destination
+   worktree $toplevel/elsewhere
EOF
test_cmp expected actual &&
git -C destination log --format=%s >actual2 &&


Nguyễn Thái Ngọc Duy (11):
  worktree.c: zero new 'struct worktree' on allocation
  worktree: reorder an if statement
  get_worktrees() must return main worktree as first item even on error
  worktree.c: get_worktrees() takes a new flag argument
  worktree list: keep the list sorted
  worktree.c: add validate_worktree()
  worktree.c: add update_worktree_location()
  worktree move: new command
  worktree move: accept destination as directory
  worktree move: refuse to move worktrees with submodules
  worktree remove: new command

 Documentation/git-worktree.txt |  28 --
 branch.c   |   2 +-
 builtin/branch.c   |   2 +-
 builtin/worktree.c | 176 +++--
 contrib/completion/git-completion.bash |   5 +-
 t/t2027-worktree-list.sh   |  40 
 t/t2028-worktree-move.sh   |  57 +++
 worktree.c | 126 +++
 worktree.h |  15 ++-
 9 files changed, 410 insertions(+), 41 deletions(-)

-- 
2.11.0.157.gd943d85