[PATCH 1/4] completion: prioritize ./git-completion.bash

2013-12-30 Thread Ramkumar Ramachandra
To ease development, prioritize ./git-completion.bash over other
standard system paths.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z $script ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e  script=$e  break
-- 
1.8.5.2.227.g53f3478

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


[PATCH 0/4] Fix branch.autosetup(merge|rebase) completion

2013-12-30 Thread Ramkumar Ramachandra
Hi,

I figured that branch.autosetupmerge, branch.autosetuprebase and
remote.pushdefault are very tedious to type out in full, and started
looking into fixing their completions this evening. The solution turns
out to be much more complex than I initally imagined, but I'm quite
happy with the solution.

I hope it's an enjoyable read.

Ramkumar Ramachandra (4):
  completion: prioritize ./git-completion.bash
  completion: introduce __gitcomp_2 ()
  completion: fix branch.autosetup(merge|rebase)
  completion: fix remote.pushdefault

 contrib/completion/git-completion.bash | 43 --
 contrib/completion/git-completion.zsh  | 12 +-
 2 files changed, 52 insertions(+), 3 deletions(-)

-- 
1.8.5.2.227.g53f3478

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


[PATCH 2/4] completion: introduce __gitcomp_2 ()

2013-12-30 Thread Ramkumar Ramachandra
There are situations where two classes of completions possible. For
example

  branch.TAB

should try to complete

  branch.master.
  branch.autosetupmerge
  branch.autosetuprebase

The first candidate has the suffix ., and the second/ third candidates
have the suffix  . To facilitate completions of this kind, create a
variation of __gitcomp_nl () that accepts two sets of arguments and two
independent suffixes.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 30 ++
 contrib/completion/git-completion.zsh  | 10 ++
 2 files changed, 40 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..64b20b8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,36 @@ __gitcomp_nl ()
__gitcompadd $1 ${2-} ${3-$cur} ${4- }
 }
 
+# Generates completion reply from two sets of completion words, with
+# configurable suffixes for each.
+#
+# It accepts 2 to 6 arguments:
+# 1: First set of possible completion words.
+# 2: Second set of possible completion words.
+# 3: A prefix to be added to each completion word (both $1 and $2)
+#(optional).
+# 4: Generate possible completion matches for this word (optional).
+# 5: A suffix to be appended to each completion word in the first set
+#($1) instead of the default space (optional).
+# 6: A suffix to be appended to each completion word in the second set
+#($2) instead of the default space (optional).
+__gitcomp_2 ()
+{
+   local pfx=${3-} cur_=${4-$cur} sfx=${5- } sfx2=${6- } i=0
+   local IFS=$' \t\n'
+
+   for x in $1; do
+   if [[ $x == $cur_* ]]; then
+   COMPREPLY[i++]=$pfx$x$sfx
+   fi
+   done
+   for x in $2; do
+   if [[ $x == $cur_* ]]; then
+   COMPREPLY[i++]=$pfx$x$sfx2
+   fi
+   done
+}
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 6fca145..261a7f5 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,16 @@ __gitcomp_nl ()
compadd -Q -S ${4- } -p ${2-} -- ${=1}  _ret=0
 }
 
+__gitcomp_2 ()
+{
+   emulate -L zsh
+
+   local IFS=$' \t\n'
+   compset -P '*[=:]'
+   compadd -Q -S ${5- } -p ${3-} -- ${=1}  _ret=0
+   compadd -Q -S ${6- } -p ${3-} -- ${=2}  _ret=0
+}
+
 __gitcomp_file ()
 {
emulate -L zsh
-- 
1.8.5.2.227.g53f3478

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


[PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2013-12-30 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config branch.autoTAB

'autosetupmerge' and 'autosetuprebase' don't come up. This is because
$cur is matched with branch.* and a list of branches are
completed. Add 'autosetup(merge|rebase)' to the list of branches using
__gitcomp_2 ().

Also take care to not complete

  $ git config branch.autosetupmerge.TAB
  $ git config branch.autosetuprebase.TAB

with the usual branch.name. candidates.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 64b20b8..0bda757 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1851,12 +1851,18 @@ _git_config ()
;;
branch.*.*)
local pfx=${cur%.*}. cur_=${cur##*.}
+   if [ $pfx == branch.autosetupmerge. ] ||
+   [ $pfx == branch.autosetuprebase. ]; then
+   return
+   fi
__gitcomp remote pushremote merge mergeoptions rebase $pfx 
$cur_
return
;;
branch.*)
local pfx=${cur%.*}. cur_=${cur#*.}
-   __gitcomp_nl $(__git_heads) $pfx $cur_ .
+   __gitcomp_2 $(__git_heads) 
+   autosetupmerge autosetuprebase
+$pfx $cur_ .
return
;;
guitool.*.*)
-- 
1.8.5.2.227.g53f3478

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


[PATCH 4/4] completion: fix remote.pushdefault

2013-12-30 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config remote.pushTAB

'pushdefault' doesn't come up. This is because $cur is matched with
remote.* and a list of remotes are completed. Add 'pushdefault' to the
list of remotes using __gitcomp_2 ().

Also take care to not complete

  $ git config remote.pushdefault.TAB

with the usual remote.name. candidates.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 0bda757..9e0213d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1896,6 +1896,9 @@ _git_config ()
;;
remote.*.*)
local pfx=${cur%.*}. cur_=${cur##*.}
+   if [ $pfx == remote.pushdefault. ]; then
+   return
+   fi
__gitcomp 
url proxy fetch push mirror skipDefaultUpdate
receivepack uploadpack tagopt pushurl
@@ -1904,7 +1907,7 @@ _git_config ()
;;
remote.*)
local pfx=${cur%.*}. cur_=${cur#*.}
-   __gitcomp_nl $(__git_remotes) $pfx $cur_ .
+   __gitcomp_2 $(__git_remotes) pushdefault $pfx $cur_ .
return
;;
url.*.*)
-- 
1.8.5.2.227.g53f3478

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


[PATCH] for-each-ref: remove unused variable

2013-12-30 Thread Ramkumar Ramachandra
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 builtin/for-each-ref.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 6551e7b..51798b4 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -92,7 +92,7 @@ static struct {
  */
 static const char **used_atom;
 static cmp_type *used_atom_type;
-static int used_atom_cnt, sort_atom_limit, need_tagged, need_symref;
+static int used_atom_cnt, need_tagged, need_symref;
 static int need_color_reset_at_eol;
 
 /*
@@ -1105,7 +1105,6 @@ int cmd_for_each_ref(int argc, const char **argv, const 
char *prefix)
 
if (!sort)
sort = default_sort();
-   sort_atom_limit = used_atom_cnt;
 
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
-- 
1.8.5.2.227.g53f3478

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


Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-02 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 If we are looking at branch.autosetupmerge. followed by something,
 who typed that final dot?

I admit that it's a very unlikely case. The user did:

  $ branch.autosetupmerTAB

hit backspace to delete the trailing space, inserted a dot, and hit TAB again.

 If you are working on a topic about
 auto-setup-merge and named your branch autosetupmerge, don't you
 want to be able to configure various aspect of that branch via
 branch.autosetupmerge.{remote,merge} etc., just like you can do so
 for your topic branch via branch.topic.{remote,merge} etc.,
 regardless of your use of autosetupmerge option across branches?

My reasoning was that being correct was more important that being
complete. So, if by some horrible chance, the user names her branch
autosetupmerge, we don't aid her in completions.

 Besides, it smells fishy to me that you need to enumerate and
 special case these two here, and then you have to repeat them below
 in a separate case arm.

I'm not too irked about correctness in this odd case; seeing that you
aren't either, I'll resubmit the series without this hunk (+ the hunk
in remote.pushdefault).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] completion: introduce __gitcomp_2 ()

2014-01-02 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 __gitcomp_nl $(__git_heads) $pfx $cur_ .
 __gitcomp_nl_append $autosetupmerge\nautosetuprebase\n $pfx 
 $cur_  

This is not a bad idea at all. I'm just afraid that we might be
leaving open ends: What happens if the $pfx isn't the same in both
cases? Who keeps track of the index i of COMPREPLY (it's currently a
local variable)? If we make it global, doesn't every function that
deals with COMPREPLY be careful to reset it?

More importantly, can you see a usecase for more than two completion classes?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/4] completion: prioritize ./git-completion.bash

2014-01-03 Thread Ramkumar Ramachandra
To ease development, prioritize ./git-completion.bash over other
standard system paths.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z $script ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e  script=$e  break
-- 
1.8.5.2.227.g53f3478

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


[PATCH v2 2/4] completion: introduce __gitcomp_2 ()

2014-01-03 Thread Ramkumar Ramachandra
There are situations where two classes of completions possible. For
example

  branch.TAB

should try to complete

  branch.master.
  branch.autosetupmerge
  branch.autosetuprebase

The first candidate has the suffix ., and the second/ third candidates
have the suffix  . To facilitate completions of this kind, create a
variation of __gitcomp_nl () that accepts two sets of arguments and two
independent suffixes.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 30 ++
 contrib/completion/git-completion.zsh  | 10 ++
 2 files changed, 40 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..64b20b8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,36 @@ __gitcomp_nl ()
__gitcompadd $1 ${2-} ${3-$cur} ${4- }
 }
 
+# Generates completion reply from two sets of completion words, with
+# configurable suffixes for each.
+#
+# It accepts 2 to 6 arguments:
+# 1: First set of possible completion words.
+# 2: Second set of possible completion words.
+# 3: A prefix to be added to each completion word (both $1 and $2)
+#(optional).
+# 4: Generate possible completion matches for this word (optional).
+# 5: A suffix to be appended to each completion word in the first set
+#($1) instead of the default space (optional).
+# 6: A suffix to be appended to each completion word in the second set
+#($2) instead of the default space (optional).
+__gitcomp_2 ()
+{
+   local pfx=${3-} cur_=${4-$cur} sfx=${5- } sfx2=${6- } i=0
+   local IFS=$' \t\n'
+
+   for x in $1; do
+   if [[ $x == $cur_* ]]; then
+   COMPREPLY[i++]=$pfx$x$sfx
+   fi
+   done
+   for x in $2; do
+   if [[ $x == $cur_* ]]; then
+   COMPREPLY[i++]=$pfx$x$sfx2
+   fi
+   done
+}
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 6fca145..261a7f5 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,16 @@ __gitcomp_nl ()
compadd -Q -S ${4- } -p ${2-} -- ${=1}  _ret=0
 }
 
+__gitcomp_2 ()
+{
+   emulate -L zsh
+
+   local IFS=$' \t\n'
+   compset -P '*[=:]'
+   compadd -Q -S ${5- } -p ${3-} -- ${=1}  _ret=0
+   compadd -Q -S ${6- } -p ${3-} -- ${=2}  _ret=0
+}
+
 __gitcomp_file ()
 {
emulate -L zsh
-- 
1.8.5.2.227.g53f3478

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


[PATCH v2 0/4] Fix branch.autosetup(merge|rebase) completion

2014-01-03 Thread Ramkumar Ramachandra
Hi,

In this iteration, I've removed hunks to prevent completing:

  $ git config remote.pushdefault.TAB
  $ git config branch.autosetupmerge.TAB
  $ git config branch.autosetuprebase.TAB

Since they're perfectly valid remote/ branch names.

Thanks.

Ramkumar Ramachandra (4):
  completion: prioritize ./git-completion.bash
  completion: introduce __gitcomp_2 ()
  completion: fix branch.autosetup(merge|rebase)
  completion: fix remote.pushdefault

 contrib/completion/git-completion.bash | 36 --
 contrib/completion/git-completion.zsh  | 12 +++-
 2 files changed, 45 insertions(+), 3 deletions(-)

-- 
1.8.5.2.227.g53f3478

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


Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-03 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 You seem to be calling it incorrect to give the same degree of
 completion for a branch the user named autosetupmerge as another
 branch topic, but I think it is incorrect not to, so I cannot tell
 if we are agreeing or disagreeing.

No, what's incorrect is providing completions for

  $ git config branch.autosetupmerge.TAB

when no branch called autosetupmerge exists. The purpose of the hunk
(which I now removed) was to prevent such completions, but it has the
side-effect of also preventing a legitimate completion in the case
when the user really has a branch named autosetupmerge.

What is your take on the issue?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/4] Fix branch.autosetup(merge|rebase) completion

2014-01-03 Thread Ramkumar Ramachandra
Hi Junio et al,

In v3, I've implemented __gitcomp_nl_append (), like you
suggested. After implementing it, I feel foolish about having gone
around my head to do __gitcomp_2 ().

Thanks.

Ramkumar Ramachandra (4):
  completion: prioritize ./git-completion.bash
  completion: introduce __gitcomp_nl_append ()
  completion: fix branch.autosetup(merge|rebase)
  completion: fix remote.pushdefault

 contrib/completion/git-completion.bash | 15 +++
 contrib/completion/git-completion.zsh  | 10 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

-- 
1.8.5.2.227.g53f3478

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


[PATCH v3 1/4] completion: prioritize ./git-completion.bash

2014-01-03 Thread Ramkumar Ramachandra
To ease development, prioritize ./git-completion.bash over other
standard system paths.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z $script ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e  script=$e  break
-- 
1.8.5.2.227.g53f3478

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


[PATCH v3 2/4] completion: introduce __gitcomp_nl_append ()

2014-01-03 Thread Ramkumar Ramachandra
There are situations where multiple classes of completions possible. For
example

  branch.TAB

should try to complete

  branch.master.
  branch.autosetupmerge
  branch.autosetuprebase

The first candidate has the suffix ., and the second/ third candidates
have the suffix  . To facilitate completions of this kind, create a
variation of __gitcomp_nl () that appends to the existing list of
completion candidates, COMPREPLY.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 13 +
 contrib/completion/git-completion.zsh  |  8 
 2 files changed, 21 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..bf358d6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,19 @@ __gitcomp_nl ()
__gitcompadd $1 ${2-} ${3-$cur} ${4- }
 }
 
+# Variation of __gitcomp_nl () that appends to the existing list of
+# completion candidates, COMPREPLY.
+__gitcomp_nl_append ()
+{
+   local IFS=$'\n'
+   local i=${#COMPREPLY[@]}
+   for x in $1; do
+   if [[ $x == $3* ]]; then
+   COMPREPLY[i++]=$2$x$4
+   fi
+   done
+}
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 6fca145..6b77968 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,14 @@ __gitcomp_nl ()
compadd -Q -S ${4- } -p ${2-} -- ${=1}  _ret=0
 }
 
+__gitcomp_nl_append ()
+{
+   emulate -L zsh
+
+   local IFS=$'\n'
+   compadd -Q -S ${4- } -p ${2-} -- ${=1}  _ret=0
+}
+
 __gitcomp_file ()
 {
emulate -L zsh
-- 
1.8.5.2.227.g53f3478

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


[PATCH v3 4/4] completion: fix remote.pushdefault

2014-01-03 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config remote.pushTAB

'pushdefault' doesn't come up. This is because $cur is matched with
remote.* and a list of remotes are completed. Add 'pushdefault' to the
list of remotes using __gitcomp_nl_append ().

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 75c7302..345ceff 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1883,6 +1883,7 @@ _git_config ()
remote.*)
local pfx=${cur%.*}. cur_=${cur#*.}
__gitcomp_nl $(__git_remotes) $pfx $cur_ .
+   __gitcomp_nl_append pushdefault $pfx $cur_
return
;;
url.*.*)
-- 
1.8.5.2.227.g53f3478

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


[PATCH v3 3/4] completion: fix branch.autosetup(merge|rebase)

2014-01-03 Thread Ramkumar Ramachandra
When attempting to complete

  $ git config branch.autoTAB

'autosetupmerge' and 'autosetuprebase' don't come up. This is because
$cur is matched with branch.* and a list of branches are
completed. Add 'autosetupmerge', 'autosetuprebase' to the list of
branches using __gitcomp_nl_append ().

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index bf358d6..75c7302 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1840,6 +1840,7 @@ _git_config ()
branch.*)
local pfx=${cur%.*}. cur_=${cur#*.}
__gitcomp_nl $(__git_heads) $pfx $cur_ .
+   __gitcomp_nl_append $'autosetupmerge\nautosetuprebase\n' $pfx 
$cur_
return
;;
guitool.*.*)
-- 
1.8.5.2.227.g53f3478

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


Re: [PATCH v3 2/4] completion: introduce __gitcomp_nl_append ()

2014-01-05 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 Is it because going this route and doing it at such a low level
 would make zsh completion (which I have no clue about ;-)
 unnecessarily complex?

The zsh completion only cares to override __gitcomp_nl () and
__gitcomp_nl_append (), without bothering to re-implement the
lower-level functions; so it's no problem at all. I wrote mine out by
thinking of a non-intrusive direct translation, while your version
focuses on eliminating duplication. I don't have a strong preference
either way, so I will resubmit with your version.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 1/4] zsh completion: find matching custom bash completion

2014-01-05 Thread Ramkumar Ramachandra
If zsh completion is being read from a location that is different from
system-wide default, it is likely that the user is trying to use a
custom version, perhaps closer to the bleeding edge, installed in her
own directory. We will more likely to find the matching bash completion
script in the same directory than in those system default places.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z $script ]; then
local -a locations
local e
locations=(
+   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e  script=$e  break
-- 
1.8.5.2.227.g53f3478

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


[PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
A very common workflow for preparing patches involves working off a
topic branch and generating patches against 'master' to send off to the
maintainer. However, a plain

  $ git format-patch -o outgoing

is a no-op on a topic branch, and the user has to remember to specify
'master' explicitly everytime. Save the user the extra keystrokes by
introducing format.defaultTo which can contain the name of a branch
against which to base patches.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Documentation/config.txt   |  4 
 builtin/log.c  |  7 +++
 contrib/completion/git-completion.bash |  1 +
 t/t4014-format-patch.sh| 10 ++
 4 files changed, 22 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..b90abd1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1135,6 +1135,10 @@ format.coverLetter::
format-patch is invoked, but in addition can be set to auto, to
generate a cover-letter only when there's more than one patch.
 
+format.defaultTo::
+   The name of a branch against which to generate patches by
+   default. You'd usually want this to be 'master'.
+
 filter.driver.clean::
The command which is used to convert the content of a worktree
file to a blob upon checkin.  See linkgit:gitattributes[5] for
diff --git a/builtin/log.c b/builtin/log.c
index b97373d..ebc419e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -674,6 +674,7 @@ static int thread;
 static int do_signoff;
 static const char *signature = git_version_string;
 static int config_cover_letter;
+static const char *config_defaultto;
 
 enum {
COVER_UNSET,
@@ -750,6 +751,8 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON : 
COVER_OFF;
return 0;
}
+   if (!strcmp(var, format.defaultto))
+   return git_config_string(config_defaultto, var, value);
 
return git_log_config(var, value, cb);
 }
@@ -1324,6 +1327,10 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
die (_(--subject-prefix and -k are mutually exclusive.));
rev.preserve_subject = keep_subject;
 
+   if (argc  2  config_defaultto) {
+   argv[1] = config_defaultto;
+   argc++;
+   }
argc = setup_revisions(argc, argv, rev, s_r_opt);
if (argc  1)
die (_(unrecognized argument: %s), argv[1]);
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 39b81f7..75699d4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1992,6 +1992,7 @@ _git_config ()
format.attach
format.cc
format.coverLetter
+   format.defaultTo
format.headers
format.numbered
format.pretty
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 73194b2..46c0337 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1370,4 +1370,14 @@ test_expect_success 'cover letter auto user override' '
test_line_count = 2 list
 '
 
+test_expect_success 'defaultTo side' '
+   mkdir -p tmp 
+   test_when_finished rm -rf tmp;
+   git config --unset format.defaultTo 
+
+   git config format.defaultTo side 
+   git format-patch -o tmp list 
+   test_line_count = 3 list
+'
+
 test_done
-- 
1.8.5.2.229.g4448466.dirty

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


[PATCH 1/2] completion: complete format.coverLetter

2014-01-06 Thread Ramkumar Ramachandra
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 51c2dd4..39b81f7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1991,6 +1991,7 @@ _git_config ()
fetch.unpackLimit
format.attach
format.cc
+   format.coverLetter
format.headers
format.numbered
format.pretty
-- 
1.8.5.2.229.g4448466.dirty

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


Re: [PATCH 0/2] Minor convinience feature: format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 Ramkumar Ramachandra (2):
   completion: complete format.coverLetter
   format-patch: introduce format.defaultTo

Any thoughts on checkout.defaultTo? I have a com alias to checkout 'master'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[BUG] rebase --onto doesn't abort properly

2014-01-06 Thread Ramkumar Ramachandra
Hi,

On the latest git, I noticed that a rebase --onto doesn't abort
properly. Steps to reproduce:

  # on some topic branch
  $ git rebase --onto master @~10
  ^C # quickly!
  $ git rebase --abort
  # HEAD is still detached

I tried going back a few revisions, and the bug seems to be very old;
I'm surprised I didn't notice it earlier.

Are others able to reproduce this?

Thanks.

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


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
  - why is a single branch name sufficient?

It does accept a revision, so any form is allowed; but why would
anyone want that in a format.defaultTo? I'm not sure we want to impose
an artificial restriction on the configuration variable though.

  - is it a better option to simply default to @{u}, if one exists,
instead of failing?

I'm not sure @{u} is a good default. Personally, my workflow involves
publishing my fork before sending out patches; mainly so that I can
compare with @{u} when I do re-spins. People can put @{u} in
format.defaultTo if it suits their workflow though.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] rebase --onto doesn't abort properly

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 I do not think --abort was designed to abort an uncontrolled stop
 like ^C in the first place.

Why not? All it requires is a reset --hard to
.git/rebase-apply/head-name, as usual, no?

 To allow that kind of recovery, you
 need to teach rebase to first record the state you would want to
 go back to before doing anything, but then what happens if the ^C
 comes when you are still in the middle of doing so?

I'm a bit puzzled: doesn't rebase write_basic_state() as soon as it
starts? It's true that we can't recover from a ^C before that, but I
would expect to be able to recover after a ^C somewhere in the middle.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Jonathan Nieder wrote:
  1. Most config settings are in noun form: e.g.,
 [remote] pushDefault = foo.  That makes their names easy to guess
 and makes them easy to talk about: I set the default remote for
 pushing by changing the remote.pushdefault setting.

 '[url foo] insteadOf' is an exception to that and a bit of an
 aberration.

 This new '[format] defaultTo' repeats the same end-with-a-preposition
 mistake, while I think it would be better to learn from it.

I agree that it's somewhat unconventional to allow people to put a
revision as a configuration variable value, but I think it's useful.
url.url.insteadOf is incredibly useful, for instance.

  2. Wouldn't a more natural default be @{u}..HEAD instead of relying on
 the user to do the make-work of keeping a local branch that tracks
 master up to date?

Like I said in my message to Junio, @{u} is not necessarily the best
default for all workflows, although you can fill that into
format.defaultTo.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 I meant a single branch as opposed to depending on what branch
 you are sending out, you may have to use a different upstream
 starting point, and a single format.defaultTo that does not read
 what your HEAD currently points at may not be enough.

 Unless you set @{u} to this new configuration, in which case the
 choice becomes dynamic depending on the current branch, but

  - if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?

  - Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?

Ah, I was going for an equivalent of merge.defaultToUpstream, but I
guess branch.*.forkedFrom is a good way to go.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Jeff King wrote:
 Yeah, I had similar thoughts. I personally use branch.*.merge as
 forkedFrom, and it seems like we are going that way anyway with things
 like git rebase and git merge defaulting to upstream.

My issue with that is that I no idea where my branch is with respect
to my forked upstream; I find that extremely useful when doing
re-spins.

 But then there
 is git push -u and push.default = upstream, which treats the
 upstream config as something else entirely.

push.default = upstream is a bit of a disaster, in my opinion. I've
advocated push.default = current on multiple occasions, and wrote the
initial remote.pushDefault series with that configuration in mind.

 I wonder if it is too late to try to clarify this dual usage. It kind of
 seems like the push config is this is the place I publish to. Which,
 in many workflows, just so happens to be the exact same as the place you
 forked from. Could we introduce a new branch.*.pushupstream variable
 that falls back to branch.*.merge? Or is that just throwing more fuel on
 the fire (more sand in the pit in my analogy, I guess).

We already have a branch.*.pushremote, and I don't see the value of
branch.*.pushbranch (what you're referring to as pushupstream, I
assume) except for Gerrit users. Frankly, I don't use full triangular
workflows myself mainly because my prompt is compromised: when I have
a branch.*.remote different from branch.*.pushremote, I'd like to see
where my branch is with respect to @{u} and @{publish} (not yet
invented); that's probably too much information to digest anyway, so I
use central workflow (pointing to my fork) for each of my branches,
except master (which points to Junio's repo).

 I admit I haven't thought it through yet, though. And even if it does
 work, it may throw a slight monkey wrench in the proposed push.default
 transition.

We're transitioning to push.default = simple which is even simpler than current.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
Junio C Hamano wrote:.
 As I said in the different subthread, I am not convinced that you
 would need the complexity of branch.*.forkedFrom.  If you set your
 upstream to the true upstream (not your publishing point), and
 have remote.pushdefaultset to 'publish', you can expect

 git push

 to do the right thing, and then always say

 git show-branch publish/topic topic

I think it's highly sub-optimal to have a local-branch @{u} for
several reasons; the prompt is almost useless in this case, and it
will always show your forked-branch ahead of 'master' (assuming that
'master' doesn't update itself in the duration of your development).
While doing respins, the prompt doesn't aid you in any way. Besides,
on several occasions, I found myself working on the same forked-branch
from two different machines; then the publish-point isn't necessarily
always a publish-point: it's just another upstream for the branch.
The point of a special branch.*.forkedFrom is that you can always show
the master..@ information in the prompt ignoring divergences; after
all, a divergence simply means that you need to rebase onto the latest
'master' ('master' is never going to get a non-ff update anyway).

So, instead of crippling the functionality around the publish-point,
let's build minimal required functionality around branch.*.forkedFrom.
I'd love a prompt like:

  branch-forkedfrom5*:~/src/git$

Clearly, branch-forkedfrom has diverged from my publish-point (I'm
probably doing a respin), and has 5 commits (it's 5 commits ahead of
'master' ignoring divergences).

 to see where your last published branch is relative to what you
 have, no?  Mapping topic@{publish} to refs/remotes/publish/topic
 (or when you have 'topic' checked out, mapping @{publish} to it)
 is a trivial but is a separate usefulness, I would guess.

I think a @{publish} is needed for when branch.*.remote is different
from remote.pushDefault. Like I said earlier, it's probably too much
information to give to the user: divergences with respect to two
remotes. So, we'll hold it off until someone finds a usecase for it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-06 Thread Ramkumar Ramachandra
John Szakmeister wrote:
 Then where does it get pushed?  Do you always specify where to save your work?

 FWIW, I think the idea of treating @{u} as the eventual recipient of
 your changes is good, but then it seems like Git is lacking the
 publish my changes to this other branch concept.

 Am I missing something?  If there is something other than @{u} to
 represent this latter concept, I think `git push` should default to
 that instead.  But, at least with my current knowledge, that doesn't
 exist--without explicitly saying so--or treating @{u} as that branch.
 If there's a better way to do this, I'd love to hear it!

That's why we invented remote.pushdefault and branch.*.pushremote. When you say

  $ git push

it automatically goes to the right remote instead of going to the
place you fetched from. You can read up on how push.default interacts
with this setting too, although I always recommend push.default =
current.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] completion: complete format.coverLetter

2014-01-07 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
  contrib/completion/git-completion.bash | 1 +
  1 file changed, 1 insertion(+)

Junio: Please push this part via 'maint' independently.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-07 Thread Ramkumar Ramachandra
John Szakmeister wrote:
 I guess it's not a good idea to
 set 'push.default' to 'upstream' in this triangle workflow then,
 otherwise the branch name being pushed to will be 'branch.*.merge'.
 Is that correct?  I just want to make sure I understand what's
 happening here.

push.default = upstream does not support triangular workflows. See
builtin/push.c:setup_push_upstream().

 Given this new found knowledge, I'm not sure it makes sense for `git
 status` to show me the status against the upstream vs. the publish
 location.  The latter makes a little more sense to me, though I see
 the usefulness of either one.

Currently, status information is only against @{u}; we haven't
invented a @{publish} yet.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC/PATCH] format-patch: introduce branch.*.forkedFrom

2014-01-07 Thread Ramkumar Ramachandra
A very common workflow for preparing patches involves working off a
topic branch and generating patches against 'master' to send off to the
maintainer. However, a plain

  $ git format-patch -o outgoing

is a no-op on a topic branch, and the user has to remember to specify
'master' explicitly everytime. This problem is not unique to
format-patch; even a

  $ git rebase -i

is a no-op because the branch to rebase against isn't specified.

To tackle this problem, introduce branch.*.forkedFrom which can specify
the parent branch of a topic branch. Future patches will build
functionality around this new configuration variable.

Cc: Jeff King p...@peff.net
Cc: Junio C Hamano gis...@pobox.com
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Since -M, -C, -D are left in the argc, checking argc  2 isn't
 sufficient.

 I wanted to get an early reaction before wiring up checkout and
 rebase.

 But I wanted to discuss the overall idea of the patch.
 builtin/log.c   | 21 +
 t/t4014-format-patch.sh | 20 
 2 files changed, 41 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index b97373d..525e696 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -674,6 +674,7 @@ static int thread;
 static int do_signoff;
 static const char *signature = git_version_string;
 static int config_cover_letter;
+static const char *config_base_branch;
 
 enum {
COVER_UNSET,
@@ -750,6 +751,22 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON : 
COVER_OFF;
return 0;
}
+   if (starts_with(var, branch.)) {
+   const char *name = var + 7;
+   const char *subkey = strrchr(name, '.');
+   struct strbuf buf = STRBUF_INIT;
+
+   if (!subkey)
+   return 0;
+   strbuf_add(buf, name, subkey - name);
+   if (branch_get(buf.buf) != branch_get(NULL))
+   return 0;
+   strbuf_release(buf);
+   if (!strcmp(subkey, .forkedfrom)) {
+   if (git_config_string(config_base_branch, var, value))
+   return -1;
+   }
+   }
 
return git_log_config(var, value, cb);
 }
@@ -1324,6 +1341,10 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
die (_(--subject-prefix and -k are mutually exclusive.));
rev.preserve_subject = keep_subject;
 
+   if (argc  2  config_base_branch) {
+   argv[1] = config_base_branch;
+   argc++;
+   }
argc = setup_revisions(argc, argv, rev, s_r_opt);
if (argc  1)
die (_(unrecognized argument: %s), argv[1]);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 73194b2..2ea94af 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1370,4 +1370,24 @@ test_expect_success 'cover letter auto user override' '
test_line_count = 2 list
 '
 
+test_expect_success 'branch.*.forkedFrom matches' '
+   mkdir -p tmp 
+   test_when_finished rm -rf tmp;
+   git config --unset branch.rebuild-1.forkedFrom 
+
+   git config branch.rebuild-1.forkedFrom master 
+   git format-patch -o tmp list 
+   test_line_count = 2 list
+'
+
+test_expect_success 'branch.*.forkedFrom does not match' '
+   mkdir -p tmp 
+   test_when_finished rm -rf tmp;
+   git config --unset branch.foo.forkedFrom 
+
+   git config branch.foo.forkedFrom master 
+   git format-patch -o tmp list 
+   test_line_count = 0 list
+'
+
 test_done
-- 
1.8.5.2.234.gba2dde8.dirty

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


Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom

2014-01-07 Thread Ramkumar Ramachandra
[Fixed typo in Junio's address]

On Wed, Jan 8, 2014 at 1:59 AM, Ramkumar Ramachandra artag...@gmail.com wrote:
 A very common workflow for preparing patches involves working off a
 topic branch and generating patches against 'master' to send off to the
 maintainer. However, a plain

   $ git format-patch -o outgoing

 is a no-op on a topic branch, and the user has to remember to specify
 'master' explicitly everytime. This problem is not unique to
 format-patch; even a

   $ git rebase -i

 is a no-op because the branch to rebase against isn't specified.

 To tackle this problem, introduce branch.*.forkedFrom which can specify
 the parent branch of a topic branch. Future patches will build
 functionality around this new configuration variable.

 Cc: Jeff King p...@peff.net
 Cc: Junio C Hamano gis...@pobox.com
 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  Since -M, -C, -D are left in the argc, checking argc  2 isn't
  sufficient.

  I wanted to get an early reaction before wiring up checkout and
  rebase.

  But I wanted to discuss the overall idea of the patch.
  builtin/log.c   | 21 +
  t/t4014-format-patch.sh | 20 
  2 files changed, 41 insertions(+)

 diff --git a/builtin/log.c b/builtin/log.c
 index b97373d..525e696 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -674,6 +674,7 @@ static int thread;
  static int do_signoff;
  static const char *signature = git_version_string;
  static int config_cover_letter;
 +static const char *config_base_branch;

  enum {
 COVER_UNSET,
 @@ -750,6 +751,22 @@ static int git_format_config(const char *var, const char 
 *value, void *cb)
 config_cover_letter = git_config_bool(var, value) ? COVER_ON 
 : COVER_OFF;
 return 0;
 }
 +   if (starts_with(var, branch.)) {
 +   const char *name = var + 7;
 +   const char *subkey = strrchr(name, '.');
 +   struct strbuf buf = STRBUF_INIT;
 +
 +   if (!subkey)
 +   return 0;
 +   strbuf_add(buf, name, subkey - name);
 +   if (branch_get(buf.buf) != branch_get(NULL))
 +   return 0;
 +   strbuf_release(buf);
 +   if (!strcmp(subkey, .forkedfrom)) {
 +   if (git_config_string(config_base_branch, var, 
 value))
 +   return -1;
 +   }
 +   }

 return git_log_config(var, value, cb);
  }
 @@ -1324,6 +1341,10 @@ int cmd_format_patch(int argc, const char **argv, 
 const char *prefix)
 die (_(--subject-prefix and -k are mutually exclusive.));
 rev.preserve_subject = keep_subject;

 +   if (argc  2  config_base_branch) {
 +   argv[1] = config_base_branch;
 +   argc++;
 +   }
 argc = setup_revisions(argc, argv, rev, s_r_opt);
 if (argc  1)
 die (_(unrecognized argument: %s), argv[1]);
 diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
 index 73194b2..2ea94af 100755
 --- a/t/t4014-format-patch.sh
 +++ b/t/t4014-format-patch.sh
 @@ -1370,4 +1370,24 @@ test_expect_success 'cover letter auto user override' '
 test_line_count = 2 list
  '

 +test_expect_success 'branch.*.forkedFrom matches' '
 +   mkdir -p tmp 
 +   test_when_finished rm -rf tmp;
 +   git config --unset branch.rebuild-1.forkedFrom 
 +
 +   git config branch.rebuild-1.forkedFrom master 
 +   git format-patch -o tmp list 
 +   test_line_count = 2 list
 +'
 +
 +test_expect_success 'branch.*.forkedFrom does not match' '
 +   mkdir -p tmp 
 +   test_when_finished rm -rf tmp;
 +   git config --unset branch.foo.forkedFrom 
 +
 +   git config branch.foo.forkedFrom master 
 +   git format-patch -o tmp list 
 +   test_line_count = 0 list
 +'
 +
  test_done
 --
 1.8.5.2.234.gba2dde8.dirty

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


Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom

2014-01-07 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 I do not mind allowing laziness by defaulting to something, but I am
 not enthused by an approach that adds the new variable whose value
 is questionable.  The description does not justify at all why
 @{upstream} is not a good default (unless the workflow is screwed up
 and @{upstream} is set to point at somewhere that is _not_ a true
 upstream, that is).

Did you find the explanation I gave in
http://article.gmane.org/gmane.comp.version-control.git/240077
reasonable? I don't know why label the respin-workflow as being
screwed up.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom

2014-01-07 Thread Ramkumar Ramachandra
Jeff King wrote:
 I have not carefully read some of the later bits of the discussion from
 last night / this morning, so maybe I am missing something, but this
 seems backwards to me from what Junio and I were discussing earlier.

 The point was that the meaning of @{upstream} (and branch.*.merge)
 is _already_ forked-from, and push -u and push.default=upstream
 are the odd men out. If we are going to add an option to distinguish the
 two branch relationships:

   1. Where you forked from

   2. Where you push to

 we should leave @{upstream} as (1), and add a new option to represent
 (2). Not the other way around.

I have a local branch 'forkedfrom' that has two sources: 'master'
and 'ram/forkedfrom'. 'ram/forkedfrom' isn't a dumb publish-point:
the relationship information I get between 'forkedfrom' and
'ram/forkedfrom' is very useful; it's what helps me tell how my
re-roll is doing with respect to the original series; I'd often want
to cherry-pick commits/ messages from my original series to prepare
the re-roll, so interaction with this source is quite high. On the
other hand, the relationship information I get between 'forkedfrom'
and 'master' is practically useless: 'forkedfrom' is always ahead of
'master', and a divergence indicates that I need to rebase; I'll never
really need to interact with this source.

I'm only thinking in terms of what infrastructure we've already built:
if @{u} is set to 'ram/forkedfrom', we get a lot of information for
free _now_. If @{u} is set to 'master', the current git-status is
unhelpful.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] format-patch: introduce format.defaultTo

2014-01-07 Thread Ramkumar Ramachandra
Jeff King wrote:
 My daily procedure is something like:

   all_topics |
   while read topic; do echo $topic $(git rev-parse $topic@{u}); done |
   topo_sort |
   while read topic upstream; do
 git rebase $upstream $topic || exit 1
   done

Ah, I was perhaps over-specializing for my own usecase, where
everything is based off 'master'. I never considered 'master' a true
upstream because I throw away topic branches after the maintainer
merges them. If you have long-running branches that you work on a
daily basis, the issue is somewhat different.

 While doing respins, the prompt doesn't aid you in any way. Besides,
 on several occasions, I found myself working on the same forked-branch
 from two different machines; then the publish-point isn't necessarily
 always a publish-point: it's just another upstream for the branch.

 Right, things get trickier then. But I don't think there is an automatic
 way around that. Sometimes the published one is more up to date, and
 sometimes the upstream thing is more up to date.  You have to manually
 tell git which you are currently basing your work on. I find in such a
 situation that it tends to resolve itself quickly, though, as the first
 step is to pull in the changes you pushed up from the other machine
 anyway (either via git reset or git rebase).

My primary concern is that the proposed @{publish} should be a
first-class citizen; if it has everything that @{u} has, then we're
both good: you'd primarily use @{u}, while I'd primarily use
@{publish}.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom

2014-01-07 Thread Ramkumar Ramachandra
Jeff King wrote:
 I definitely respect the desire to reuse the existing tooling we have
 for @{u}. At the same time, I think you are warping the meaning of
 @{u} somewhat. It is _not_ your upstream here, but rather another
 version of the branch that has useful changes in it. That might be
 splitting hairs a bit, but I think you will find that the differences
 leak through in inconvenient spots (like format-patch, where you really
 _do_ want to default to the true upstream).

Thanks for the clear reasoning.

 If we add @{publish} (and @{pu}), then it becomes very convenient to
 refer to the ram/ version of your branch. That seems like an obvious
 first step to me. We don't have to add new config, because
 branch.*.pushremote already handles this.

Agreed. I'll start working on @{publish}. It's going to take quite a
bit of effort, because I won't actually start using it until my prompt
is @{publish}-aware.

 Now you can do git rebase @{pu} which is nice, but not _quite_ as nice
 as git rebase, which defaults to @{u}. That first step might be
 enough, and I'd hold off there and try it out for a few days or weeks
 first. But if you find in your workflow that you are having to specify
 @{pu} a lot, then maybe it is worth adding an option to default rebase
 to @{pu} instead of @{u}.

Actually, I'm not sure I'd use git rebase @{pu}; for me @{pu} is
mainly a source of information for taking apart to build a new series.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/5] interpret_branch_name: factor out upstream handling

2014-01-08 Thread Ramkumar Ramachandra
Jeff King wrote:
  sha1_name.c | 83 
 ++---
  1 file changed, 52 insertions(+), 31 deletions(-)

Thanks. I applied this to my series as-it-is.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 0/5] branch@{publish} shorthand

2014-01-08 Thread Ramkumar Ramachandra
Jeff King wrote:
 There's a fair bit of refactoring involved. I took a stab at it and came
 up with the series below. No docs or tests, and some of the refactoring
 in remote.c feels a little weird. I can't help but feel more of the
 logic from git push should be shared here.

 But it at least works with my rudimentary examples. I'm hoping it will
 make a good starting point for you to build on. Otherwise, I may get to
 it eventually, but it's not a high priority for me right now.

Thanks; I'll see what I can do about getting it to share code with 'git push'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Error in documentation for @{-n} in gitrevisions

2014-01-12 Thread Ramkumar Ramachandra
Kevin wrote:
 Either the documentation is wrong, and should be changed to nth
 branch/commit checkout out before the current one, or the behavior of
 @{-1} is wrong.

Yeah, the documentation needs to be updated. Patches welcome.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] t1507 (rev-parse-upstream): fix typo in test title

2014-01-12 Thread Ramkumar Ramachandra
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 t/t1507-rev-parse-upstream.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 2a19e79..15f2e7e 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -54,7 +54,7 @@ test_expect_success 'my-side@{upstream} resolves to correct 
full name' '
test refs/remotes/origin/side = $(full_name my-side@{u})
 '
 
-test_expect_success 'refs/heads/my-side@{upstream} does not resolve to 
my-side{upstream}' '
+test_expect_success 'refs/heads/my-side@{upstream} does not resolve to 
my-side@{upstream}' '
test_must_fail full_name refs/heads/my-side@{upstream}
 '
 
-- 
1.8.5.2.313.g5abf4c0.dirty

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


[PATCH 0/3] Minor preparation for @{publish}

2014-01-12 Thread Ramkumar Ramachandra
Hi,

I'm getting ready to switch timezones in a few days, so the @{publish}
series is on hold for some time. In the meantime, I thought I'd send
in a few patches early.

[1/3] is a minor typo fix I happened to notice while writing tests.

[2/3] is an is an excellent patch by Peff, that greatly helped
prettify the code. It's a pure refactor and should have no effect on
functionality.

[3/3] introduces branch-pushremote corresponding to branch-remote,
which is crucial for getting @{publish} from the branch_get()
interface. Peff had sent a similar patch, but mine has some subtle
differences.

Thanks.

Jeff King (1):
  interpret_branch_name: factor out upstream handling

Ramkumar Ramachandra (2):
  t1507 (rev-parse-upstream): fix typo in test title
  remote: introduce and fill branch-pushremote

 remote.c  | 15 ++--
 remote.h  |  3 ++
 sha1_name.c   | 83 +++
 t/t1507-rev-parse-upstream.sh |  2 +-
 4 files changed, 68 insertions(+), 35 deletions(-)

-- 
1.8.5.2.313.g5abf4c0.dirty

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


[PATCH 2/3] interpret_branch_name: factor out upstream handling

2014-01-12 Thread Ramkumar Ramachandra
From: Jeff King p...@peff.net

This function checks a few different @{}-constructs. The
early part checks for and dispatches us to helpers for each
construct, but the code for handling @{upstream} is inline.

Let's factor this out into its own function. This makes
interpret_branch_name more readable, and will make it much
simpler to add more constructs in future patches.

While we're at it, let's also break apart the refactored
code into a few helper functions. These will be useful when
we implement similar @{upstream}-like constructs.

Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 sha1_name.c | 83 ++---
 1 file changed, 52 insertions(+), 31 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index b1873d8..7ebb8ee 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1046,6 +1046,54 @@ static int reinterpret(const char *name, int namelen, 
int len, struct strbuf *bu
return ret - used + len;
 }
 
+static void set_shortened_ref(struct strbuf *buf, const char *ref)
+{
+   char *s = shorten_unambiguous_ref(ref, 0);
+   strbuf_reset(buf);
+   strbuf_addstr(buf, s);
+   free(s);
+}
+
+static const char *get_upstream_branch(const char *branch_buf, int len)
+{
+   char *branch = xstrndup(branch_buf, len);
+   struct branch *upstream = branch_get(*branch ? branch : NULL);
+
+   /*
+* Upstream can be NULL only if branch refers to HEAD and HEAD
+* points to something different than a branch.
+*/
+   if (!upstream)
+   die(_(HEAD does not point to a branch));
+   if (!upstream-merge || !upstream-merge[0]-dst) {
+   if (!ref_exists(upstream-refname))
+   die(_(No such branch: '%s'), branch);
+   if (!upstream-merge) {
+   die(_(No upstream configured for branch '%s'),
+   upstream-name);
+   }
+   die(
+   _(Upstream branch '%s' not stored as a remote-tracking 
branch),
+   upstream-merge[0]-src);
+   }
+   free(branch);
+
+   return upstream-merge[0]-dst;
+}
+
+static int interpret_upstream_mark(const char *name, int namelen,
+  int at, struct strbuf *buf)
+{
+   int len;
+
+   len = upstream_mark(name + at, namelen - at);
+   if (!len)
+   return -1;
+
+   set_shortened_ref(buf, get_upstream_branch(name, at));
+   return len + at;
+}
+
 /*
  * This reads short-hand syntax that not only evaluates to a commit
  * object name, but also can act as if the end user spelled the name
@@ -1070,9 +1118,7 @@ static int reinterpret(const char *name, int namelen, int 
len, struct strbuf *bu
 int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
 {
char *cp;
-   struct branch *upstream;
int len = interpret_nth_prior_checkout(name, buf);
-   int tmp_len;
 
if (!namelen)
namelen = strlen(name);
@@ -1094,36 +1140,11 @@ int interpret_branch_name(const char *name, int 
namelen, struct strbuf *buf)
if (len  0)
return reinterpret(name, namelen, len, buf);
 
-   tmp_len = upstream_mark(cp, namelen - (cp - name));
-   if (!tmp_len)
-   return -1;
+   len = interpret_upstream_mark(name, namelen, cp - name, buf);
+   if (len  0)
+   return len;
 
-   len = cp + tmp_len - name;
-   cp = xstrndup(name, cp - name);
-   upstream = branch_get(*cp ? cp : NULL);
-   /*
-* Upstream can be NULL only if cp refers to HEAD and HEAD
-* points to something different than a branch.
-*/
-   if (!upstream)
-   die(_(HEAD does not point to a branch));
-   if (!upstream-merge || !upstream-merge[0]-dst) {
-   if (!ref_exists(upstream-refname))
-   die(_(No such branch: '%s'), cp);
-   if (!upstream-merge) {
-   die(_(No upstream configured for branch '%s'),
-   upstream-name);
-   }
-   die(
-   _(Upstream branch '%s' not stored as a remote-tracking 
branch),
-   upstream-merge[0]-src);
-   }
-   free(cp);
-   cp = shorten_unambiguous_ref(upstream-merge[0]-dst, 0);
-   strbuf_reset(buf);
-   strbuf_addstr(buf, cp);
-   free(cp);
-   return len;
+   return -1;
 }
 
 int strbuf_branchname(struct strbuf *sb, const char *name)
-- 
1.8.5.2.313.g5abf4c0.dirty

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


[PATCH 3/3] remote: introduce and fill branch-pushremote

2014-01-12 Thread Ramkumar Ramachandra
When a caller uses branch_get() to retrieve a struct branch, they get
the per-branch remote name and a pointer to the remote struct. However,
they have no way of knowing about the per-branch pushremote from this
interface. So, let's expose that information via fields similar to
remote and remote_name; pushremote and pushremote_name.

Helped-by: Jeff King p...@peff.net
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 remote.c | 15 ---
 remote.h |  3 +++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/remote.c b/remote.c
index a89efab..286cdce 100644
--- a/remote.c
+++ b/remote.c
@@ -351,9 +351,10 @@ static int handle_config(const char *key, const char 
*value, void *cb)
explicit_default_remote_name = 1;
}
} else if (!strcmp(subkey, .pushremote)) {
+   if (git_config_string(branch-pushremote_name, key, 
value))
+   return -1;
if (branch == current_branch)
-   if (git_config_string(pushremote_name, key, 
value))
-   return -1;
+   pushremote_name = branch-pushremote_name;
} else if (!strcmp(subkey, .merge)) {
if (!value)
return config_error_nonbool(key);
@@ -1543,7 +1544,9 @@ struct branch *branch_get(const char *name)
ret = current_branch;
else
ret = make_branch(name, 0);
-   if (ret  ret-remote_name) {
+   if (!ret)
+   return ret;
+   if (ret-remote_name) {
ret-remote = remote_get(ret-remote_name);
if (ret-merge_nr) {
int i;
@@ -1557,6 +1560,12 @@ struct branch *branch_get(const char *name)
}
}
}
+   if (ret-pushremote_name)
+   ret-pushremote = remote_get(ret-pushremote_name);
+   else if (pushremote_name)
+   ret-pushremote = remote_get(pushremote_name);
+   else
+   ret-pushremote = ret-remote;
return ret;
 }
 
diff --git a/remote.h b/remote.h
index 00c6a76..ac5aadc 100644
--- a/remote.h
+++ b/remote.h
@@ -201,6 +201,9 @@ struct branch {
const char *remote_name;
struct remote *remote;
 
+   const char *pushremote_name;
+   struct remote *pushremote;
+
const char **merge_name;
struct refspec **merge;
int merge_nr;
-- 
1.8.5.2.313.g5abf4c0.dirty

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


Re: [PATCH 3/3] remote: introduce and fill branch-pushremote

2014-01-13 Thread Ramkumar Ramachandra
Jeff King wrote:
   2. If the current branch has a branch.*.pushremote set, but we want to
  know where a _different_ branch would be pushed, we have no way to
  access remote.pushdefault (it gets overwritten in the hunk above).

  @{upstream} does not have this problem, because it is _only_
  defined if branch.*.remote is set. There is no such thing as
  defaulting to a remote.default (or origin) there, and we never
  need to look at default_remote_name.

  For @{publish}, though, I think we will want that default. The
  common config will be to simply set remote.pushdefault, rather
  than setting up branch.*.pushremote for each branch, and we would
  want @{publish} to handle that properly.

Not sure I understand what the problem is. Let's say we have two
branches: master, and side with remote.pushdefault = ram,
branch.*.remote = origin, and branch.side.pushremote = peff. Now, when
I query master's pushremote, I get ram and when I query side's
pushremote, I get peff; all the logic for falling-back from
branch.*.pushremote to remote.pushdefault to branch.*.remote is in
branch_get(), so I need to do nothing extra on the caller-side. From
the caller's perspective, why does it matter if the pushremote of a
particular branch is due to branch.*.pushremote or remote.pushdefault?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/5] implement @{publish} shorthand

2014-01-24 Thread Ramkumar Ramachandra
Jeff King wrote:
 As far as merging it to 'next', I had not really intended it to go that
 far. :) It was more for Ram to use as a base.

Sorry about not having posted a follow-up yet; I'm adjusting to a new
timezone and environment.

 I find some of the
 refactoring questionable, including:

   1. The meaning of branch-pushremote is subtly different from that of
  branch-remote. Ram's followup refactoring did a better job of
  that (but he is missing the patches on top to finish out the
  feature).

   2. We are duplicating the where to push logic here. That should
  probably be factored out so that git push and @{publish} use
  the same logic.

 And of course there are no tests or documentation. It might work,
 though.

Actually, task (2) is somewhat involved: I still haven't figured out
how to share code with 'git push'.

 I don't mind if you want to merge it and do more work in-tree, but I do
 not think it should graduate as-is. And you may want check from Ram that
 he is not in the middle of his own version based on the patches he sent
 earlier, as reworking them on top of mine would probably just be
 needless extra work.

On that note, can you hold off graduating
jk/branch-at-publish-rebased, Junio? Hopefully, I'll come up with a
replacement over the weekend.

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


[TEST EMAIL] Testing rk/send-email-ssl-cert in pu

2014-01-26 Thread Ramkumar Ramachandra
Hi,

This email tests that 01645b7 (send-email: /etc/ssl/certs/ directory
may not be usable as ca_path, 2014-01-15) doesn't cause a regression.

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


Re: [PATCH] send-email: If the ca path is not specified, use the defaults

2014-01-26 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 This change could introduce a regression for people on a platform
 whose certificate directory is /etc/ssl/certs but its IO::Socket:SSL
 somehow fails to use it as SSL_ca_path without being told.

I can confirm that my git-send-email doesn't regress to the
pre-35035bbf state; my certificate directory is /etc/ssl/certs. I'm
somewhat surprised that IO::Socket::SSL picks the right file/
directory on every platform without being told explicitly. This change
definitely looks like the right fix.

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


Re: Git GSoC 2014

2014-02-13 Thread Ramkumar Ramachandra
Jeff King wrote:
   - ideas ideas ideas

I'll throw in a few ideas from half-finished work.

1. Speed up git-rebase--am.sh

Currently, git-rebase--am.sh is really slow because it dumps each
patch to a file using git-format-patch, and picks it up to apply
subsequently using git-am. Find a way to speed this up, without
sacrificing safety. You can use the continuation features of
cherry-pick, and dump to file only to persist state in the case of a
failure.

Language: Shell script, C
Difficulty: Most of the difficulty lies in what to do, not so much
how to do it. Might require modifying cherry-pick to do additional
work on failure.

2. Invent new conflict style

As an alternative to the diff3 conflict style, invent a conflict style
that shows the original unpatched segment along with the raw patch
text. The user can then apply the patch by hand.

Language: C
Difficulty: Since it was first written, very few people have touched
the xdiff portion of the code. Since the area is very core to git, the
series will have to go through a ton of iterations.

3. Rewrite git-branch to use git-for-each-ref

For higher flexibility in command-line options and output format, use
git for-each-ref to re-implement git-branch. The first task is to grow
features that are in branch but not fer into fer (like --column,
--merged, --contains). The second task is to refactor fer so that an
external program can call into it.

Language: C
Difficulty: fer was never written with the idea of being reusable; it
therefore persists a lot of global state, and even leaks memory in
some places. Refactoring it to be more modern is definitely a
challenge.

4. Implement @{publish}
(I just can't find the time to finish this)

@{publish} is a feature like @{upstream}, showing the state of the
publish-point in the case of triangular workflows. Implement this
while sharing code with git-push, and polish it until the prompt shows
publish-state.

Language: C, Shell script
Difficulty: Once you figure out how to share code with git-push, this
task should be relatively straightforward.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git GSoC 2014

2014-02-14 Thread Ramkumar Ramachandra
Jeff King wrote:
 1. Speed up git-rebase--am.sh

 Isn't the merge backend faster? I thought that was the point of it.

I suppose, but I thought git-rebase--am.sh (the default flavor) could
be improved by leveraging relatively new cherry-pick features; I
assumed that the reason it was using format-patch/ am was because it
was written before cherry-pick matured. Alternatively, can you think
of a project that involves working on the sequencer?

 3. Rewrite git-branch to use git-for-each-ref

 I actually have this about 95% done, waiting for the patches to be
 polished. So I don't think it makes a good GSoC project (it would be
 stupid to start from scratch, and polishing my patches is a lame
 project).

Oh. I look forward to using a nicer git-branch soon.

 4. Implement @{publish}

 I think this could be a good GSoC-sized topic. I'm going to adjust the
 title to be better support for triangular workflows. I think they may
 want to examine other issues in the area, rather than drilling down on
 @{publish} in particular (but ultimately, it is up to the student to
 propose what they want to do).

That makes the project a little more open-ended then. I like it.

I was hoping you'd have more comments on 3. Invent new conflict
style. Although I'm not sure the conflict style I proposed would be
terribly useful in the general case, it'll give the student an
opportunity to look at older/ lesser-known portions of the codebase.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Feb 2014, #06; Wed, 19)

2014-02-21 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 * jk/branch-at-publish-rebased (2014-01-17) 5 commits
  - t1507 (rev-parse-upstream): fix typo in test title
  - implement @{publish} shorthand
  - branch_get: provide per-branch pushremote pointers
  - branch_get: return early on error
  - sha1_name: refactor upstream_mark

  Give an easier access to the tracking branches from other side in
  a triangular workflow by introducing B@{publish} that works in a
  similar way to how B@{upstream} does.

  Meant to be used as a basis for whatever Ram wants to build on.

  Will hold.

Since I've decided that I don't have the time to work on this, we've
added this to the list of project ideas for GSoC 2014
(http://git.github.io/SoC-2014-Ideas.html). Hopefully, a student will
come along and volunteer to finish this.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Documentation/merge-strategies: avoid hyphenated commands

2014-03-16 Thread Ramkumar Ramachandra
Replace git-pull and git-merge with the corresponding un-hyphenated
versions. While at it, use ` to mark it up instead of '.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Documentation/merge-strategies.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/merge-strategies.txt 
b/Documentation/merge-strategies.txt
index 12133b9..19f3a5d 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -1,10 +1,10 @@
 MERGE STRATEGIES
 
 
-The merge mechanism ('git-merge' and 'git-pull' commands) allows the
+The merge mechanism (`git merge` and `git pull` commands) allows the
 backend 'merge strategies' to be chosen with `-s` option.  Some strategies
 can also take their own options, which can be passed by giving `-Xoption`
-arguments to 'git-merge' and/or 'git-pull'.
+arguments to `git merge` and/or `git pull`.
 
 resolve::
This can only resolve two heads (i.e. the current branch
-- 
1.9.0.431.g014438b

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


[PATCH pu] Documentation/giteveryday: fix some obvious problems

2014-03-16 Thread Ramkumar Ramachandra
Fix a few minor things.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Philip,

 I spotted a few obvious issues with your giteveryday patch in
 pu. Maybe Junio can squash this into your patch? Contents are still a
 bit stale, but I'm not sure what other markup problems are there.

 Documentation/giteveryday.txt | 78 +--
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/Documentation/giteveryday.txt b/Documentation/giteveryday.txt
index 8dc298f..82ff8ec 100644
--- a/Documentation/giteveryday.txt
+++ b/Documentation/giteveryday.txt
@@ -35,8 +35,6 @@ following commands.
 
   * linkgit:git-init[1] to create a new repository.
 
-  * linkgit:git-show-branch[1] to see where you are.
-
   * linkgit:git-log[1] to see what happened.
 
   * linkgit:git-checkout[1] and linkgit:git-branch[1] to switch
@@ -61,8 +59,8 @@ following commands.
 Examples
 
 
-Use a tarball as a starting point for a new repository.::
-+
+Use a tarball as a starting point for a new repository:
+
 
 $ tar zxf frotz.tar.gz
 $ cd frotz
@@ -71,12 +69,12 @@ $ git add . 1
 $ git commit -m import of frotz source tree.
 $ git tag v2.43 2
 
-+
+
 1 add everything under the current directory.
 2 make a lightweight, unannotated tag.
 
-Create a topic branch and develop.::
-+
+Create a topic branch and develop:
+
 
 $ git checkout -b alsa-audio 1
 $ edit/compile/test
@@ -95,7 +93,7 @@ $ git merge alsa-audio 10
 $ git log --since='3 days ago' 11
 $ git log v2.43.. curses/ 12
 
-+
+
 1 create a new topic branch.
 2 revert your botched changes in `curses/ux_audio_oss.c`.
 3 you need to tell Git if you added a new file; removal and
@@ -137,8 +135,8 @@ addition to the ones needed by a standalone developer.
 Examples
 
 
-Clone the upstream and work on it.  Feed changes to upstream.::
-+
+Clone the upstream and work on it.  Feed changes to upstream:
+
 
 $ git clone git://git.kernel.org/pub/scm/.../torvalds/linux-2.6 my2.6
 $ cd my2.6
@@ -151,7 +149,7 @@ $ git reset --hard ORIG_HEAD 6
 $ git gc 7
 $ git fetch --tags 8
 
-+
+
 1 repeat as needed.
 2 extract patches from your branch for e-mail submission.
 3 `git pull` fetches from `origin` by default and merges into the
@@ -166,8 +164,8 @@ area we are interested in.
 and store them under `.git/refs/tags/`.
 
 
-Push into another repository.::
-+
+Push into another repository:
+
 
 satellite$ git clone mothership:frotz frotz 1
 satellite$ cd frotz
@@ -185,7 +183,7 @@ mothership$ cd frotz
 mothership$ git checkout master
 mothership$ git merge satellite/master 5
 
-+
+
 1 mothership machine has a frotz repository under your home
 directory; clone from it to start a repository on the satellite
 machine.
@@ -200,8 +198,8 @@ as a back-up method.
 5 on mothership machine, merge the work done on the satellite
 machine into the master branch.
 
-Branch off of a specific tag.::
-+
+Branch off of a specific tag:
+
 
 $ git checkout -b private2.6.14 v2.6.14 1
 $ edit/compile/test; git commit -a
@@ -209,7 +207,7 @@ $ git checkout master
 $ git format-patch -k -m --stdout v2.6.14..private2.6.14 |
   git am -3 -k 2
 
-+
+
 1 create a private branch based on a well known (but somewhat behind)
 tag.
 2 forward port all changes in `private2.6.14` branch to `master` branch
@@ -240,8 +238,8 @@ commands in addition to the ones needed by participants.
 Examples
 
 
-My typical Git day.::
-+
+My typical Git day:
+
 
 $ git status 1
 $ git show-branch 2
@@ -261,10 +259,10 @@ $ git cherry-pick master~4 9
 $ compile/test
 $ git tag -s -m GIT 0.99.9x v0.99.9x 10
 $ git fetch ko  git show-branch master maint 'tags/ko-*' 11
-$ git push ko 12
-$ git push ko v0.99.9x 13
+$ git push ko
+$ git push ko v0.99.9x
 
-+
+
 1 see what I was in the middle of doing, if any.
 2 see what topic branches I have and think about how ready
 they are.
@@ -282,7 +280,7 @@ master, nor exposed as a part of a stable branch.
 11 make sure I did not accidentally rewind master beyond what I
 already pushed out.  `ko` shorthand points at the repository I have
 at kernel.org, and looks like this:
-+
+
 
 $ cat .git/remotes/ko
 URL: kernel.org:/pub/scm/git/git.git
@@ -294,7 +292,7 @@ Push: next
 Push: +pu
 Push: maint
 
-+
+
 In the output from `git show-branch`, `master` should have
 everything `ko-master` has, and `next` should have
 everything `ko-next` has.
@@ -322,24 +320,24 @@ example of managing a shared central repository.
 Examples
 
 We assume the following in /etc/services::
-+
+
 
 $ grep 9418 /etc/services
 git9418/tcp# Git Version Control System
 
 
-Run git-daemon to serve /pub/scm from inetd.::
-+
+Run git-daemon to serve /pub/scm from inetd:
+
 
 $ grep git /etc/inetd.conf
 gitstream  tcp nowait  nobody \
   /usr/bin

Re: What's cooking in git.git (Mar 2014, #03; Fri, 14)

2014-03-16 Thread Ramkumar Ramachandra
Philip Oakley wrote:
 * po/everyday-doc (2014-01-27) 1 commit
 - Make 'git help everyday' work

 This may make the said command to emit something, but the source is
 not meant to be formatted into a manual pages to begin with, and
 also its contents are a bit stale.  It may be a good first step in
 the right direction, but needs more work to at least get the
 mark-up right before public consumption.

 I'm not sure what elements you feel need adjustment. At the moment the
 markup formats quite reasonably to my eyes, both as an HTML page and as a
 man page.

I sent you a small patch fixing some minor markup issues.

 That said, the (lack of) introduction could do with a paragraph to introduce
 the guide. I have something in draft..

 I had a thought that it may be worth a slight rearrangement to add a section
 covering the setting up of one's remotes, depending whether it was forked,
 corporate, or independent, but the lack of resolution on the next
 {publish/push} topic makes such a re-write awkward at this stage. (Ram cc'd)

Before attempting to introduce remote.pushdefault and triangular
workflows, I'd first fix the main issue: stale content. I'm not sure
who uses git show-branch or mailx anymore, for instance.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 9/9] sha1_name: add support for @{publish} marks

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 @@ -1068,23 +1069,35 @@ static const char *get_upstream_branch(const char 
 *name_buf, int len)
  */
 if (!branch)
 die(_(HEAD does not point to a branch));
 -   if (!branch-merge || !branch-merge[0]-dst) {
 -   if (!ref_exists(branch-refname))
 -   die(_(No such branch: '%s'), name);
 -   if (!branch-merge) {
 -   die(_(No upstream configured for branch '%s'),
 -   branch-name);
 +   switch (type) {
 +   case 'u':
 +   if (!branch-merge || !branch-merge[0]-dst) {
 +   if (!ref_exists(branch-refname))
 +   die(_(No such branch: '%s'), name);
 +   if (!branch-merge) {
 +   die(_(No upstream configured for branch 
 '%s'),
 +   branch-name);
 +   }
 +   die(
 +   _(Upstream branch '%s' not stored as a 
 remote-tracking branch),
 +   branch-merge[0]-src);
 +   }
 +   tracking = branch-merge[0]-dst;
 +   break;
 +   case 'p':
 +   if (!branch-push.dst) {
 +   die(_(No publish configured for branch '%s'),
 +   branch-name);

This assumes a push.default value of 'current' or 'matching'. What
happens if push.default is set to 'nothing' or 'upstream', for
instance?

p.s- Good to see you back on the list :)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 8/9] sha1_name: simplify track finding

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 It's more efficient to check for the braces first.

Why is it more efficient? So you can error out quickly in the case of
a malformed string? I'm personally not thrilled about this change.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 7/9] sha1_name: cleanup interpret_branch_name()

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
  sha1_name.c | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)

I like this variable rename. This instance has annoyed me in the past.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 9/9] sha1_name: add support for @{publish} marks

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 diff --git a/sha1_name.c b/sha1_name.c
 index aa3f3e0..a36852d 100644
 --- a/sha1_name.c
 +++ b/sha1_name.c
 @@ -415,9 +415,9 @@ static int ambiguous_path(const char *path, int len)
 return slash;
  }

 -static inline int upstream_mark(const char *string, int len)
 +static inline int tracking_mark(const char *string, int len)
  {
 -   const char *suffix[] = { upstream, u };
 +   const char *suffix[] = { upstream, u, publish, p };

Oh, another thing: on some threads, people decided that @{push}
would be a more apt name (+ alias @{u} to @{pull} for symmetry).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 6/9] branch: display publish branch

2014-04-10 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com

Please write a commit message, preferably showing the new git-branch output.

I noticed that this only picks up a publish-branch if
branch.*.pushremote is configured. What happened to the case when
remote.pushdefault is configured?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 03/12] revert/cherry-pick: add --quiet option

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 @@ -635,9 +637,10 @@ static int do_pick_commit(struct commit *commit, struct 
 replay_opts *opts)
 }

 if (opts-skip_empty  is_index_unchanged() == 1) {
 -   warning(_(skipping %s... %s),
 -   find_unique_abbrev(commit-object.sha1, 
 DEFAULT_ABBREV),
 -   msg.subject);
 +   if (!opts-quiet)
 +   warning(_(skipping %s... %s),
 +   find_unique_abbrev(commit-object.sha1, 
 DEFAULT_ABBREV),
 +   msg.subject);

Personally, I don't see much value in inventing a new option for
suppressing one message.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 04/12] revert/cherry-pick: add --skip option

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 Akin to 'am --skip' and 'rebase --skip'.

I don't recall why my original sequencer series didn't include this
option. Perhaps Jonathan remembers?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 06/12] builtin: add rewrite helper

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 So that we can load and store rewrites, as well as other operations on a
 list of rewritten commits.

Please elaborate. Explain why this code shouldn't go in sequencer.c.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 12/12] cherry-pick: copy notes and run hooks

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 If no action-name is specified, nothing is done.

Why? Is it because git-rebase implements its own notes-copy-on-rewrite logic?

Otherwise, I agree with the goal of making notes.rewrite.command
work for cherry-pick.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 12/12] cherry-pick: copy notes and run hooks

2014-04-24 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index 43631b4..fd085e1 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -248,7 +248,7 @@ pick_one () {

 test -d $rewritten 
 pick_one_preserving_merges $@  return
 -   output eval git cherry-pick $strategy_args $empty_args $ff $@
 +   output eval git cherry-pick --action-name '' $strategy_args 
 $empty_args $ff $@

Passing an empty action-name looks quite ugly. Is there a better way
to achieve this?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] rebase -i: handle Nothing to do case with autostash

2014-05-19 Thread Ramkumar Ramachandra
When a user invokes

  $ git rebase -i @~3

with dirty files and rebase.autostash turned on, and exits the $EDITOR
with an empty buffer, the autostash fails to apply. Although the primary
focus of rr/rebase-autostash was to get the git-rebase--backend.sh
scripts to return control to git-rebase.sh, it missed this case in
git-rebase--interactive.sh. Since this case is unlike the other cases
which return control for housekeeping, assign it a special return status
and handle that return value explicitly in git-rebase.sh.

Reported-by: Karen Etheridge et...@cpan.org
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Thanks to Karen for reporting this.

 I chose 2 arbitrarily. Let me know if you have a rationale for other
 return values.

 git-rebase--interactive.sh |  4 ++--
 git-rebase.sh  | 11 ++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..f267d8b 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1049,14 +1049,14 @@ fi
 
 
 has_action $todo ||
-   die_abort Nothing to do
+   return 2
 
 cp $todo $todo.backup
 git_sequence_editor $todo ||
die_abort Could not execute editor
 
 has_action $todo ||
-   die_abort Nothing to do
+   return 2
 
 expand_todo_ids
 
diff --git a/git-rebase.sh b/git-rebase.sh
index 4543815..47ca3b9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -155,7 +155,7 @@ move_to_original_branch () {
esac
 }
 
-finish_rebase () {
+apply_autostash () {
if test -f $state_dir/autostash
then
stash_sha1=$(cat $state_dir/autostash)
@@ -171,6 +171,10 @@ You can run git stash pop or git stash drop at any 
time.
 '
fi
fi
+}
+
+finish_rebase () {
+   apply_autostash 
git gc --auto 
rm -rf $state_dir
 }
@@ -186,6 +190,11 @@ run_specific_rebase () {
if test $ret -eq 0
then
finish_rebase
+   elif test $ret -eq 2 # special exit status for rebase -i
+   then
+   apply_autostash 
+   rm -rf $state_dir 
+   die Nothing to do
fi
exit $ret
 }
-- 
2.0.0.rc2.20.gfc2568d.dirty

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


Re: [PATCH 10/10] pack-revindex: radix-sort the revindex

2013-07-10 Thread Ramkumar Ramachandra
Jeff King wrote:
 That does O(n log n) offset comparisons, and profiling shows
 that we spend most of our time in cmp_offset. However, since
 we are sorting on a simple off_t, we can use numeric sorts
 that perform better. A radix sort can run in O(k*n), where k
 is the number of digits in our number. For a 64-bit off_t,
 using 16-bit digits gives us k=4.

Wait, isn't off_t a signed data type?  Did you account for that in
your algorithm?

 On the linux.git repo, with about 3M objects to sort, this
 yields a 400% speedup. Here are the best-of-five numbers for
 running echo HEAD | git cat-file --batch-disk-size, which
 is dominated by time spent building the pack revindex:

Okay.

 diff --git a/pack-revindex.c b/pack-revindex.c
 index 1aa9754..9365bc2 100644
 --- a/pack-revindex.c
 +++ b/pack-revindex.c
 @@ -59,11 +59,85 @@ static int cmp_offset(const void *a_, const void *b_)
 /* revindex elements are lazily initialized */
  }

 -static int cmp_offset(const void *a_, const void *b_)
 +/*
 + * This is a least-significant-digit radix sort.
 + */

Any particular reason for choosing LSD, and not MSD?

 +#define DIGIT_SIZE (16)
 +#define BUCKETS (1  DIGIT_SIZE)

Okay, NUMBER_OF_BUCKETS = 2^RADIX, and you choose a hex radix.  Is
off_t guaranteed to be fixed-length though?  I thought only the ones
in stdint.h were guaranteed to be fixed-length?

 +   /*
 +* We want to know the bucket that a[i] will go into when we are using
 +* the digit that is N bits from the (least significant) end.
 +*/
 +#define BUCKET_FOR(a, i, bits) (((a)[(i)].offset  (bits))  (BUCKETS-1))

Ouch!  This is unreadable.  Just write an inline function instead?  A
% would've been easier on the eyes, but you chose base-16.

 +   /*
 +* We need O(n) temporary storage, so we sort back and forth between
 +* the real array and our tmp storage. To keep them straight, we 
 always
 +* sort from a into buckets in b.
 +*/
 +   struct revindex_entry *tmp = xcalloc(n, sizeof(*tmp));

Shouldn't this be sizeof (struct revindex_entry), since tmp hasn't
been declared yet?  Also, s/n/revindex_nr/, and something more
appropriate for tmp?

 +   struct revindex_entry *a = entries, *b = tmp;

It's starting to look like you have something against descriptive names ;)

 +   int bits = 0;
 +   unsigned *pos = xmalloc(BUCKETS * sizeof(*pos));

sizeof(unsigned int), for clarity, if not anything else.  You picked
malloc over calloc here, because you didn't want to incur the extra
cost of zero-initializing the memory?  Also, pos is the actual buckets
array, I presume (hence unsigned, because there can't be a negative
number of keys in any bucket)?

 +   while (max  bits) {

No clue what max is.  Looked at the caller and figured out that it's
the pack-size, although I'm still clueless about why it's appearing
here.

 +   struct revindex_entry *swap;
 +   int i;
 +
 +   memset(pos, 0, BUCKETS * sizeof(*pos));

Ah, so that's why you used malloc there.  Wait, shouldn't this be
memset(pos, 0, sizeof(*pos))?

 +   for (i = 0; i  n; i++)
 +   pos[BUCKET_FOR(a, i, bits)]++;

Okay, so you know how many numbers are in each bucket.

 +   for (i = 1; i  BUCKETS; i++)
 +   pos[i] += pos[i-1];

Cumulative sums; right.

 +   for (i = n - 1; i = 0; i--)
 +   b[--pos[BUCKET_FOR(a, i, bits)]] = a[i];

Classical queue.  You could've gone for something more complex, but I
don't think it would have been worth the extra complexity.

 +   swap = a;
 +   a = b;
 +   b = swap;

Wait a minute: why don't you just throw away b?  You're going to
rebuild the queue in the next iteration anyway, no?  a is what is
being sorted.

 +   /* And bump our bits for the next round. */
 +   bits += DIGIT_SIZE;

I'd have gone for a nice for-loop.

 +   /*
 +* If we ended with our data in the original array, great. If not,
 +* we have to move it back from the temporary storage.
 +*/
 +   if (a != entries)
 +   memcpy(entries, tmp, n * sizeof(*entries));

How could a be different from entries?  It has no memory allocated for
itself, no?  Why did you even create a, and not directly operate on
entries?

 +   free(tmp);
 +   free(pos);

Overall, I found it quite confusing :(

 +#undef BUCKET_FOR

Why not DIGIT_SIZE and BUCKETS too, while at it?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/10] cat-file: add --batch-check=format

2013-07-10 Thread Ramkumar Ramachandra
Jeff King wrote:
 +If `--batch` or `--batch-check` is given, `cat-file` will read objects
 +from stdin, one per line, and print information about them.
 +
 +You can specify the information shown for each object by using a custom
 +`format`. The `format` is copied literally to stdout for each
 +object, with placeholders of the form `%(atom)` expanded, followed by a
 +newline. The available atoms are:
 +
 +If no format is specified, the default format is `%(objectname)
 +%(objecttype) %(objectsize)`.
 +
 +If `--batch` is specified, the object information is followed by the
 +object contents (consisting of `%(objectsize)` bytes), followed by a
 +newline.

I find this slightly hideous, and would have expected an
%(objectcontents) or similar.

Perhaps --batch-check should become a non-configurable alias for
--batch=%(objectname) %(objecttype) %(objectsize), and let --batch
default to the format %(objectname) %(objecttype) %(objectsize)
%(objectcontents).

I'm frankly okay with not having --pretty, and making the output in
the non-batch mode non-configurable (does anyone care?).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/10] cat-file: split --batch input lines on whitespace

2013-07-10 Thread Ramkumar Ramachandra
Jeff King wrote:
   git rev-list --objects HEAD |
   git cat-file --batch-check='%(objectsize) %(text)'

If anything, I would have expected %(rest), not %(text).  This atom is
specific to commands that accept input via stdin (i.e. not log, f-e-r,
branch, or anything else I can think of).

Also, this makes me wonder if %(field:0), %(field:1), and probably
%(field:@) are good ideas.  Even if we go down that road, I don't
think %(rest) is a problem per-se.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] send-email: squelch warning from Net::SMTP::SSL

2013-07-14 Thread Ramkumar Ramachandra
Torsten Bögershausen wrote:
 /usr/bin/perl -MIO::Socket::SSL -e 'print $IO::Socket::SSL::VERSION\n;'
 1.22

This is ancient!  (I have 1.84).  Is it not possible to do an
ssl-verify-peer in older versions (is it exported as something else)?
The older versions don't display the warning anyway, and this series
is about squelching the warning in newer versions.  Does

  require IO::Socket::SSL qw(SSL_VERIFY_NONE SSL_VERIFY_PEER) or print
warning: not using SSL_VERIFY_PEER due to outdated IO::Socket::SSL

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


Re: [PATCH v2 1/2] send-email: squelch warning from Net::SMTP::SSL

2013-07-14 Thread Ramkumar Ramachandra
brian m. carlson wrote:
 I didn't stick the require in the eval because git-send-email will fail
 in this case anyway if you don't have it, since Net::SMTP::SSL requires
 it.  Let me know if you want a patch for this on top of the existing two
 in this series and I'll provide one.

Yeah, that'd be nice.  I'm not enjoying this Perl-wrestling very much.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re* [PATCH] send-email: improve SSL certificate verification

2013-07-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 Ramkumar, as you will still be the primary author of the resulting
 commit, I tentatively added a line for your sign-off even though you
 haven't signed off _this_ version yet, so that I would not forget.
 If this version looks good, please tell us you would.

Yeah, it seems to work fine for me.  Sign-off is fine.

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


Re: Git Clone Parameter

2013-07-19 Thread Ramkumar Ramachandra
Allan Acheampong wrote:
 I could write a script with for each in but thats way too much hassle

  $ git for-each-ref --format=%(refname) refs/remotes/origin/ | sed
's/refs\/remotes\/origin\///;/HEAD\|master/d' | xargs git checkout -b

(completely untested ofcourse)

Do you see what the problem is immediately?  There's nothing special
about origin: I could have branches with the same name on several
remotes.  Without detaching local branches from remote branches, there
is no distributed workflow: your central workflow is just a special
case.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC] checkout --rebase

2013-07-19 Thread Ramkumar Ramachandra
Hi,

I'm often work on small topic branches, and find myself doing this quite often:

  # on branch master
  $ git checkout um-build
  $ git rebase master

This is horribly inefficient; the first operation takes a _really_
long time to complete, since master updates itself very often and I
have to check out an old worktree.  So, I work around this issue by
doing:

  # on branch master
  $ git checkout -b um-build-2
  $ git cherry-pick ..um-build
  $ git branch -M um-build

... and I scripted it.  Perhaps we should get this functionality in
core, as `git checkout --rebase` (or something)?

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


[RFC] Delete current branch

2013-07-19 Thread Ramkumar Ramachandra
Hi,

Many of my ideas turn out to be really stupid, and I need to throw
away my feature branch.  So, I find myself doing this often:

  # on branch menuconfig-jk
  $ git checkout master
  $ git branch -DBACKSAPCE
  # er, what was the branch name again?
  $ git checkout -
  # Ah
  $ git checkout master
  $ git branch -D menuconfig-jk

So, I scripted it for myself.  Perhaps we should get the functionality
in core as `git branch -Dc` (c for current; or something)?

Also, perhaps a `git describe -` corresponding to `git checkout -`?
Then I can use it with --contains --all to get the name of the
previous branch.

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


Re: [RFC] checkout --rebase

2013-07-19 Thread Ramkumar Ramachandra
Sebastian Staudt wrote:
 Isn't this what you want?

  $ git rebase master um-build

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


Re: [RFC] checkout --rebase

2013-07-19 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 Sebastian Staudt wrote:
 Isn't this what you want?

  $ git rebase master um-build

 Ha, yes.  Sorry about the stupidity.

Hm, I'm not entirely sure how to optimize the codepath to eliminate
the checkout.  It seems to be absolutely necessary atleast in the -i
codepath.

Let's inspect when $switch_to is set:

# Is it rebase other $branchname or rebase other $commit?
branch_name=$1
switch_to=$1

$revisions seems to be set correctly, and rebase--am does a
move_to_original_branch as the last step.  So, I wonder if this will
work:

diff --git a/git-rebase.sh b/git-rebase.sh
index 0039ecf..7405d9a 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -542,9 +542,15 @@ then
if test -z $force_rebase
then
# Lazily switch to the target branch if needed...
-   test -z $switch_to ||
-   GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $switch_to \
-   git checkout $switch_to --
+   if ! test -z $switch_to; then
+   if $type = am; then
+   GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout 
-b $switch_to-2 \
+   git checkout -b $switch_to-2 --
+   else
+   GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout 
$switch_to \
+   git checkout $switch_to --
+   fi
+   fi
say $(eval_gettext Current branch \$branch_name is up to 
date.)
finish_rebase
exit 0

(ofcourse, we still need a mechanism to remove this temporary
$switch_to-2 branch)

What about rebase--merge?  Can we eliminate the checkout then?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] checkout --rebase

2013-07-19 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 diff --git a/git-rebase.sh b/git-rebase.sh
 index 0039ecf..7405d9a 100755
 --- a/git-rebase.sh
 +++ b/git-rebase.sh

Er, sorry.  I think it suffices to check that $branch_name equals
HEAD, for this optimization.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] checkout --rebase

2013-07-19 Thread Ramkumar Ramachandra
Damn it: checkout doesn't get executed at all because the $mb = $onto
condition fails; I was looking in the wrong place.  It's format-patch
and am that are slowing things down terribly.  Has anyone looked into
stripping out the dependency?  Why is cherry-pick deficient?

My current workaround is to use the faster `--keep-empty` codepath,
but it doesn't persist state on conflict.  Isn't this an undocumented
fact (aka. bug)?

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


Re: [RFC] Delete current branch

2013-07-19 Thread Ramkumar Ramachandra
Andreas Schwab wrote:
 Ramkumar Ramachandra artag...@gmail.com writes:

   # er, what was the branch name again?
   $ git checkout -

 You could take a look in the reflog.

Yeah, or use the @{-N} revision to do that for me.  My scripted
version is essentially:

  test true = $(git rev-parse --is-inside-work-tree 2/dev/null) || exit 1
  git checkout master
  git branch -D @{-1}

The main problem is the hard-coding of master: I suppose I could
replace that with @{-1} too.

Not a big deal: I was just wondering if others use it often enough for
it to become `branch -Dc` in core; @{-N} is quite obscure.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Delete current branch

2013-07-19 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 Did you know that the general way to spell the branch previously you
 were on is @{-1} and checkout - is an ugly special case that is
 possible only because checkout does not happen to take a - as a
 valid argument that means something else (like the more usual read
 from standard input)?

I disagree that it is ugly: it's a very commonly used shortcut that I
like.  I love it so much that I have the following in my ~/.zshrc:

function - () {
if test true = $(g rp --is-inside-work-tree 2/dev/null); then
g co -
else
cd - /dev/null
fi
}

So, I just

  $ -

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


Re: Git Clone Parameter

2013-07-19 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
   git branch -t $branchname origin/$branchname

A couple of notes here:

1. I use git branch -u personally.  Why the -t variant?
2. Don't we auto-track? (or is that only on checkout)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git Clone Parameter

2013-07-19 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 You would at least need xargs -n 1 for the produced command line
 to make any sense, and it is wasteful to actually check out each
 and every branch to the working tree only to create it.

Right.  xargs -n 1, git branch, and refname:short.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] Delete current branch

2013-07-19 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 That limits the context we could use - and we cannot consistently
 use it everywhere. I find _that_ ugly from the design cleanliness
 point of view.

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


Re: [PATCH] Fix compilation on OS X.

2013-07-20 Thread Ramkumar Ramachandra
Benoit Sigoure wrote:
 diff --git a/compat/unsetenv.c b/compat/unsetenv.c
 index 4ea1856..addf3dc 100644
 --- a/compat/unsetenv.c
 +++ b/compat/unsetenv.c
 @@ -1,5 +1,10 @@
  #include ../git-compat-util.h

 +#ifdef __APPLE__
 +// On OS X libc headers don't define this symbol.
 +extern char **environ;
 +#endif
 +

Shouldn't this go into git-compat-util.h, since there may be other
files depending on this variable?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Dead link

2013-07-20 Thread Ramkumar Ramachandra
Ondřej Bílka wrote:
 http://marc.theaimsgroup.com/?l=gitm=112927316408690w=2

Just run a sed 's|http://marc.theaimsgroup.com|http://marc.info|', and
submit the resulting patch.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2013, #03; Tue, 9)

2013-07-23 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 * mv/merge-ff-tristate (2013-07-02) 1 commit
   (merged to 'next' on 2013-07-09 at c32b95d)
  + merge: handle --ff/--no-ff/--ff-only as a tri-state option

Sorry I didn't notice sooner, but I was confused by the second test
title this added:

test_expect_success 'option --ff-only overwrites merge.ff=only config' '
git reset --hard c0 
test_config merge.ff only 
git merge --no-ff c1
'

How is --ff-only overwriting merge.ff=only here?  Was it a typo?

-- 8 --
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 3ff5fb8..aea8137 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -502,7 +502,7 @@ test_expect_success 'option --ff-only overwrites --no-ff' '
test_must_fail git merge --no-ff --ff-only c2
 '

-test_expect_success 'option --ff-only overwrites merge.ff=only config' '
+test_expect_success 'option --no-ff overwrites merge.ff=only config' '
git reset --hard c0 
test_config merge.ff only 
git merge --no-ff c1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] commit: correct advice about aborting a cherry-pick

2013-07-26 Thread Ramkumar Ramachandra
When a cherry-pick results in an empty commit, git prints:

  The previous cherry-pick is now empty, possibly due to conflict resolution.
  If you wish to commit it anyway, use:

  git commit --allow-empty

  Otherwise, please use 'git reset'

The last line is plain wrong in the case of a ranged pick, as a 'git
reset' will fail to remove $GIT_DIR/sequencer, failing a subsequent
cherry-pick or revert.  Change the advice to:

  git cherry-pick --abort

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Another candidate for maint?

 I'd also really like to squelch this with an advice.* variable; any
 suggestions?

 builtin/commit.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 003bd7d..1b213f7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -64,7 +64,10 @@ N_(The previous cherry-pick is now empty, possibly due to 
conflict resolution.\
 \n
 git commit --allow-empty\n
 \n
-Otherwise, please use 'git reset'\n);
+Otherwise, use:\n
+\n
+git cherry-pick --abort\n
+\n);
 
 static const char *use_message_buffer;
 static const char commit_editmsg[] = COMMIT_EDITMSG;
-- 
1.8.4.rc0.1.g8f6a3e5.dirty

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


Re: [PATCH] commit: correct advice about aborting a cherry-pick

2013-07-26 Thread Ramkumar Ramachandra
Jeff King wrote:
 Hmm. I don't think I've run across this message myself, so perhaps I do
 not understand the situation.

It's very simple.

  # on branch master
  $ git checkout -b test
  $ git cherry-pick master
  $ ls .git/sequencer
  # missing

In the pseudo multi-pick case (I say pseudo because there's really
just one commit to pick):

  # on branch master
  $ git checkout -b test
  $ git cherry-pick master~..
  $ ls .git/sequencer

cat .git/sequencer/todo if you like.

   2. Skip this commit and continue the rest of the cherry-pick sequence.

Nope, this is unsupported afaik.

 Those are the options presented when rebase runs into an empty commit,
 where (2) is presented as rebase --skip. I'm not sure how to do that
 here; is it just cherry-pick --continue?

No, --continue will just print the same message over and over again.

Yes, the whole ranged cherry-pick thing can use a lot more polish.

   1. What happened (the cherry-pick is empty).

   2. How to proceed from here (allow-empty, abort, etc).

 You still want to say (1), but (2) is useless to old-timers.  Probably
 something like advice.cherryPickInstructions would be a good name for an
 option to squelch (2), and it should apply wherever we tell the user how
 to proceed. Potentially it should even be advice.sequenceInstructions,
 and apply to rebase and am as well.

Good suggestion.  I'll pick advice.cherryPickInstructions when I
decide to polish sequencer.c a bit.

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


Re: [PATCH] commit: correct advice about aborting a cherry-pick

2013-07-26 Thread Ramkumar Ramachandra
Jeff King wrote:
 Ah. I don't mind improving the message in the meantime, but it sounds
 like this is a deficiency in sequenced cherry-pick that needs addressed
 eventually.

I'm especially irked by how slow rebase--am is, and want to replace it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: correct advice about aborting a cherry-pick

2013-07-27 Thread Ramkumar Ramachandra
Jeff King wrote:
  builtin/commit.c | 25 ++---
  1 file changed, 22 insertions(+), 3 deletions(-)

Overall, highly inelegant.  The single-commit pick has been special
cased only because we needed to preserve backward compatibility: I
would hate for the detail to be user-visible.  I'd have expected a
series that polishes sequencer.c instead.

But whatever.  I'm going to squelch them anyway.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: correct advice about aborting a cherry-pick

2013-07-27 Thread Ramkumar Ramachandra
Jonathan Nieder wrote:
 Your patch is just swapping out git reset for cherry-pick --abort,
 so I think that is a good improvement in the meantime.

 Um, wasn't the idea of the original message that you can run git
 reset and then git cherry-pick --continue?

No, and I don't know where you got that idea.  Certainly not by
reading the history.

37f7a85 (Teach commit about CHERRY_PICK_HEAD, 2011-02-19) introduced
that string.  This happened before I introduced cherry-pick --continue
in 5a5d80f (revert: Introduce --continue to continue the operation,
2011-08-04).

A proper solution to the problem would involve polishing sequencer.c,
and probably getting --skip-empty so it behaves more like rebase.  In
case you missed it, one person already did that work and posted the
code to the list [1].

[1]: http://article.gmane.org/gmane.comp.version-control.git/226982
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[BUG] git_path() returns relative paths

2013-07-27 Thread Ramkumar Ramachandra
Hi,

I noticed a regression in the latest master, and I've been trying to
debug it for 30 minutes now.  I'm still clueless about the root cause,
but I'll list whatever I found so far:

I suddenly noticed that I wasn't able to commit to a certain
repository with submodules anymore.  This was because git commit was
opening a COMMIT_EDITMSG in the wrong path.  To reproduce the problem,
you need a setup like mine:

  ~/dotfiles is a repository containing submodules
  ~/.elisp is a symbolic link to ~/dotfiles/.elisp, a normal directory
  ~/.elisp/flx is the submodule repository to which I'm trying to commit

The buffer-file-name of COMMIT_EDITMSG comes out as
/home/artagnon/.git/modules/flx/.git/COMMIT_EDITMSG, which is
completely wrong because ~ does not even contain a .git.

So, I started debugging the issue with this patch:

diff --git a/builtin/commit.c b/builtin/commit.c
index 003bd7d..38a7c77 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -678,6 +678,9 @@ static int prepare_to_commit(const char
hook_arg2 = ;
}

+   char *buf = get_current_dir_name();
+   die(DBG: %s | %s, buf, git_path(commit_editmsg));
+
s-fp = fopen(git_path(commit_editmsg), w);
if (s-fp == NULL)
die_errno(_(could not open '%s'), git_path(commit_editmsg));

On master, commit returns:

  fatal: DBG: /home/artagnon/.elisp/flx |
../../.git/modules/.elisp/flx/COMMIT_EDITMSG

When backported to v1.8.3.3, commit returns:

  fatal: DBG: /home/artagnon/.elisp/flx |
/home/artagnon/dotfiles/.git/modules/.elisp/flx/COMMIT_EDITMSG

I tried looking through the logs to see what has changed in
path.c/environment.c, but have come up with nothing so far.  I think
I'll have to resort to using a hammer like bisect now.

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


Re: [BUG] git_path() returns relative paths

2013-07-27 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 *scratches head*

Just took a much-needed shower and came back.  It was trivial to find.

  $ git log v1.8.3.4.. -- path.c

e02ca72 (path.c: refactor relative_path(), not only strip prefix,
2013-06-25) is the offender.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git_path() returns relative paths

2013-07-27 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 e02ca72 (path.c: refactor relative_path(), not only strip prefix,
 2013-06-25) is the offender.

The problem is the callsite in setup.c:setup_work_tree(). When
relative_path() is called with
/home/artagnon/dotfiles/.git/modules/.elisp/flx and
/home/artagnon/dotfiles/.elisp/flx as the first and second
arguments, it sets sb to ../../.git/modules/.elisp/flx.

Makes me wonder why setup_git_dir() doesn't just use git_dir; why does
it need a relative path at all?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git_path() returns relative paths

2013-07-27 Thread Ramkumar Ramachandra
Fredrik Gustafsson wrote:
 When I hear submodules

The only reason this bug has something to do with submodules is
because of the relative path in the gitfile (although I'm yet to
corner the exact issue).  Otherwise, it doesn't exercise any new code
in submodule.c/ git-submodule.sh.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git_path() returns relative paths

2013-07-27 Thread Ramkumar Ramachandra
Duy Nguyen wrote:
 044bbbc (Make git_dir a path relative to work_tree in
 setup_work_tree() - 2008-06-19)

Okay, so it does seem to be a significant optimization.  Frankly,
e02ca72 only improves the relative_path() algorithm, and it's not
really doing anything Wrong: it's has just uncovered a previously
undiscovered bug.

So far, we know that the line:

  s-fp = fopen(git_path(commit_editmsg), w);

in commit.c:prepare_to_commit() isn't failing; so, the work_tree and
git_dir do seem to be set correctly. The problem seems to be in
launch_editor().  What's worse? Everything works just fine when I have
a symbolic link to a directory in a normal repository; I still can't
figure out what submodules have to do with any of this.

What the hell is going on?!
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git_path() returns relative paths

2013-07-27 Thread Ramkumar Ramachandra
Duy Nguyen wrote:
 I was involved with this code (the gitdir setup code, not submodule)
 and am interested to know what's going on too. Could you produce a
 small script to reproduce it?

Here's your reduced testcase. Just point mygit to a HEAD build.

  #!/bin/sh

  mygit=~/src/git/git
  cd /tmp
  $mygit clone https://github.com/artagnon/clayoven
  cd clayoven
  $mygit submodule add https://github.com/lewang/flx .elisp/flx
  $mygit commit -a -m Added submodule
  cd /tmp
  ln -s clayoven/.elisp
  cd .elisp/flx
  EDITOR=emacs -Q git commit --amend
  # buffer-file-name = /tmp/.git/modules/.elisp/flx/COMMIT_EDITMSG

Note that this is emacs 24.3. I used -Q to make sure that none of my
init magic (magit etc.) was responsible for changing directories or
doing something equally stupid. However, considering that it's
impossible to reproduce the problem with either cat or vim as the
EDITOR, you might be inclined to classify this as an Emacs bug. In
that case, why can't I reproduce it without submodules?

I'm going off to eat cake before I tear my hair out in frustration.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git_path() returns relative paths

2013-07-27 Thread Ramkumar Ramachandra
Jiang Xin wrote:
 I change the EDITOR(GIT_EDITOR) environment in your test script
 as follows:

 GIT_EDITOR=echo PWD: $(pwd); echo REALPATH: $(pwd -P); echo \
 git commit --amend

See, what stumps be about this is the no-submodule case:

  #!/bin/sh

  mygit=~/src/git/git
  cd /tmp
  rm -rf clayoven lib
  $mygit clone https://github.com/artagnon/clayoven
  ln -s clayoven/lib
  cd lib/clayoven
  EDITOR=echo PWD: $(pwd); echo REALPATH: $(pwd -P); echo \
  git commit --amend
  # buffer-file-name = /tmp/.git/modules/.elisp/flx/COMMIT_EDITMSG

From the point of view of $EDITOR, how is this different?  Yet, when
you change EDITOR to emacs -Q, it works fine.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] git_path() returns relative paths

2013-07-28 Thread Ramkumar Ramachandra
Duy Nguyen wrote:
 I think instead of letting the kernel walk the path, emacs does it by
 itself.

If this were true, shouldn't we be able to reproduce the behavior with
my no-submodules symlink testcase?  How can it resolve symlinks in one
case, and not in the other case?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   3   4   5   6   7   8   9   10   >