security vulnerability disclosure procedure?

2014-04-20 Thread Richard Hansen
Hi all,

I have discovered a minor security vulnerability.  I could send the
patch to the mailing list, but I wanted someone else to take a look
first just to make sure it's minor enough that folks will think it's OK
to publicly announce.

Who should I send the patch to?

Thanks,
Richard
--
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


[SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1

2014-04-21 Thread Richard Hansen
Both bash and zsh subject the value of PS1 to parameter expansion,
command substitution, and arithmetic expansion.  Rather than include
the raw, unescaped branch name in PS1 when running in two- or
three-argument mode, construct PS1 to reference a variable that holds
the branch name.  Because the shells do not recursively expand, this
avoids arbitrary code execution by specially-crafted branch names such
as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
To see the vulnerability in action, follow the instructions at:
https://github.com/richardhansen/clonepwn

 contrib/completion/git-prompt.sh | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 7b732d2..bd7ff29 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -207,7 +207,18 @@ __git_ps1_show_upstream ()
p= u+${count#* }-${count%  *} ;;
esac
if [[ -n $count  -n $name ]]; then
-   p=$p $(git rev-parse --abbrev-ref $upstream 
2/dev/null)
+   __git_ps1_upstream_name=$(git rev-parse \
+   --abbrev-ref $upstream 2/dev/null)
+   if [ $pcmode = yes ]; then
+   # see the comments around the
+   # __git_ps1_branch_name variable below
+   p=$p \${__git_ps1_upstream_name}
+   else
+   p=$p ${__git_ps1_upstream_name}
+   # not needed anymore; keep user's
+   # environment clean
+   unset __git_ps1_upstream_name
+   fi
fi
fi
 
@@ -438,8 +449,27 @@ __git_ps1 ()
__git_ps1_colorize_gitstring
fi
 
+   b=${b##refs/heads/}
+   if [ $pcmode = yes ]; then
+   # In pcmode (and only pcmode) the contents of
+   # $gitstring are subject to expansion by the shell.
+   # Avoid putting the raw ref name in the prompt to
+   # protect the user from arbitrary code execution via
+   # specially crafted ref names (e.g., a ref named
+   # '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)' would execute
+   # 'sudo rm -rf /' when the prompt is drawn).  Instead,
+   # put the ref name in a new global variable (in the
+   # __git_ps1_* namespace to avoid colliding with the
+   # user's environment) and reference that variable from
+   # PS1.
+   __git_ps1_branch_name=$b
+   # note that the $ is escaped -- the variable will be
+   # expanded later (when it's time to draw the prompt)
+   b=\${__git_ps1_branch_name}
+   fi
+
local f=$w$i$s$u
-   local gitstring=$c${b##refs/heads/}${f:+$z$f}$r$p
+   local gitstring=$c$b${f:+$z$f}$r$p
 
if [ $pcmode = yes ]; then
if [ ${__git_printf_supports_v-} != yes ]; then
-- 
1.9.2

--
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: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1

2014-04-21 Thread Richard Hansen
On 2014-04-21 16:24, Jeff King wrote:
 On Mon, Apr 21, 2014 at 03:07:28PM -0400, Richard Hansen wrote:
 
 Both bash and zsh subject the value of PS1 to parameter expansion,
 command substitution, and arithmetic expansion.  Rather than include
 the raw, unescaped branch name in PS1 when running in two- or
 three-argument mode, construct PS1 to reference a variable that holds
 the branch name.  Because the shells do not recursively expand, this
 avoids arbitrary code execution by specially-crafted branch names such
 as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'.
 
 Cute. We already disallow quite a few characters in refnames (including
 space, as you probably discovered), and generally enforce that during
 ref transfer. I wonder if we should tighten that more as a precuation.
 It would be backwards-incompatible, but I wonder if things like $ and
 ; in refnames are actually useful to people.

That's a tough call.  I imagine those that legitimately use '$', ';', or
'`' would be annoyed but generally accepting given the security benefit.

I wonder how many repos at sites like GitHub use unusual punctuation in
ref names.

Perhaps the additional character restrictions could be controlled via a
config option.  It would default to the more secure mode but
developers/repo admins could relax it where required.

If imposing additional character restrictions is unpalatable, hooks
could be used to reject funny branch names in shared repos.  But this
would require administrator action -- it's not as secure by default.

 
 Did you look into similar exploits with completion? That's probably
 slightly less dire (this one hits you as soon as you cd into a
 malicious clone, whereas completion problems require you to actually hit
 tab). I'm fairly sure that we miss some quoting on pathnames, for
 example. That can lead to bogus completion, but I'm not sure offhand if
 it can lead to execution.

I have not looked at the completion code.

-Richard
--
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: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1

2014-04-21 Thread Richard Hansen
On 2014-04-21 18:33, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
 Richard Hansen rhan...@bbn.com writes:

 Both bash and zsh subject the value of PS1 to parameter expansion,
 command substitution, and arithmetic expansion.  Rather than include
 the raw, unescaped branch name in PS1 when running in two- or
 three-argument mode, construct PS1 to reference a variable that holds
 the branch name.  Because the shells do not recursively expand, this
 avoids arbitrary code execution by specially-crafted branch names such
 as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'.

 Signed-off-by: Richard Hansen rhan...@bbn.com

 I'd like to see this patch eyeballed by those who have been involved
 in the script (shortlog and blame tells me they are SZEDER and
 Simon, CC'ed), so that we can hopefully merge it by the time -rc1 is
 tagged.

 Will queue so that I won't lose it in the meantime.

 Thanks.
 
 Sadly, this does not seem to pass t9903.41 for me.
 
 $ bash t9903-*.sh -i -v

Oops!  Because git-prompt.sh is in contrib I didn't realize there was a
test for it.

The test will have to change.  I'll think about the best way to adjust
the test and send a reroll.

Thanks,
Richard
--
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


[SECURITY PATCH v2] git-prompt.sh: don't put unsanitized branch names in $PS1

2014-04-21 Thread Richard Hansen
Both bash and zsh subject the value of PS1 to parameter expansion,
command substitution, and arithmetic expansion.  Rather than include
the raw, unescaped branch name in PS1 when running in two- or
three-argument mode, construct PS1 to reference a variable that holds
the branch name.  Because the shells do not recursively expand, this
avoids arbitrary code execution by specially-crafted branch names such
as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
Changes since v1:  update t/t9903-bash-prompt.sh

 contrib/completion/git-prompt.sh | 34 +--
 t/t9903-bash-prompt.sh   | 44 
 2 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 7b732d2..bd7ff29 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -207,7 +207,18 @@ __git_ps1_show_upstream ()
p= u+${count#* }-${count%  *} ;;
esac
if [[ -n $count  -n $name ]]; then
-   p=$p $(git rev-parse --abbrev-ref $upstream 
2/dev/null)
+   __git_ps1_upstream_name=$(git rev-parse \
+   --abbrev-ref $upstream 2/dev/null)
+   if [ $pcmode = yes ]; then
+   # see the comments around the
+   # __git_ps1_branch_name variable below
+   p=$p \${__git_ps1_upstream_name}
+   else
+   p=$p ${__git_ps1_upstream_name}
+   # not needed anymore; keep user's
+   # environment clean
+   unset __git_ps1_upstream_name
+   fi
fi
fi
 
@@ -438,8 +449,27 @@ __git_ps1 ()
__git_ps1_colorize_gitstring
fi
 
+   b=${b##refs/heads/}
+   if [ $pcmode = yes ]; then
+   # In pcmode (and only pcmode) the contents of
+   # $gitstring are subject to expansion by the shell.
+   # Avoid putting the raw ref name in the prompt to
+   # protect the user from arbitrary code execution via
+   # specially crafted ref names (e.g., a ref named
+   # '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)' would execute
+   # 'sudo rm -rf /' when the prompt is drawn).  Instead,
+   # put the ref name in a new global variable (in the
+   # __git_ps1_* namespace to avoid colliding with the
+   # user's environment) and reference that variable from
+   # PS1.
+   __git_ps1_branch_name=$b
+   # note that the $ is escaped -- the variable will be
+   # expanded later (when it's time to draw the prompt)
+   b=\${__git_ps1_branch_name}
+   fi
+
local f=$w$i$s$u
-   local gitstring=$c${b##refs/heads/}${f:+$z$f}$r$p
+   local gitstring=$c$b${f:+$z$f}$r$p
 
if [ $pcmode = yes ]; then
if [ ${__git_printf_supports_v-} != yes ]; then
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 59f875e..6efd0d9 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -452,53 +452,53 @@ test_expect_success 'prompt - format string starting with 
dash' '
 '
 
 test_expect_success 'prompt - pc mode' '
-   printf BEFORE: (master):AFTER expected 
+   printf BEFORE: (\${__git_ps1_branch_name}):AFTER\\nmaster expected 
printf  expected_output 
(
__git_ps1 BEFORE: :AFTER $actual 
test_cmp expected_output $actual 
-   printf %s $PS1 $actual
+   printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual
) 
test_cmp expected $actual
 '
 
 test_expect_success 'prompt - bash color pc mode - branch name' '
-   printf BEFORE: (${c_green}master${c_clear}):AFTER expected 
+   printf BEFORE: 
(${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmaster expected 
(
GIT_PS1_SHOWCOLORHINTS=y 
__git_ps1 BEFORE: :AFTER $actual
-   printf %s $PS1 $actual
+   printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual
) 
test_cmp expected $actual
 '
 
 test_expect_success 'prompt - bash color pc mode - detached head' '
-   printf BEFORE: (${c_red}(%s...)${c_clear}):AFTER $(git log -1 
--format=%h b1^) expected 
+   printf BEFORE: 
(${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...) $(git log -1 
--format=%h b1^) expected 
git checkout b1^ 
test_when_finished git checkout master 
(
GIT_PS1_SHOWCOLORHINTS=y 
__git_ps1 BEFORE: :AFTER 
-   printf %s $PS1 $actual
+   printf %s\\n%s $PS1 ${__git_ps1_branch_name

Re: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1

2014-04-22 Thread Richard Hansen
On 2014-04-22 13:38, Junio C Hamano wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:
 
 While we're at it, I think it would be prudent to ban '-' at the
 beginning of reference name segments.  For example, reference names like

 refs/heads/--cmd=/sbin/halt
 refs/tags/--exec=forkbomb(){forkbomb|forkbomb};forkbomb

 are currently both legal, but I think they shouldn't be.
 
 I think we forbid these at the Porcelain level (git branch, git
 checkout -b and git tag should not let you create -aBranch),
 while leaving the plumbing lax to allow people experimenting with
 their repositories.
 
 It may be sensible to discuss and agree on what exactly should be
 forbidden (we saw leading dash, semicolon and dollar anywhere
 so far in the discussion)

Also backquote anywhere.

 and plan for transition to forbid them
 everywhere in a next big version bump (it is too late for 2.0).

Would it be acceptable to have a config option to forbid these in a
non-major version bump?  Does parsing config files add too much overhead
for this to be feasible?

If it's OK to have a config option, then here's one possible transition
path (probably flawed, but my intent is to bootstrap discussion):

  1. Add an option to forbid dangerous characters.  The option defaults
 to disabled for compatibility.  If the option is unset, print a
 warning upon encountering a ref name that would be forbidden.
  2. Later, flip the default to enabled.
  3. Later, in the weeks/months leading up to the next major version
 release, print the warning even if the config option is set to
 disabled.

Thanks,
Richard
--
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: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1

2014-04-25 Thread Richard Hansen
On 2014-04-25 03:37, Simon Oosthoek wrote:
 (though tbh, I think you'd have to be in an automated situation
 to check out a branch that is basically a command to hack your
 system, a human would probably figure it too cumbersome, or too
 fishy)

You can get in trouble by cloning a malicious repository and cding to
the resulting directory.  See:

https://github.com/richardhansen/clonepwn

for a (benign) demonstration.  (Note the name of the default branch in
that repository -- it's not master.)

 
 + # not needed anymore; keep user's
 + # environment clean
 + unset __git_ps1_upstream_name
 
 We already have a lot of stuff in the user's environment beginning
 with __git, so I don't think the unset is necessary.
 
 If people rely on the string being set in their scripts, it can be
 bad to remove it. But if it's new in this patch,

The variable is new.

 I don't see the need to keep it. Cruft is bad IMO.

Agreed, although I am willing to remove those three lines if that is the
collective preference.

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


Re: [PATCH v5 1/6] pull: rename pull.rename to pull.mode

2014-04-30 Thread Richard Hansen
On 2014-04-29 07:17, Felipe Contreras wrote:
 Also 'branch.name.rebase' to 'branch.name.pullmode'.
 
 This way 'pull.mode' can be set to 'merge', and the default can be
 something else.
 
 The old configurations still work, but get deprecated.

Should users be warned if both pull.rebase and pull.mode are set?  (Also
if both branch.name.rebase and branch.name.pullmode are set.)

Hmm, thinking about it more, I think I prefer it to *not* warn if both
are set (as is currently coded).  If the user set *mode then presumably
they've read the documentation and know that *mode is the right way to
configure the behavior, so a warning would only serve to badger them
into cleaning up their config file.  And it would annoy users that keep
their ~/.gitconfig in a dotfiles repo shared between machines with
different versions of Git.

I prefer a deprecation note in the documentation as Philip suggests, but
I'm OK without one too.

-Richard


 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/config.txt   | 34 +-
  Documentation/git-pull.txt |  2 +-
  branch.c   |  4 ++--
  builtin/remote.c   | 14 --
  git-pull.sh| 39 +--
  t/t3200-branch.sh  | 40 
  t/t5601-clone.sh   |  4 ++--
  7 files changed, 91 insertions(+), 46 deletions(-)
 
 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index c26a7c8..5978d35 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -708,7 +708,7 @@ branch.autosetupmerge::
  branch.autosetuprebase::
   When a new branch is created with 'git branch' or 'git checkout'
   that tracks another branch, this variable tells Git to set
 - up pull to rebase instead of merge (see branch.name.rebase).
 + up pull to rebase instead of merge (see branch.name.pullmode).
   When `never`, rebase is never automatically set to true.
   When `local`, rebase is set to true for tracked branches of
   other local branches.
 @@ -764,15 +764,15 @@ branch.name.mergeoptions::
   option values containing whitespace characters are currently not
   supported.
  
 -branch.name.rebase::
 - When true, rebase the branch name on top of the fetched branch,
 - instead of merging the default branch from the default remote when
 - git pull is run. See pull.rebase for doing this in a non
 - branch-specific manner.
 +branch.name.pullmode::
 + When git pull is run, this determines if it would either merge or
 + rebase the fetched branch. The possible values are 'merge',
 + 'rebase', and 'rebase-preserve'. See pull.mode for doing this in a
 + non branch-specific manner.
  +
 - When preserve, also pass `--preserve-merges` along to 'git rebase'
 - so that locally committed merge commits will not be flattened
 - by running 'git pull'.
 + When 'rebase-preserve', also pass `--preserve-merges` along to
 + 'git rebase' so that locally committed merge commits will not be
 + flattened by running 'git pull'.
  +
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]
 @@ -1881,15 +1881,15 @@ pretty.name::
   Note that an alias with the same name as a built-in format
   will be silently ignored.
  
 -pull.rebase::
 - When true, rebase branches on top of the fetched branch, instead
 - of merging the default branch from the default remote when git
 - pull is run. See branch.name.rebase for setting this on a
 - per-branch basis.
 +pull.mode::
 + When git pull is run, this determines if it would either merge or
 + rebase the fetched branch. The possible values are 'merge',
 + 'rebase', and 'rebase-preserve'. See branch.name.pullmode for doing
 + this in a non branch-specific manner.
  +
 - When preserve, also pass `--preserve-merges` along to 'git rebase'
 - so that locally committed merge commits will not be flattened
 - by running 'git pull'.
 + When 'rebase-preserve', also pass `--preserve-merges` along to
 + 'git rebase' so that locally committed merge commits will not be
 + flattened by running 'git pull'.
  +
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]
 diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
 index 200eb22..9a91b9f 100644
 --- a/Documentation/git-pull.txt
 +++ b/Documentation/git-pull.txt
 @@ -117,7 +117,7 @@ locally created merge commits will not be flattened.
  +
  When false, merge the current branch into the upstream branch.
  +
 -See `pull.rebase`, `branch.name.rebase` and `branch.autosetuprebase` in
 +See `pull.mode`, `branch.name.pullmode` and `branch.autosetuprebase` in
  linkgit:git-config[1] if you want to make `git pull` always use
  `--rebase` 

Re: [PATCH 00/32] Split index mode for very large indexes

2014-04-30 Thread Richard Hansen
On 2014-04-28 06:55, Nguyễn Thái Ngọc Duy wrote:
 From the user point of view, this reduces the writable size of index
 down to the number of updated files. For example my webkit index v4 is
 14MB. With a fresh split, I only have to update an index of 200KB.
 Every file I touch will add about 80 bytes to that. As long as I don't
 touch every single tracked file in my worktree, I should not pay
 penalty for writing 14MB index file on every operation.

I played around with these changes a bit and have some questions:

  * These changes should only affect performance when the index is
updated, right?  In other words, if I do git status; git status
the second git status shouldn't update the index and therefore
shouldn't have a noticeable performance improvement relative to Git
without these patches.  Right?

  * Do you have any before/after benchmark results you can share?

  * Are there any benchmark scripts I can use to test it out in my own
repositories?

  * Is there a debug utility I can use to examine the contents of the
index and sharedindex.* files in a more human-readable way?

I'm asking because in my (very basic) tests I noticed that with the
following command:

git status; time git status

the second git status had an unexpected ~20% performance improvement
in my repo relative to a build without your patches.  The second git
status in the following command also had about a ~20% performance
improvement:

git status; touch file-in-index; time git status

So it seems like the patches did improve performance somewhat, but in
ways I wasn't expecting.  (I'm not entirely certain my benchmark method
is sound.)

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


Re: [PATCH v6 1/7] pull: rename pull.rebase to pull.mode

2014-05-02 Thread Richard Hansen
On 2014-05-01 20:00, Felipe Contreras wrote:
 Also 'branch.name.rebase' to 'branch.name.pullmode'.
 
 This way we can add more modes and the default can be something else,
 namely it can be set to merge-ff-only, so eventually we can reject
 non-fast-forward merges by default.
 
 The old configurations still work, but get deprecated.

s/get/are/

 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/config.txt   | 39 ++-
  Documentation/git-pull.txt |  2 +-
  branch.c   |  4 ++--
  builtin/remote.c   | 14 --
  git-pull.sh| 31 +--
  t/t3200-branch.sh  | 40 
  t/t5601-clone.sh   |  4 ++--
  7 files changed, 88 insertions(+), 46 deletions(-)
 
 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index c26a7c8..c028aeb 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -708,7 +708,7 @@ branch.autosetupmerge::
  branch.autosetuprebase::
   When a new branch is created with 'git branch' or 'git checkout'
   that tracks another branch, this variable tells Git to set
 - up pull to rebase instead of merge (see branch.name.rebase).
 + up pull to rebase instead of merge (see branch.name.pullmode).
   When `never`, rebase is never automatically set to true.
   When `local`, rebase is set to true for tracked branches of
   other local branches.

Should branch.autosetuprebase be replaced with a new
branch.autosetupmode setting?

 @@ -764,15 +764,17 @@ branch.name.mergeoptions::
   option values containing whitespace characters are currently not
   supported.
  
 -branch.name.rebase::
 - When true, rebase the branch name on top of the fetched branch,
 - instead of merging the default branch from the default remote when
 - git pull is run. See pull.rebase for doing this in a non
 - branch-specific manner.
 +branch.name.pullmode::
 + When git pull is run, this determines if it would either merge or
 + rebase the fetched branch.

To me this sentence implies that 'rebase' would rebase the fetched
branch onto HEAD, when it's actually the other way around.

  The possible values are 'merge',
 + 'rebase', and 'rebase-preserve'.

While the name 'merge' is mostly self-explanatory, I think it needs
further clarification:  Does 'merge' imply --no-ff?  Or --ff?  Or the
value of merge.ff?  Which side will be the first parent?

Similarly, 'rebase' could use some clarification:
  * the local branch is rebased onto the pulled branch, not the other
way around
  * it doesn't simply do 'git rebase FETCH_HEAD' -- it also walks the
reflog of the upstream ref until it finds an ancestor of the local
branch

See pull.mode for doing this in a
 + non branch-specific manner.

I find this sentence to be a bit unclear and would prefer something
like:  Defaults to the value of pull.mode.

  +
 - When preserve, also pass `--preserve-merges` along to 'git rebase'
 - so that locally committed merge commits will not be flattened
 - by running 'git pull'.
 + When 'rebase-preserve', also pass `--preserve-merges` along to
 + 'git rebase' so that locally committed merge commits will not be
 + flattened by running 'git pull'.
 ++
 + It was named 'branch.name.rebase' but that is deprecated now.

To me this sentence implies that .rebase was simply renamed to .pullmode
with no other changes.  I'd prefer something like this:

branch.name.rebase::
Deprecated in favor of branch.name.pullmode.

(Same goes for pull.rebase.)

  +
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]
 @@ -1881,15 +1883,18 @@ pretty.name::
   Note that an alias with the same name as a built-in format
   will be silently ignored.
  
 -pull.rebase::
 - When true, rebase branches on top of the fetched branch, instead
 - of merging the default branch from the default remote when git
 - pull is run. See branch.name.rebase for setting this on a
 - per-branch basis.
 +pull.mode::
 + When git pull is run, this determines if it would either merge or
 + rebase the fetched branch. The possible values are 'merge',
 + 'rebase', and 'rebase-preserve'. See branch.name.pullmode for doing
 + this in a non branch-specific manner.
 ++
 + When 'rebase-preserve', also pass `--preserve-merges` along to
 + 'git rebase' so that locally committed merge commits will not be
 + flattened by running 'git pull'.
 ++
  +
 - When preserve, also pass `--preserve-merges` along to 'git rebase'
 - so that locally committed merge commits will not be flattened
 - by running 'git pull'.

The default value should be documented.  Also, rather than copy+paste
the 

Re: [PATCH v6 1/7] pull: rename pull.rebase to pull.mode

2014-05-02 Thread Richard Hansen
On 2014-05-02 17:12, Felipe Contreras wrote:
 Richard Hansen wrote:
 Should branch.autosetuprebase be replaced with a new
 branch.autosetupmode setting?
 
 Maybe. But if so, I think that should be done in another series.
 Otherwise we'll never have a chance to change anything.

Sure.

  The possible values are 'merge',
 +   'rebase', and 'rebase-preserve'.

 While the name 'merge' is mostly self-explanatory, I think it needs
 further clarification:  Does 'merge' imply --no-ff?  Or --ff?  Or the
 value of merge.ff?
 
 'pull.mode=merge' will do the same as `git merge`, I don't see where or
 how it can be explained more clearly.

How about:

branch.name.pullmode::
Determines how 'git pull' integrates the fetched branch into
branch name.
Defaults to the value of `pull.mode`.
Supported values:
+
--
`merge`:::
Merge the fetched branch into name.
See also `merge.ff`.
`rebase`:::
Find the point at which name forked from the fetched
branch (see the `--fork-point` option of
linkgit:git-merge-base[1]), then rebase the commits
between the fork point and branch name onto the
fetched branch.
`rebase-preserve`:::
Like `rebase` but passes `--preserve-merges` to 'git
rebase'.
--
+
*NOTE*: `rebase` and `rebase-preserve` are potentially dangerous; do
*not* use them unless you understand the implications (see
linkgit:git-rebase[1] for details).

pull.mode::
See `branch.name.pullmode`.  Defaults to `merge`.

 
 Which side will be the first parent?
 
 The same as things currently are: the pulled branch into the current
 branch (current branch is first parent).

This was a rhetorical question -- I was trying to point out that the
current behavior should be documented.

 
 We should probably change this, but that's out of scope of this series,
 and hasn't been decided yet.

Agreed.  If this series is merged, a future series could add a
'merge-there' pull mode.

 Also, rather than copy+paste
 the description from branch.name.pullmode, I'd prefer a brief
 reference.  For example:

 pull.mode::
 See branch.name.pullmode.  Defaults to 'merge'.
 
 I'd say pull.mode is the important one.

Either way works for me.  How about this:

branch.name.pullmode::
Overrides the value of `pull.mode` for branch name.

pull.mode::
Determines how 'git pull' integrates the fetched branch into
the current branch.
Supported values:
+
--
`merge`:::
(default)
Merge the fetched branch into the current branch.
See also `merge.ff`.
`rebase`:::
Find the point at which the current branch forked from
the fetched branch (see the `--fork-point` option of
linkgit:git-merge-base[1]), then rebase the commits
between the fork point and the current branch onto the
fetched branch.
`rebase-preserve`:::
Like `rebase` but passes `--preserve-merges` to 'git
rebase'.
--
+
*NOTE*: `rebase` and `rebase-preserve` are potentially dangerous; do
*not* use them unless you understand the implications (see
linkgit:git-rebase[1] for details).

-Richard
--
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: Pull is Mostly Evil

2014-05-03 Thread Richard Hansen
On 2014-05-02 14:13, Junio C Hamano wrote:
 Stepping back even further, and thinking what is different between
 these two pulls, we notice that the first one is pulling from the
 place we push back to.

I think the fundamental difference is in the relationship between the
local and the remote branch (which branch derives from the other).
The relationship between the branches determines what the user wants
from 'git pull'.

In my experience 'git pull' is mostly (only?) used for the following
three tasks:

 1. update a local branch to incorporate the latest upstream changes

In this case, the local branch (master) is a derivative of the
upstream branch (origin/master).  The user wants all of the
commits in the remote branch to be in the local branch.  And the
user would like the local changes, if any, to descend from the tip
of the remote branch.

For this case, 'git pull --ff-only' followed by 'git rebase -p'
works well, as does 'git pull --rebase=preserve' if the user is
comfortable rebasing without reviewing the incoming commits first.
A plain 'git pull' or 'git pull --ff' is suboptimal due to the
awkward backwards-parents merge commit.

 2. update a published feature branch with the latest changes from its
parent branch

In this case, the local branch (foo) is a derivative of the
upstream branch (origin/foo) which is itself a derivative of
another branch (origin/master).  All commits in origin/master
should be in origin/foo, and ideally all commits unique to
origin/foo would descend from the tip of origin/master.

The relationship between origin/foo and origin/master is similar
to the relationship between master and origin/master in case #1
above, but rebase is frowned upon because the feature branch has
been shared with other developers (and the shared repository might
reject non-ff updates).

This case is sort-of like case #1 above (updating) and sort-of
like case #3 below (integrating).

For this case, after the local branch foo is updated (case #1
above), 'git pull --ff origin master' or 'git fetch --all  git
merge --ff origin/master' work well to update origin/foo.

 3. integrate a more-or-less complete feature/fix back into the line
of development it forked off of

In this case the local branch is a primary line of development and
the remote branch contains the derivative work.  Think Linus
pulling in contributions.  Different situations will call for
different ways to handle this case, but most will probably want
some or all of:

 * rebase the remote commits onto local HEAD
 * merge into local HEAD so that the first parent (if a real merge
   and not a ff) is the previous version of the main line of
   development and the second parent is the derivative work
 * merge --no-ff so that:
- the merge can serve as a cover letter (who reviewed it,
  which bug reports were fixed, where the changes came from,
  etc.)
- the commits that compose the new topic are grouped together
- the first-parent path represents a series of completed tasks

(I prefer to do all three, although I may skip the rebase if the
commits came from another public repository so as to not annoy
users of that downstream repository.)

For this case, 'git pull --no-ff' is better than 'git pull --ff'
(for the reasons listed above), but perhaps something more
elaborate would be ideal (e.g., rebase there onto here, then merge
--no-ff).

These three usage patterns are at odds; it's hard to change the
default behavior of 'git pull' to favor one usage case without harming
another.  Perhaps this is why there's so much disagreement about what
'git pull' should do.

I see a few ways to improve the situation:

  1. Add one or two new commands to split up how the three cases are
 handled.  For example:

  * Add a new 'git update' command that is friendly for case
#1.  Update tutorials to recommend 'git update' instead of
'git pull'.  It would behave like 'git pull --ff-only' by
default.

It could behave like 'git pull --rebase[=preserve]' instead,
but this has a few downsides:
 - It doesn't give the user an opportunity to review the
   incoming commits before rebasing (e.g., to see what sort of
   conflicts to expect).
 - It subjects new users to that scary rebase thing before
   they are prepared to handle it.
 - The branch to be updated must be checked out.  If 'git
   update' used --ff-only, then 'git update --all' could
   fast-forward all local branches to their configured
   upstreams when possible.  (How cool would that be?)

  * Leave 'git pull' and 'git pull $remote [$refspec]' alone --
the current defaults are acceptable (though maybe not ideal)
for cases #2 and #3.

 Another example:

Re: Pull is Mostly Evil

2014-05-03 Thread Richard Hansen
On 2014-05-03 05:26, Felipe Contreras wrote:
 Richard Hansen wrote:
 
 I think the fundamental difference is in the relationship between the
 local and the remote branch (which branch derives from the other).
 The relationship between the branches determines what the user wants
 from 'git pull'.

 In my experience 'git pull' is mostly (only?) used for the following
 three tasks:
 
 I agree.
 
  1. update a local branch to incorporate the latest upstream changes

 In this case, the local branch (master) is a derivative of the
 upstream branch (origin/master).  The user wants all of the
 commits in the remote branch to be in the local branch.  And the
 user would like the local changes, if any, to descend from the tip
 of the remote branch.
 
 My current propsal of making `git pull` by default do --ff-only would
 solve this.

It would go a long way toward improving the situation, yes.

 In addition I think by default 'master' should be merged to
 'origin/master', if say --merge is given.

This would break cases #2 and #3.  (With cases #2 and #3 you want the
fetched branch to be the second parent, not the first.)

Or are you proposing that pull --merge should reverse the parents if and
only if the remote ref is @{u}?

 
 For this case, 'git pull --ff-only' followed by 'git rebase -p'
 works well, as does 'git pull --rebase=preserve' if the user is
 comfortable rebasing without reviewing the incoming commits first.
 
 I suppose you mean a `git rebase -p` if the `git pull --ff-only` failed.

Yes.

 This might be OK on most projects, but not all.

The rebase only affects the local repository (the commits haven't been
pushed yet or else they'd be in @{u} already), so I'd say it's more of
an individual developer decision than a project decision.

In my opinion rebase would be the best option here, but if the project
is OK with developers pushing merge or merge-there commits and the
developer isn't yet comfortable with rebasing, then merge is also an
acceptable option.

 
 What happens after a `git pull --ff-only` fails should be totally
 up to the user.

I tend to agree, mostly because I want users to have an opportunity to
review incoming commits before action is taken.  Also, though rebasing
would yield the nicest history, some users aren't yet comfortable with
rebase.  If a project is OK with silly little merge commits from users
that aren't comfortable with rebase, then I don't want to force everyone
to rebase by default.

As an added bonus:  Defaulting to --ff-only makes it possible for 'git
pull --all' to fast-forward every local branch to their configured
upstream, not just the currently checked-out branch.  I think this would
be a huge usability win.

 
  2. update a published feature branch with the latest changes from its
 parent branch

 In this case, the local branch (foo) is a derivative of the
 upstream branch (origin/foo) which is itself a derivative of
 another branch (origin/master).  All commits in origin/master
 should be in origin/foo, and ideally all commits unique to
 origin/foo would descend from the tip of origin/master.
 
 I don't understand why are you tainting the example with 'origin/foo',

Originally I didn't have this case in my list, but I added it after
thinking about Peff's comment:

  On 2014-05-02 17:48, Jeff King wrote:
   One common workflow for GitHub users is to back-merge master into a
   topic, because they want the final integrated version on the topic
   branch.

This almost but doesn't quite fit neatly into the other two cases.  It's
not case #1 because the shared nature of origin/foo means that rebasing
origin/foo onto origin/master is usually bad instead of usually good.
It's not case #3 because rebasing origin/master commits onto origin/foo
(assuming that the user would usually want to rebase the topic branch
when integrating) would definitely be bad.

 'foo' and 'origin/master' are enough for this example. In fact, the
 mention of 'origin/master' made it wrong: after the pull not all the
 commits of origin/master would be in origin/foo, you need a push for
 that.

The push of foo to origin/foo was meant to be implied.

 We have enough in our plate to taint this with yet another branch
 and push.
 
 For this case `git pull origin master` already work correctly for most
 projects.

Yes, it does.

 We probably shouldn't change that.

If we change 'git pull' to default to --ff-only but let 'git pull
$remote [$refspec]' continue to default to --ff then we have two
different behaviors depending on how 'git pull' is invoked.  I'm worried
that this would trip up users.  I'm not convinced that having two
different behaviors would be bad, but I'm not convinced that it would be
good either.

 
  3. integrate a more-or-less complete feature/fix back into the line
 of development it forked off of

 In this case the local branch is a primary line of development and
 the remote branch contains the derivative work

Re: Pull is Mostly Evil

2014-05-04 Thread Richard Hansen
On 2014-05-03 23:08, Felipe Contreras wrote:
 Richard Hansen wrote:
 Or are you proposing that pull --merge should reverse the parents if and
 only if the remote ref is @{u}?
 
 Only if no remote or branch are specified `git pull --merge`.

OK.  Let me summarize to make sure I understand your full proposal:

  1. if plain 'git pull', default to --ff-only
  2. if 'git pull --merge', default to --ff.  If the local branch can't
 be fast-forwarded to the upstream branch, then create a merge
 commit where the local branch is the *second* parent, not the first
  3. if 'git pull $remote [$refspec]', default to --merge --ff.  If the
 local branch can't be fast-forwarded to the remote branch, then
 create a merge commit where the remote branch is the second parent
 (the current behavior)

Is that accurate?

 If we change 'git pull' to default to --ff-only but let 'git pull
 $remote [$refspec]' continue to default to --ff then we have two
 different behaviors depending on how 'git pull' is invoked.  I'm worried
 that this would trip up users.  I'm not convinced that having two
 different behaviors would be bad, but I'm not convinced that it would be
 good either.
 
 It is the only solution that has been proposed.

It's not the only proposal -- I proposed a few alternatives in my
earlier email (though not in the form of code), and others have too.  In
particular:

  * create a new 'git integrate' command/alias that behaves like 'git
pull --no-ff'
  * change 'git pull' and 'git pull $remote [$refspec]' to do --ff-only
by default

Another option that I just thought of:  Instead of your proposed
pull.mode and branch.name.pullmode, add the following two sets of configs:

  * pull.updateMode, branch.name.pullUpdateMode:

The default mode to use when running 'git pull' without naming a
remote repository or when the named remote branch is @{u}.  Valid
options: ff-only (default), merge-ff, merge-ff-there, merge-no-ff,
merge-no-ff-there, rebase, rebase-here, rebase-here-then-merge-no-ff

  * pull.integrateMode, branch.name.pullIntegrateMode:

The default mode to use when running 'git pull $remote [$refspec]'
when '$remote [$refspec]' is not @{u}.  Valid options are the same
as those for pull.updateMode.  Default is merge-ff.

This gives the default split behavior as you propose, but the user can
reconfigure to suit personal preference (and we can easily change the
default for one or the other if there's too much outcry).

 
 Moreover, while it's a bit worrisome, it wouldn't create any actual
 problems. Since `git pull $what` remains the same, there's no problems
 there. The only change would be on `git pull`.
 
 Since most users are not going to do `git pull $what` therefore it would
 only be a small subset of users that would notice the discrepancy
 between running with $what, or not. And the only discrepancy they could
 notice is that when they run `git pull $what` they expect it to be
 --ff-only, or when the run `git pull` they don't. Only the former could
 be an issue, but even then, it's highly unlikely that `git pull $what`
 would ever be a fast-forward.
 
 So althought conceptually it doesn't look clean, in reality there
 wouldn't be any problems.

Yes, it might not be a problem, but I'm still nervous.  I'd need more
input (e.g., user survey, broad mailing list consensus, long beta test
period, decree by a benevolent dictator) before I'd be comfortable with it.

 
  3. integrate a more-or-less complete feature/fix back into the line
 of development it forked off of

 In this case the local branch is a primary line of development and
 the remote branch contains the derivative work.  Think Linus
 pulling in contributions.  Different situations will call for
 different ways to handle this case, but most will probably want
 some or all of:

  * rebase the remote commits onto local HEAD

 No. Most people will merge the remote branch as it is. There's no reason
 to rebase, specially if you are creating a merge commit.

 I disagree.  I prefer to rebase a topic branch before merging (no-ff) to
 the main line of development for a couple of reasons:
 
 Well that is *your* preference. Most people would prefer to preserve the
 history.

Probably.  My point is that the behavior should be configurable, and I'd
like that particular behavior to be one of the options (but not the
default -- that wouldn't be appropriate).

 
   * It makes commits easier to review.
 
 The review in the vast majority of cases happens *before* the
 integration.

True, although even when review happens before integration there is
value in making code archeology easier.

 
 And the problem comes when the integrator makes a mistake, which they
 inevitable do (we all do), then there's no history about how the
 conflict was resolved, and what whas the original patch.

Good point, although if I was the integrator and there was a
particularly hairy conflict I'd still rebase

Re: Pull is Mostly Evil

2014-05-04 Thread Richard Hansen
On 2014-05-04 06:17, Felipe Contreras wrote:
 Richard Hansen wrote:
 On 2014-05-03 23:08, Felipe Contreras wrote:
 It is the only solution that has been proposed.

 It's not the only proposal -- I proposed a few alternatives in my
 earlier email (though not in the form of code), and others have too.  In
 particular:

   * create a new 'git integrate' command/alias that behaves like 'git
 pull --no-ff'
 
 Yeah but that's for a different issue altogheter. I doesn't solve the
 problems in 1. nor 2. nor 3.

'git integrate' would handle usage cases #2 (update a published branch
to its parent branch) and #3 (integrate a completed task into the main
line of development), making it feasible to change 'git pull' and 'git
pull $remote [$refspec]' to default to --ff-only to handle usage case #1
(update local branch to @{u}).

 
   * change 'git pull' and 'git pull $remote [$refspec]' to do --ff-only
 by default

 Another option that I just thought of:  Instead of your proposed
 pull.mode and branch.name.pullmode, add the following two sets of configs:

   * pull.updateMode, branch.name.pullUpdateMode:

 The default mode to use when running 'git pull' without naming a
 remote repository or when the named remote branch is @{u}.  Valid
 options: ff-only (default), merge-ff, merge-ff-there, merge-no-ff,
 merge-no-ff-there, rebase, rebase-here, rebase-here-then-merge-no-ff
 
 Those are way too many options to be able to sensibly explain them.

Certainly this is too many options for a first patch series, but I don't
think they're unexplainable.  (I listed a bunch of options because I was
trying to envision where this might take us in the long run.)

For the first patch series, I'd expect:  merge (which uses the merge.ff
option to determine whether to ff, ff-only, or no-ff), rebase, and ff-only.

Later ff-only would be made the default.

Later some or all of the other options would be added depending on user
interest.

 
   * pull.integrateMode, branch.name.pullIntegrateMode:

 The default mode to use when running 'git pull $remote [$refspec]'
 when '$remote [$refspec]' is not @{u}.  Valid options are the same
 as those for pull.updateMode.  Default is merge-ff.

 This gives the default split behavior as you propose, but the user can
 reconfigure to suit personal preference (and we can easily change the
 default for one or the other if there's too much outcry).
 
 If we reduce the number of options to begin with (more can be added
 later),

yup

 then it might make sense to have these two options.
 
 However, that doesn't change the proposal you described above (1. 2.
 3.).

Not sure what you mean.  I oulined three usage cases:
  #1 update local branch to @{u}
  #2 update a published branch to its parent branch
  #3 integrate a completed task into the main line of development

Having these two sets of options (updateMode and integrateMode) would
make it possible to configure plain 'git pull' to handle usage case #1
and 'git pull $remote [$refspec]' to handle usage cases #2 and #3.

Or the user could configure 'git pull' and 'git pull $remote [$refspec]'
to behave the same, in case they find the different behaviors to be too
confusing.

 There's something we can do, and let me clarify my proposal. What you
 described above is what I think should happen eventually, however, we
 can start by doing something like what my patch series is doing; issue a
 warning that the merge is not fast-forward and things might change in
 the future.

OK, let me rephrase to make sure I understand:

  1. leave the default behavior as-is for now (merge with local
 branch the first parent)
  2. add --merge argument
  3. add ff-only setting
  4. plan to eventually change the plain 'git pull' default to ff-only,
 but don't change the default yet
  5. add a warning if the plain 'git pull' is a non-ff
  6. wait and see how users react.  If they're OK with it, switch the
 default of the plain 'git pull' to ff-only.

Is that accurate?  If so, sounds OK to me.

 
 If people find this behavior confusing they will complain in the mailing
 list.

true

 Although I suspect it would be for other reasons, not the 'git
 pull'/'git pull $there' division.

probably

 Either way we would see in the discussion.

sounds good to me

 
  3. integrate a more-or-less complete feature/fix back into the line
 of development it forked off of

 In this case the local branch is a primary line of development and
 the remote branch contains the derivative work.  Think Linus
 pulling in contributions.  Different situations will call for
 different ways to handle this case, but most will probably want
 some or all of:

  * rebase the remote commits onto local HEAD

 No. Most people will merge the remote branch as it is. There's no reason
 to rebase, specially if you are creating a merge commit.

 I disagree.  I prefer to rebase a topic branch before merging (no-ff) to
 the main line of development

Re: Pull is Mostly Evil

2014-05-04 Thread Richard Hansen
On 2014-05-04 17:13, Felipe Contreras wrote:
 Richard Hansen wrote:
 On 2014-05-04 06:17, Felipe Contreras wrote:
 Richard Hansen wrote:
 On 2014-05-03 23:08, Felipe Contreras wrote:
 It is the only solution that has been proposed.

 It's not the only proposal -- I proposed a few alternatives in my
 earlier email (though not in the form of code), and others have too.  In
 particular:

   * create a new 'git integrate' command/alias that behaves like 'git
 pull --no-ff'

 Yeah but that's for a different issue altogheter. I doesn't solve the
 problems in 1. nor 2. nor 3.

 'git integrate' would handle usage cases #2 (update a published branch
 to its parent branch) and #3 (integrate a completed task into the main
 line of development),
 
 But these cases are completely different. One should reverse the
 parents, the other one not.

No -- for both #2 and #3 I want the remote branch to be merged into the
local branch.

In the example I gave for use case #2, foo is a local branch with
origin/foo as the configured upstream and origin/foo was forked off of
origin/master.  Someone pushed new stuff to origin/master, and the user
wants the new stuff to also be in origin/foo.  So the user does this:

  git checkout foo
  git pull --ff-only  # this is use case #1
  git pull origin master  # this is use case #2
  git push

The merge commit created by 'git pull origin master' should have
origin/master as the second parent, not the first.

-Richard
--
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: Pull is Mostly Evil

2014-05-05 Thread Richard Hansen
On 2014-05-03 06:00, John Szakmeister wrote:
 FWIW, at my company, we took another approach.  We introduced a `git
 ffwd` command that fetches from all remotes, and fast-forwards all
 your local branches that are tracking a remote, and everyone on the
 team uses it all the time.  It should be said this team also likes to
 use Git bare-metal, because they like knowing how things work
 out-of-the-box.  But they all use the command because it's so
 convenient.

I also wrote a script to fast-forward all local branches to their
configured upstream refs.  I finally got around to uploading it
somewhere public:

   https://github.com/richardhansen/git-update-branch

I use it in my 'git up' alias:

   git config --global alias.up \
   '!git remote update -p; git update-branch -a'

If there's interest I can tweak the style to conform to
Documentation/CodingGuidelines and stick it in contrib/ or something.

-Richard
--
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] git-prompt.sh: don't assume the shell expands the value of PS1

2014-05-19 Thread Richard Hansen
Not all shells subject the prompt string to parameter expansion.  Test
whether the shell will expand the value of PS1, and use the result to
control whether raw ref names are included directly in PS1.

This fixes a regression introduced in commit 8976500 (git-prompt.sh:
don't put unsanitized branch names in $PS1):  zsh does not expand PS1
by default, but that commit assumed it did.  The bug resulted in
prompts containing the literal string '${__git_ps1_branch_name}'
instead of the actual branch name.

Reported-by: Caleb Thompson ca...@calebthompson.io
Signed-off-by: Richard Hansen rhan...@bbn.com
---

To prevent a regression like this from happening again, I plan on
adding new zsh test cases and expanding the bash test cases (to test
the behavior with 'shopt -u promptvars').  I'd like the zsh tests to
cover the same stuff as the bash tests.  These are the steps I am
considering:

  1. delete the last test case in t9903 (prompt - zsh color pc mode)
  2. add two new functions to t/lib-bash.sh:
 ps1_expansion_enable () { shopt -s promptvars; }
 ps1_expansion_disable () { shopt -u promptvars; }
  3. loop over the relevant test cases twice:  once after calling
 ps1_expansion_enable and once after calling ps1_expansion_disable
 (with appropriate adjustments to the expected output)
  4. move the test cases in t9903 to a separate library file and
 source it from t9903-bash-prompt.sh
  5. create two new files:
   * t/lib-zsh.sh (same as t/lib-bash.sh but tweaked for zsh)
   * t/t9904-zsh-prompt.sh (same as t/t9903-bash-prompt.sh but
 tweaked for zsh)

Does this approach sound reasonable?

 contrib/completion/git-prompt.sh | 56 
 t/t9903-bash-prompt.sh   |  6 ++---
 2 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 853425d..9d684b1 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -209,9 +209,7 @@ __git_ps1_show_upstream ()
if [[ -n $count  -n $name ]]; then
__git_ps1_upstream_name=$(git rev-parse \
--abbrev-ref $upstream 2/dev/null)
-   if [ $pcmode = yes ]; then
-   # see the comments around the
-   # __git_ps1_branch_name variable below
+   if [ $pcmode = yes ]  [ $ps1_expanded = yes ]; then
p=$p \${__git_ps1_upstream_name}
else
p=$p ${__git_ps1_upstream_name}
@@ -308,6 +306,43 @@ __git_ps1 ()
;;
esac
 
+   # ps1_expanded:  This variable is set to 'yes' if the shell
+   # subjects the value of PS1 to parameter expansion:
+   #
+   #   * bash does unless the promptvars option is disabled
+   #   * zsh does not unless the PROMPT_SUBST option is set
+   #   * POSIX shells always do
+   #
+   # If the shell would expand the contents of PS1 when drawing
+   # the prompt, a raw ref name must not be included in PS1.
+   # This protects the user from arbitrary code execution via
+   # specially crafted ref names.  For example, a ref named
+   # 'refs/heads/$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)' might cause the
+   # shell to execute 'sudo rm -rf /' when the prompt is drawn.
+   #
+   # Instead, the ref name should be placed in a separate global
+   # variable (in the __git_ps1_* namespace to avoid colliding
+   # with the user's environment) and that variable should be
+   # referenced from PS1.  For example:
+   #
+   # __git_ps1_foo=$(do_something_to_get_ref_name)
+   # PS1=...stuff...\${__git_ps1_foo}...stuff...
+   #
+   # If the shell does not expand the contents of PS1, the raw
+   # ref name must be included in PS1.
+   #
+   # The value of this variable is only relevant when in pcmode.
+   #
+   # Assume that the shell follows the POSIX specification and
+   # expands PS1 unless determined otherwise.  (This is more
+   # likely to be correct if the user has a non-bash, non-zsh
+   # shell and safer than the alternative if the assumption is
+   # incorrect.)
+   #
+   local ps1_expanded=yes
+   [ -z $ZSH_VERSION ] || [[ -o PROMPT_SUBST ]] || ps1_expanded=no
+   [ -z $BASH_VERSION ] || shopt -q promptvars || ps1_expanded=no
+
local repo_info rev_parse_exit_code
repo_info=$(git rev-parse --git-dir --is-inside-git-dir \
--is-bare-repository --is-inside-work-tree \
@@ -457,21 +492,8 @@ __git_ps1 ()
fi
 
b=${b##refs/heads/}
-   if [ $pcmode = yes ]; then
-   # In pcmode (and only pcmode) the contents of
-   # $gitstring are subject to expansion by the shell.
-   # Avoid putting the raw ref name

Re: [PATCH] RelNotes/2.0.0.txt: Fix several grammar issues, notably a lack of hyphens, double quotes, or articles

2014-05-19 Thread Richard Hansen
On 2014-05-19 19:46, Junio C Hamano wrote:
 Jason St. John jstj...@purdue.edu writes:
 @@ -53,7 +53,7 @@ Updates since v1.9 series
  UI, Workflows  Features
  
   * The multi-mail post-receive hook (in contrib/) has been updated
 -   to a more recent version from the upstream.
 +   to a more recent version from upstream.
 
 Hmph, I have only one multi-mail upstream; shouldn't I call it the
 upstream from my point of view?

Plain upstream (without the) is correct because it's an adverb, not
a noun.  (Alternatively, this could be written from the upstream
repository or from the upstream project.)

 @@ -217,7 +218,7 @@ notes for details).
   * git diff --no-index -Mq a b fell into an infinite loop.
 (merge ad1c3fb jc/fix-diff-no-index-diff-opt-parse later to maint).
  
 - * git fetch --prune, when the right-hand-side of multiple fetch
 + * git fetch --prune, when the right-hand side of multiple fetch
 refspecs overlap (e.g. storing refs/heads/* to
 
 Hmph, I read this as a right-hand, a multi-word adjective, is used
 to describe one side (the other side being the left-hand side).
 Otherwise, you would be writing command-line-option, no?

Are you reading the diff backwards?  (The second hyphen is being
removed, not added.)

-Richard
--
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: [ANNOUNCE] Git v2.0.0-rc4

2014-05-21 Thread Richard Hansen
On 2014-05-20 20:24, Junio C Hamano wrote:
 Fixes since v1.9 series
 ---
 
 Unless otherwise noted, all the fixes since v1.9 in the maintenance
 track are contained in this release (see the maintenance releases'
 notes for details).
[...]

  * The shell prompt script (in contrib/), when using the PROMPT_COMMAND
interface, used an unsafe construct when showing the branch name in
$PS1.
(merge 8976500 rh/prompt-pcmode-avoid-eval-on-refname later to maint).

That commit has been merged to maint and is in v1.9.3.

Also, 1e4119c (git-prompt.sh: don't assume the shell expands the value
of PS1) is a fix that is in v2.0.0-rc4 but not v1.9.x.  Maybe add
something like:

 * The shell prompt script (in contrib/), when using Zsh and the
   precmd() interface, printed ${__git_ps1_branch_name} in the prompt
   instead of the branch name (regression in v1.9.3).
   (merge 1e4119c rh/prompt-pcmode-avoid-eval-on-refname later to
   maint).

-Richard
--
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] git-prompt.sh: shorter equal upstream branch name

2014-09-30 Thread Richard Hansen
On 2014-09-30 11:36, Julien Carsique wrote:
 From: Julien Carsique julien.carsi...@gmail.com
 
 When using the name option of GIT_PS1_SHOWUPSTREAM to show the upstream
 abbrev name, if the upstream name is the same as the local name, then some
 space could be saved in the prompt. This is especially needed on long branch
 names.
 
 Replace the upstream name with the sign '=' if equal to the local one.
 Example:[master * u= origin/=]$
 instead of: [master * u= origin/master]$

Seems like a good idea to me.

 
 Signed-off-by: Julien Carsique julien.carsi...@gmail.com
 ---
  contrib/completion/git-prompt.sh | 7 +++
  1 file changed, 7 insertions(+)

Please add some new tests in t/9903-bash-prompt.sh.  In particular:
  * upstream ref in refs/heads
  * upstream is git-svn
  * branch names containing slashes

 
 diff --git a/contrib/completion/git-prompt.sh 
 b/contrib/completion/git-prompt.sh
 index c5473dc..a9aba20 100644
 --- a/contrib/completion/git-prompt.sh
 +++ b/contrib/completion/git-prompt.sh
 @@ -209,6 +209,13 @@ __git_ps1_show_upstream ()
   if [[ -n $count  -n $name ]]; then
   __git_ps1_upstream_name=$(git rev-parse \
   --abbrev-ref $upstream 2/dev/null)
 +
 + __head=${b##refs/heads/}

To avoid colliding with other stuff, this variable should either be
local or prefixed with '__git_ps1'.

 + if [ $__head = ${__git_ps1_upstream_name##*/} ]; 
 then

This comparison breaks on branches containing a slash (e.g., foo/bar).

Also, how does this interact with git-svn?  (I don't use git-svn so I'm
not very familiar with how it manages refs.)

Assuming remote names can't contain a slash (which I think is true), a
safer approach might be parse the full ref and special-case refs/remotes:

__git_ps1_upstream_name=$(git rev-parse \
--abbrev-ref ${upstream} 2/dev/null)
local tmp
tmp=$(git rev-parse --symbolic-full-name ${upstream} 2/dev/null)
case ${tmp} in
refs/remotes/*)
# todo: can ${b} be something other than refs/heads/* here?
[ ${__git_ps1_upstream_name#*/} != ${b#refs/heads/} ] \
|| __git_ps1_upstream_name=${__git_ps1_upstream_name%%/*}/\=
;;
esac

Additional cases could be added to handle git-svn if needed.

 + 
 __git_ps1_upstream_name=${__git_ps1_upstream_name/$__head/=}

  * This could break if ${__head} contains any pattern-special
characters.

  * While this syntax works in both Bash and Zsh (assuming no
pattern-special characters), my preference is to stick to POSIX[1]
when possible.  For example, assuming the upstream name is
always in refs/remotes (which is not true, but this is an example)
and remote names can't contain a '/', you could do this:

__git_ps1_upstream_name=${__git_ps1_upstream_name%%/*}/\=

  * I don't think the CodingGuidelines explicitly prohibit long lines
for shell code, and this file already contains plenty of long
lines, but I really dislike lines longer than 80 characters.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html

 + fi
 + unset __head
 +
   if [ $pcmode = yes ]  [ $ps1_expanded = yes ]; then
   p=$p \${__git_ps1_upstream_name}
   else

-Richard
--
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] git-prompt.sh: shorter equal upstream branch name

2014-09-30 Thread Richard Hansen
On 2014-09-30 18:35, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:
   * While this syntax works in both Bash and Zsh (assuming no
 pattern-special characters), my preference is to stick to POSIX[1]
 when possible.
 
 Nah.  The existing script is full of bash-isms like local you
 suggested to add (and other constructs like shell arrays and [[ ]]
 tests, I suspect),

True.

 and there is no hope to fix them to stick to
 the bare-minimum POSIX,

I don't think it'd be hard to convert it to pure POSIX if there was a
desire to do so.  The biggest challenge would be 'local', which would
require subshells or uniquified prefixed global variables.  Both of
those are likely to make the code a bit grotesque.

 and there is no need to do so (isn't this
 bash-prompt script after all?)

It would be unwise to go to great lengths to avoid Bashisms, but I think
it would be smart to use POSIX syntax when it is easy to do so.  Rarely
is it hard or awkward to use POSIX syntax ('local' and arrays are two
major exceptions), so Bashisms like the ${//} expansion in this patch
are usually unnecessary divergences from a ubiquitous standard.  POSIX
is a stable foundation, and it's easy to get POSIX shell code to run
consistently on all POSIX-like shells.

One of these days I'll try converting git-prompt.sh to POSIX -- I'm
curious to see how bad it would be.

-Richard
--
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] git-prompt.sh: shorter equal upstream branch name

2014-10-07 Thread Richard Hansen
On 2014-10-07 11:57, Julien Carsique wrote:
 Hi,
 
 Thank you both for your feedback!
 I'm looking at applying your requests:
 - add tests,
 - variable renaming,
 - use of local,
 - fix multiple issues on string parsing
 - avoid useless bash-isms? Did you agree on the ones I should remove?

I'm guessing the structure of your code will change somewhat when you
address the other issues, so I think it may be premature to discuss
specific Bashisms right now.  (There aren't any particular Bashisms that
I think should always be avoided -- I just want people to ponder whether
a particular use of a Bashism is truly preferable to a POSIX-conformant
alternative.)

Write the code in the way you think is best, and if I see a good way to
convert a Bashism to POSIX I'll let you know.  And feel free to ignore
me -- I'm a member of the Church of POSIX Conformance while Junio is
much more grounded in reality.  :)

 
 I'll send an updated patch asap. Tell me if I forgot something.

Your list looks complete to me.  Thank you for contributing!

-Richard


 
 Regards,
 Julien
 
 On 01/10/2014 19:49, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:

 and there is no hope to fix them to stick to
 the bare-minimum POSIX,
 I don't think it'd be hard to convert it to pure POSIX if there was a
 desire to do so.
 Not necessarily; if you make it so slow to be usable as a prompt
 script, that is not a conversion.  Bash-isms in the script is
 allowed for a reason, unfortunately.

 It would be unwise to go to great lengths to avoid Bashisms, but I think
 it would be smart to use POSIX syntax when it is easy to do so.  
 In general, I agree with you. People who know only bash tend to
 overuse bash-isms where they are not necessary, leaving an
 unreadable mess.

 For the specific purpose of Julien's if the tail part of this
 string matches the other string, replace that with an equal sign,
 ${parameter/pattern/string} is a wrong bash-ism to use.  But the
 right solution to count the length of the other string and take a
 substring of this string from its beginning would require other
 bash-isms ${#parameter} and ${parameter:offset:length}.

 And that's 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: [PATCH] git-prompt.sh: Omit prompt for ignored directories

2014-10-08 Thread Richard Hansen
On 2014-10-08 15:04, Jess Austin wrote:
 Introduce a new environmental variable, GIT_PS1_OMITIGNORED, which
 tells __git_ps1 to display nothing when the current directory is
 set (e.g. via .gitignore) to be ignored by git. In the absence of
 GIT_PS1_OMITIGNORED this change has no effect.
 
 Many people manage e.g. dotfiles in their home directory with git.
 This causes the prompt generated by __git_ps1 to refer to that top
 level repo while working in any descendant directory. That can be
 distracting, so this patch helps one shut off that noise.

Interesting idea, though I would prefer this to be configurable on a
per-repository basis.  (I wouldn't want to hide the prompt in any
repository besides my home repository.)

I'm not a big fan of the name OMITIGNORED (it's not immediately
obvious what this means), but I can't think of anything better off the
top of my head...

-Richard


 
 Signed-off-by: Jess Austin jess.aus...@gmail.com
 ---
  contrib/completion/git-prompt.sh |  9 +
  t/t9903-bash-prompt.sh   | 21 +
  2 files changed, 30 insertions(+)
 
 diff --git a/contrib/completion/git-prompt.sh 
 b/contrib/completion/git-prompt.sh
 index c5473dc..6a26cb4 100644
 --- a/contrib/completion/git-prompt.sh
 +++ b/contrib/completion/git-prompt.sh
 @@ -84,6 +84,10 @@
  # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
  # the colored output of git status -sb and are available only when
  # using __git_ps1 for PROMPT_COMMAND or precmd.
 +#
 +# If you would like __git_ps1 to do nothing in the case when the current
 +# directory is set up to be ignored by git, then set GIT_PS1_OMITIGNORED
 +# to a nonempty value.
  
  # check whether printf supports -v
  __git_printf_supports_v=
 @@ -501,6 +505,11 @@ __git_ps1 ()
   local f=$w$i$s$u
   local gitstring=$c$b${f:+$z$f}$r$p
  
 + if [ -n $(git check-ignore .) ]  [ -n ${GIT_PS1_OMITIGNORED} ]
 + then
 + printf_format=
 + fi
 +
   if [ $pcmode = yes ]; then
   if [ ${__git_printf_supports_v-} != yes ]; then
   gitstring=$(printf -- $printf_format $gitstring)
 diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
 index 9150984..55bcb6b 100755
 --- a/t/t9903-bash-prompt.sh
 +++ b/t/t9903-bash-prompt.sh
 @@ -35,6 +35,8 @@ test_expect_success 'setup for prompt tests' '
   git commit -m another b2 file 
   echo 000 file 
   git commit -m yet another b2 file 
 + mkdir ignored_dir 
 + echo ignored_dir/  .gitignore 
   git checkout master
  '
  
 @@ -588,4 +590,23 @@ test_expect_success 'prompt - zsh color pc mode' '
   test_cmp expected $actual
  '
  
 +test_expect_success 'prompt - prompt omitted in ignored directory' '
 + printf  expected 
 + (
 + cd ignored_dir 
 + GIT_PS1_OMITIGNORED=y 
 + __git_ps1 $actual
 + ) 
 + test_cmp expected $actual
 +'
 +
 +test_expect_success 'prompt - prompt not omitted without 
 GIT_PS1_OMITIGNORED' '
 + printf  (master) expected 
 + (
 + cd ignored_dir 
 + __git_ps1 $actual
 + ) 
 + test_cmp expected $actual
 +'
 +
  test_done
 

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


Re: [PATCH] git-prompt.sh: Omit prompt for ignored directories

2014-10-08 Thread Richard Hansen
On 2014-10-08 17:37, Jess Austin wrote:
 On Wed, Oct 8, 2014 at 4:12 PM, Richard Hansen rhan...@bbn.com wrote:
 On 2014-10-08 15:04, Jess Austin wrote:
 Introduce a new environmental variable, GIT_PS1_OMITIGNORED, which
 tells __git_ps1 to display nothing when the current directory is
 set (e.g. via .gitignore) to be ignored by git. In the absence of
 GIT_PS1_OMITIGNORED this change has no effect.

 Many people manage e.g. dotfiles in their home directory with git.
 This causes the prompt generated by __git_ps1 to refer to that top
 level repo while working in any descendant directory. That can be
 distracting, so this patch helps one shut off that noise.

 Interesting idea, though I would prefer this to be configurable on a
 per-repository basis.  (I wouldn't want to hide the prompt in any
 repository besides my home repository.)
 
 Sorry my description was unclear. Let's say you have a repo in ~,
 and another in ~/projects/foo. Also, the file ~/.gitignore has the line
 projects/ in it. In this case, you'd see repo info in your prompt while
 in ~ or in ~/projects/foo, but not if you were in ~/projects. In that
 sense, the prompt is not distracting you with the status of the top-level
 repo when you're not looking at anything in that repo.

I understand; I was concerned about this case:

$ PS1='\n\w$(__git_ps1  (%s))\n\$ '

/home/rhansen/projects (dotfiles)
$ GIT_PS1_OMITIGNORED=y

/home/rhansen/projects  -- Git prompt goes away as desired
$ cd foo

/home/rhansen/projects/foo (master) -- Git prompt back as expected
$ echo ignored/ .gitignore  mkdir -p ignored  cd ignored

/home/rhansen/projects/foo/ignored  -- I want the Git prompt here
$

In other words:  If I were to use this feature, I'd want to be able to
hide the prompt when I'm in an ignored directory in my dotfiles work
tree, but show the prompt when I'm in an ignored directory in any other
work tree.

-Richard
--
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] git-prompt.sh: Omit prompt for ignored directories

2014-10-09 Thread Richard Hansen
On 2014-10-09 06:27, Jess Austin wrote:
 On Thu, Oct 9, 2014 at 12:37 AM, Richard Hansen rhan...@bbn.com wrote:
 On 2014-10-08 17:37, Jess Austin wrote:
 On Wed, Oct 8, 2014 at 4:12 PM, Richard Hansen rhan...@bbn.com wrote:
 On 2014-10-08 15:04, Jess Austin wrote:
 Introduce a new environmental variable, GIT_PS1_OMITIGNORED, which
 tells __git_ps1 to display nothing when the current directory is
 set (e.g. via .gitignore) to be ignored by git. In the absence of
 GIT_PS1_OMITIGNORED this change has no effect.

 Many people manage e.g. dotfiles in their home directory with git.
 This causes the prompt generated by __git_ps1 to refer to that top
 level repo while working in any descendant directory. That can be
 distracting, so this patch helps one shut off that noise.
...

 $ PS1='\n\w$(__git_ps1  (%s))\n\$ '

 /home/rhansen/projects (dotfiles)
 $ GIT_PS1_OMITIGNORED=y

 /home/rhansen/projects  -- Git prompt goes away as desired
 $ cd foo

 /home/rhansen/projects/foo (master) -- Git prompt back as expected
 $ echo ignored/ .gitignore  mkdir -p ignored  cd ignored

 /home/rhansen/projects/foo/ignored  -- I want the Git prompt here
 $

 In other words:  If I were to use this feature, I'd want to be able to
 hide the prompt when I'm in an ignored directory in my dotfiles work
 tree, but show the prompt when I'm in an ignored directory in any other
 work tree.
 
 Would you want this configured in each repo (i.e. via a line in 
 .git/config),
 or would you prefer something global so that it only need be set in one
 place? I'm not sure how the latter technique would work, so if that seems
 better please advise on how to go about that.

A 'git config' variable is fine.  The bash.showDirtyState,
bash.showUntrackedFiles, and bash.showUpstream config variables seem
like good examples to follow.

-Richard
--
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] git-prompt.sh: Hide prompt for ignored pwd

2014-10-14 Thread Richard Hansen
On 2014-10-14 14:47, Johannes Sixt wrote:
 Am 14.10.2014 um 04:32 schrieb Jess Austin:
 diff --git a/contrib/completion/git-prompt.sh 
 b/contrib/completion/git-prompt.sh
 index c5473dc..d7559ff 100644
 --- a/contrib/completion/git-prompt.sh
 +++ b/contrib/completion/git-prompt.sh
 @@ -84,6 +84,11 @@
  # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
  # the colored output of git status -sb and are available only when
  # using __git_ps1 for PROMPT_COMMAND or precmd.
 +#
 +# If you would like __git_ps1 to do nothing in the case when the current
 +# directory is set up to be ignored by git, then set
 +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set
 +# bash.hideOnIgnoredPwd to true in the repository configuration.
  
  # check whether printf supports -v
  __git_printf_supports_v=
 @@ -501,6 +506,13 @@ __git_ps1 ()
  local f=$w$i$s$u
  local gitstring=$c$b${f:+$z$f}$r$p
  
 +if [ -n $(git check-ignore .) ] 
 +   ( [ -n ${GIT_PS1_HIDE_ON_IGNORED_PWD} ] ||
 + [ $(git config --bool bash.hideOnIgnoredPwd) = true ] )
 
 Ahem, no. Please do not punish users who are not interested in the new
 feature with two new processes every time __git_ps() is run. Think of
 Windows where fork() is really, *really* expensive.

Is this why bash.showDirtyState and friends aren't checked unless the
corresponding environment variable is set to a non-empty value?

Regardless, it would be nice if the behavior matched the other bash.*
variables (only check the bash.* variable if the corresponding
environment variable is set, and default to true).  The following should
fix it:

if [ -n ${GIT_PS1_HIDE_ON_IGNORED_PWD} ] 
   [ $(git config --bool bash.hideOnIgnoredPwd) != false ] 
   [ $(git check-ignore .) ]
then
...

-Richard

 
 BTW, you can write '{ foo || bar; }' to bracket a || chain without a
 sub-process.
 
 +then
 +printf_format=
 +fi
 +
  if [ $pcmode = yes ]; then
  if [ ${__git_printf_supports_v-} != yes ]; then
  gitstring=$(printf -- $printf_format $gitstring)
 
 -- Hannes
 

--
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] git-prompt.sh: Hide prompt for ignored pwd

2014-10-14 Thread Richard Hansen
On 2014-10-13 22:32, Jess Austin wrote:
 Set __git_ps1 to display nothing when present working directory is
 ignored, triggered by either the new environmental variable
 GIT_PS1_HIDE_ON_IGNORED_PWD or the new repository configuration
 variable bash.hideOnIgnoredPwd (or both). In the absence of these
 settings this change has no effect.
 
 Many people manage e.g. dotfiles in their home directory with git.
 This causes the prompt generated by __git_ps1 to refer to that top
 level repo while working in any descendant directory. That can be
 distracting, so this patch helps one shut off that noise.
 
 Signed-off-by: Jess Austin jess.aus...@gmail.com
 ---
 On Thu, Oct 9, 2014 at 5:09 PM, Richard Hansen rhan...@bbn.com wrote:
 On 2014-10-09 06:27, Jess Austin wrote:
 Would you want this configured in each repo (i.e. via a line in 
 .git/config),
 or would you prefer something global so that it only need be set in one
 place? I'm not sure how the latter technique would work, so if that seems
 better please advise on how to go about that.

 A 'git config' variable is fine.  The bash.showDirtyState,
 bash.showUntrackedFiles, and bash.showUpstream config variables seem
 like good examples to follow.
 
 I think this is what you meant. I changed the name of the envvar. Now the
 variables are GIT_PS1_HIDE_ON_IGNORED_PWD and bash.hideOnIgnoredPwd. I
 admit these are still kind of unwieldy, but maybe now they're more 
 descriptive?

I do prefer the new names.  They are long, but how often will someone
have to type it?  In this case it's better to be descriptive than to be
short.  (I wonder if adding two letters would improve readability
further:  GIT_PS1_HIDE_WHEN_PWD_IGNORED and bash.hideWhenPwdIgnored.)

To avoid scaring people who might not want this feature enabled, I
recommend changing the subject line to something like this:

git-prompt.sh: Option to hide prompt for ignored pwd

 
 Please advise!
 
 cheers,
 Jess
 
  contrib/completion/git-prompt.sh | 12 
  t/t9903-bash-prompt.sh   | 42 
 
  2 files changed, 54 insertions(+)
 
 diff --git a/contrib/completion/git-prompt.sh 
 b/contrib/completion/git-prompt.sh
 index c5473dc..d7559ff 100644
 --- a/contrib/completion/git-prompt.sh
 +++ b/contrib/completion/git-prompt.sh
 @@ -84,6 +84,11 @@
  # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
  # the colored output of git status -sb and are available only when
  # using __git_ps1 for PROMPT_COMMAND or precmd.
 +#
 +# If you would like __git_ps1 to do nothing in the case when the current
 +# directory is set up to be ignored by git, then set
 +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set
 +# bash.hideOnIgnoredPwd to true in the repository configuration.

As mentioned in my previous email, I would prefer the code to follow the
behavior of the other config variables (the environment variable has to
be set *and* the config variable has to be non-false).

  
  # check whether printf supports -v
  __git_printf_supports_v=
 @@ -501,6 +506,13 @@ __git_ps1 ()
   local f=$w$i$s$u
   local gitstring=$c$b${f:+$z$f}$r$p
  
 + if [ -n $(git check-ignore .) ] 

Rather than:

[ -n $(git check-ignore .) ]

I would prefer:

git check-ignore -q .

For example:

if [ -n ${GIT_PS1_HIDE_ON_IGNORED_PWD} ] 
   [ $(git config --bool bash.hideOnIgnoredPwd) != false ] 
   git check-ignore -q .
then
...

-Richard


 +( [ -n ${GIT_PS1_HIDE_ON_IGNORED_PWD} ] ||
 +  [ $(git config --bool bash.hideOnIgnoredPwd) = true ] )
 + then
 + printf_format=
 + fi
 +
   if [ $pcmode = yes ]; then
   if [ ${__git_printf_supports_v-} != yes ]; then
   gitstring=$(printf -- $printf_format $gitstring)
 diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
 index 9150984..a8ef8a3 100755
 --- a/t/t9903-bash-prompt.sh
 +++ b/t/t9903-bash-prompt.sh
 @@ -35,6 +35,8 @@ test_expect_success 'setup for prompt tests' '
   git commit -m another b2 file 
   echo 000 file 
   git commit -m yet another b2 file 
 + mkdir ignored_dir 
 + echo ignored_dir/  .gitignore 
   git checkout master
  '
  
 @@ -588,4 +590,44 @@ test_expect_success 'prompt - zsh color pc mode' '
   test_cmp expected $actual
  '
  
 +test_expect_success 'prompt - hide on ignored pwd - shell variable unset 
 with config disabled' '
 + printf  (master) expected 
 + (
 + cd ignored_dir 
 + __git_ps1 $actual
 + ) 
 + test_cmp expected $actual
 +'
 +
 +test_expect_success 'prompt - hide on ignored pwd - shell variable unset 
 with config enabled' '
 + printf  expected 
 + test_config bash.hideOnIgnoredPwd true 
 + (
 + cd ignored_dir 
 + __git_ps1 $actual
 + ) 
 + test_cmp expected $actual
 +'
 +
 +test_expect_success 'prompt - hide on ignored pwd - shell variable set with 
 config

Re: [PATCH] git-prompt.sh: Option to hide prompt for ignored pwd

2014-10-15 Thread Richard Hansen
On 2014-10-15 00:06, Jess Austin wrote:
 @@ -501,6 +506,13 @@ __git_ps1 ()
   local f=$w$i$s$u
   local gitstring=$c$b${f:+$z$f}$r$p
  
 + if [ -n ${GIT_PS1_HIDE_IF_PWD_IGNORED} ] 
 +[ $(git config --bool bash.hideIfPwdIgnored) != false ] 
 +git check-ignore -q .
 + then
 + printf_format=
 + fi
 +
   if [ $pcmode = yes ]; then
   if [ ${__git_printf_supports_v-} != yes ]; then
   gitstring=$(printf -- $printf_format $gitstring)

This is broken in pcmode due to a Bash bug.  The command:
printf -v foo  asdf
is a no-op in Bash.  The variable foo is never changed in any way --
it is neither unset nor set to the empty string.

Also, I noticed that I get an error message if I cd into .git:
fatal: This operation must be run in a work tree

I think the following change will fix the above issues, and it has the
advantage of avoiding unnecessary work if the directory is ignored:

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 6a4ce53..68ac82a 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -374,6 +374,17 @@ __git_ps1 ()
local inside_gitdir=${repo_info##*$'\n'}
local g=${repo_info%$'\n'*}
 
+   if [ true = $inside_worktree ] 
+  [ -n ${GIT_PS1_HIDE_IF_PWD_IGNORED} ] 
+  [ $(git config --bool bash.hideIfPwdIgnored) != false ] 
+  git check-ignore -q .
+   then
+   if [ $pcmode = yes ]; then
+   PS1=$ps1pc_start$ps1pc_end
+   fi
+   return
+   fi
+
local r=
local b=
local step=
@@ -506,13 +517,6 @@ __git_ps1 ()
local f=$w$i$s$u
local gitstring=$c$b${f:+$z$f}$r$p
 
-   if [ -n ${GIT_PS1_HIDE_IF_PWD_IGNORED} ] 
-  [ $(git config --bool bash.hideIfPwdIgnored) != false ] 
-  git check-ignore -q .
-   then
-   printf_format=
-   fi
-
if [ $pcmode = yes ]; then
if [ ${__git_printf_supports_v-} != yes ]; then
gitstring=$(printf -- $printf_format $gitstring)

It would be good to add additional test cases for pcmode (two or three
arguments to __git_ps1) and 'cd .git' so that the above issues don't
reappear.

Thanks,
Richard
--
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/7] glossary: mention 'treeish' as an alternative to 'tree-ish'

2013-09-01 Thread Richard Hansen
The documentation contains a mix of the two spellings, so include both
in the glossary so that a search for either will lead to the
definition.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index dba5062..0273095 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -486,7 +486,7 @@ should not be combined with other pathspec.
with refs to the associated blob and/or tree objects. A
def_tree,tree is equivalent to a def_directory,directory.
 
-[[def_tree-ish]]tree-ish::
+[[def_tree-ish]]tree-ish (also treeish)::
A def_ref,ref pointing to either a def_commit_object,commit
object, a def_tree_object,tree object, or a def_tag_object,tag
object pointing to a tag or commit or tree object.
-- 
1.8.4

--
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 7/7] glossary: fix and clarify the definition of 'ref'

2013-09-01 Thread Richard Hansen
Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index a2edcc3..44d524b 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -395,10 +395,20 @@ should not be combined with other pathspec.
to the result.
 
 [[def_ref]]ref::
-   A 40-byte hex representation of a def_SHA1,SHA-1 or a name that
-   denotes a particular def_object,object. They may be stored in
-   a file under `$GIT_DIR/refs/` directory, or
-   in the `$GIT_DIR/packed-refs` file.
+   A name that begins with `refs/` (e.g. `refs/heads/master`)
+   that points to an def_object_name,object name or another
+   ref (the latter is called a def_symref,symbolic ref).
+   For convenience, a ref can sometimes be abbreviated when used
+   as an argument to a Git command; see linkgit:gitrevisions[7]
+   for details.
+   Refs are stored in the def_repository,repository.
++
+The ref namespace is hierarchical.
+Different subhierarchies are used for different purposes (e.g. the
+`refs/heads/` hierarchy is used to represent local branches).
++
+There are a few special-purpose refs that do not begin with `refs/`.
+The most notable example is `HEAD`.
 
 [[def_reflog]]reflog::
A reflog shows the local history of a ref.  In other words,
-- 
1.8.4

--
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/7] documentation cleanups for rev^{type}

2013-09-02 Thread Richard Hansen
The documentation for the rev^{type} syntax (e.g., v1.8.4^{tree})
needed some fixing, and the fixes motivated some other documentation
cleanups.

Changes since v1 (2013-06-18, see
http://thread.gmane.org/gmane.comp.version-control.git/228325):
  * standardize on 'treeish' (don't use 'tree-ish')
  * standardize on 'committish' (don't use 'commit-ish')
  * don't base the definition of 'committish' on the broken definition
of 'ref'
  * fix the definition of 'treeish' so that it is not based on the
broken definition of 'ref'
  * fix the definiton of 'ref'

Richard Hansen (7):
  glossary: mention 'treeish' as an alternative to 'tree-ish'
  glossary: define committish (a.k.a. commit-ish)
  use 'treeish' instead of 'tree-ish'
  use 'committish' instead of 'commit-ish'
  glossary: more precise definition of treeish (a.k.a. tree-ish)
  revisions.txt: fix and clarify rev^{type}
  glossary: fix and clarify the definition of 'ref'

 Documentation/RelNotes/1.6.0.5.txt   |  2 +-
 Documentation/RelNotes/1.6.2.4.txt   |  2 +-
 Documentation/RelNotes/1.8.1.2.txt   |  2 +-
 Documentation/RelNotes/1.8.2.txt |  2 +-
 Documentation/diff-format.txt| 10 +++
 Documentation/diff-generate-patch.txt|  2 +-
 Documentation/git-archive.txt|  4 +--
 Documentation/git-checkout.txt   | 16 +--
 Documentation/git-diff-index.txt |  4 +--
 Documentation/git-diff-tree.txt  | 14 +-
 Documentation/git-ls-files.txt   |  6 ++--
 Documentation/git-ls-tree.txt|  6 ++--
 Documentation/git-read-tree.txt  | 10 +++
 Documentation/git-rebase.txt |  2 +-
 Documentation/git-reset.txt  | 16 +--
 Documentation/git-rev-parse.txt  |  2 +-
 Documentation/git-svn.txt|  6 ++--
 Documentation/git-tar-tree.txt   |  4 +--
 Documentation/git.txt|  8 +++---
 Documentation/gitcli.txt |  2 +-
 Documentation/gittutorial-2.txt  |  2 +-
 Documentation/glossary-content.txt   | 47 ++--
 Documentation/revisions.txt  | 14 ++
 archive.c|  6 ++--
 builtin/checkout.c   |  6 ++--
 builtin/diff-index.c |  2 +-
 builtin/diff-tree.c  |  2 +-
 builtin/diff.c   |  2 +-
 builtin/ls-files.c   |  8 +++---
 builtin/ls-tree.c|  2 +-
 builtin/read-tree.c  |  2 +-
 builtin/reset.c  |  4 +--
 builtin/revert.c |  4 +--
 builtin/tar-tree.c   | 12 
 contrib/examples/git-checkout.sh | 10 +++
 contrib/examples/git-reset.sh|  2 +-
 contrib/examples/git-revert.sh   |  4 +--
 fast-import.c|  2 +-
 git-svn.perl |  6 ++--
 gitweb/gitweb.perl   |  4 +--
 po/de.po | 20 +++---
 po/fr.po | 20 +++---
 po/git.pot   | 20 +++---
 po/sv.po | 20 +++---
 po/vi.po | 41 ++--
 po/zh_CN.po  | 20 +++---
 remote.c |  2 +-
 t/t1512-rev-parse-disambiguation.sh  | 26 +-
 t/t2010-checkout-ambiguous.sh|  2 +-
 t/t4100/t-apply-3.patch  |  8 +++---
 t/t4100/t-apply-7.patch  |  8 +++---
 t/t9300-fast-import.sh   |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh   |  2 +-
 t/t9501-gitweb-standalone-http-status.sh |  8 +++---
 54 files changed, 247 insertions(+), 213 deletions(-)

-- 
1.8.4

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


[PATCH v2 3/7] use 'treeish' instead of 'tree-ish'

2013-09-02 Thread Richard Hansen
Replace all instances of 'tree-ish' with 'treeish':
  * to standardize on a single spelling (the documentation contained a
mix of 'treeish' and 'tree-ish')
  * to be consistent with variable names (hyphens are not usually
allowed in variable names)
  * to be consistent with 'committish'
  * some search engines don't handle hyphens gracefully

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/RelNotes/1.6.0.5.txt   |  2 +-
 Documentation/RelNotes/1.6.2.4.txt   |  2 +-
 Documentation/RelNotes/1.8.1.2.txt   |  2 +-
 Documentation/RelNotes/1.8.2.txt |  2 +-
 Documentation/diff-format.txt| 10 +-
 Documentation/diff-generate-patch.txt|  2 +-
 Documentation/git-archive.txt|  4 ++--
 Documentation/git-checkout.txt   | 16 
 Documentation/git-diff-index.txt |  4 ++--
 Documentation/git-diff-tree.txt  | 14 +++---
 Documentation/git-ls-files.txt   |  6 +++---
 Documentation/git-ls-tree.txt|  6 +++---
 Documentation/git-read-tree.txt  | 10 +-
 Documentation/git-reset.txt  | 16 
 Documentation/git-svn.txt|  6 +++---
 Documentation/git-tar-tree.txt   |  4 ++--
 Documentation/git.txt|  4 ++--
 Documentation/gitcli.txt |  2 +-
 Documentation/gittutorial-2.txt  |  2 +-
 Documentation/glossary-content.txt   |  2 +-
 Documentation/revisions.txt  |  2 +-
 archive.c|  6 +++---
 builtin/checkout.c   |  6 +++---
 builtin/diff-index.c |  2 +-
 builtin/diff-tree.c  |  2 +-
 builtin/diff.c   |  2 +-
 builtin/ls-files.c   |  8 
 builtin/ls-tree.c|  2 +-
 builtin/read-tree.c  |  2 +-
 builtin/reset.c  |  4 ++--
 builtin/tar-tree.c   | 12 ++--
 contrib/examples/git-checkout.sh | 10 +-
 fast-import.c|  2 +-
 git-svn.perl |  6 +++---
 gitweb/gitweb.perl   |  4 ++--
 po/de.po | 16 
 po/fr.po | 16 
 po/git.pot   | 16 
 po/sv.po | 16 
 po/vi.po | 33 
 po/zh_CN.po  | 16 
 t/t1512-rev-parse-disambiguation.sh  | 10 +-
 t/t2010-checkout-ambiguous.sh|  2 +-
 t/t4100/t-apply-3.patch  |  8 
 t/t4100/t-apply-7.patch  |  8 
 t/t9300-fast-import.sh   |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh   |  2 +-
 t/t9501-gitweb-standalone-http-status.sh |  8 
 48 files changed, 169 insertions(+), 170 deletions(-)

diff --git a/Documentation/RelNotes/1.6.0.5.txt 
b/Documentation/RelNotes/1.6.0.5.txt
index a08bb96..d47386b 100644
--- a/Documentation/RelNotes/1.6.0.5.txt
+++ b/Documentation/RelNotes/1.6.0.5.txt
@@ -13,7 +13,7 @@ Fixes since v1.6.0.4
 * git diff always allowed GIT_EXTERNAL_DIFF and --no-ext-diff was no-op for
   the command.
 
-* Giving 3 or more tree-ish to git diff is supposed to show the combined
+* Giving 3 or more treeish to git diff is supposed to show the combined
   diff from second and subsequent trees to the first one, but the order was
   screwed up.
 
diff --git a/Documentation/RelNotes/1.6.2.4.txt 
b/Documentation/RelNotes/1.6.2.4.txt
index f4bf1d0..f75086d 100644
--- a/Documentation/RelNotes/1.6.2.4.txt
+++ b/Documentation/RelNotes/1.6.2.4.txt
@@ -13,7 +13,7 @@ Fixes since v1.6.2.3
 * git-add -p lacked a way to say quit to refuse staging any hunks for
   the remaining paths.  You had to say d and then ^C.
 
-* git-checkout tree-ish submodule did not update the index entry at
+* git-checkout treeish submodule did not update the index entry at
   the named path; it now does.
 
 * git-fast-export choked when seeing a tag that does not point at commit.
diff --git a/Documentation/RelNotes/1.8.1.2.txt 
b/Documentation/RelNotes/1.8.1.2.txt
index 5ab7b18..0cdb2f9 100644
--- a/Documentation/RelNotes/1.8.1.2.txt
+++ b/Documentation/RelNotes/1.8.1.2.txt
@@ -12,7 +12,7 @@ Fixes since v1.8.1.1
after completing a single directory name.
 
  * Command line completion leaked an unnecessary error message while
-   looking for possible matches with paths in tree-ish.
+   looking for possible matches with paths in treeish.
 
  * git archive did not record uncompressed size in the header when
streaming a zip archive, which confused some implementations of unzip.
diff --git a/Documentation/RelNotes/1.8.2.txt b/Documentation/RelNotes/1.8.2.txt
index fc606ae

[PATCH v2 6/7] revisions.txt: fix and clarify rev^{type}

2013-09-02 Thread Richard Hansen
If possible, rev will be dereferenced even if it is not a tag type
(e.g., commit dereferenced to a tree).

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/revisions.txt | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 569b563..1f1e79b 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -111,10 +111,14 @@ some output processing may assume ref names in UTF-8.
 
 'rev{caret}\{type\}', e.g. 'v0.99.8{caret}\{commit\}'::
   A suffix '{caret}' followed by an object type name enclosed in
-  brace pair means the object
-  could be a tag, and dereference the tag recursively until an
-  object of that type is found or the object cannot be
-  dereferenced anymore (in which case, barf).  'rev{caret}0'
+  brace pair means dereference the object at 'rev' recursively until
+  an object of type 'type' is found or the object cannot be
+  dereferenced anymore (in which case, barf).
+  For example, if 'rev' is a committish, 'rev{caret}\{commit\}'
+  describes the corresponding commit object.
+  Similarly, if 'rev' is a treeish, 'rev{caret}\{tree\}' describes
+  the corresponding tree object.
+  'rev{caret}0'
   is a short-hand for 'rev{caret}\{commit\}'.
 +
 'rev{caret}\{object\}' can be used to make sure 'rev' names an
-- 
1.8.4

--
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] peel_onion(): add support for rev^{tag}

2013-09-03 Thread Richard Hansen
Complete the rev^{type} family of object descriptors by having
rev^{tag} dereference rev until a tag object is found (or fail if
unable).

At first glance this may not seem very useful, as commits, trees, and
blobs cannot be peeled to a tag, and a tag would just peel to itself.
However, this can be used to ensure that rev names a tag object:

$ git rev-parse --verify v1.8.4^{tag}
04f013dc38d7512eadb915eba22efc414f18b869
$ git rev-parse --verify master^{tag}
error: master^{tag}: expected tag type, but the object dereferences to tree 
type
fatal: Needed a single revision

Users can already ensure that rev is a tag object by checking the
output of 'git cat-file -t rev', but:
  * users may expect rev^{tag} to exist given that rev^{commit},
rev^{tree}, and rev^{blob} all exist
  * this syntax is more convenient/natural in some circumstances

Signed-off-by: Richard Hansen rhan...@bbn.com
---
Changes since v2 (2013-09-02, see
http://thread.gmane.org/gmane.comp.version-control.git/233598):
  * add a test case (hopefully I didn't go overboard)
  * explain in the commit message why we might want rev^{tag} when
it's already possible to parse the output of 'git cat-file -t
rev'

 Documentation/revisions.txt |  3 +++
 sha1_name.c |  2 ++
 t/t1511-rev-parse-caret.sh  | 29 -
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index d477b3f..b3322ad 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -121,6 +121,9 @@ some output processing may assume ref names in UTF-8.
 object that exists, without requiring 'rev' to be a tag, and
 without dereferencing 'rev'; because a tag is already an object,
 it does not have to be dereferenced even once to get to an object.
++
+'rev{caret}\{tag\}' can be used to ensure that 'rev' identifies an
+existing tag object.
 
 'rev{caret}\{\}', e.g. 'v0.99.8{caret}\{\}'::
   A suffix '{caret}' followed by an empty brace pair
diff --git a/sha1_name.c b/sha1_name.c
index 65ad066..6dc496d 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -679,6 +679,8 @@ static int peel_onion(const char *name, int len, unsigned 
char *sha1)
sp++; /* beginning of type name, or closing brace for empty */
if (!strncmp(commit_type, sp, 6)  sp[6] == '}')
expected_type = OBJ_COMMIT;
+   else if (!strncmp(tag_type, sp, 3)  sp[3] == '}')
+   expected_type = OBJ_TAG;
else if (!strncmp(tree_type, sp, 4)  sp[4] == '}')
expected_type = OBJ_TREE;
else if (!strncmp(blob_type, sp, 4)  sp[4] == '}')
diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index eaefc77..5771cbd 100755
--- a/t/t1511-rev-parse-caret.sh
+++ b/t/t1511-rev-parse-caret.sh
@@ -6,14 +6,21 @@ test_description='tests for ref^{stuff}'
 
 test_expect_success 'setup' '
echo blob a-blob 
-   git tag -a -m blob blob-tag `git hash-object -w a-blob` 
+   BLOB_SHA1=`git hash-object -w a-blob` 
+   git tag -a -m blob blob-tag $BLOB_SHA1 
+   BLOB_TAG_SHA1=`git rev-parse --verify blob-tag` 
mkdir a-tree 
echo moreblobs a-tree/another-blob 
git add . 
TREE_SHA1=`git write-tree` 
git tag -a -m tree tree-tag $TREE_SHA1 
+   TREE_TAG_SHA1=`git rev-parse --verify tree-tag` 
git commit -m Initial 
+   COMMIT_SHA1=`git rev-parse --verify HEAD` 
git tag -a -m commit commit-tag 
+   COMMIT_TAG_SHA1=`git rev-parse --verify commit-tag` 
+   git tag -a -m tag tag-tag $COMMIT_TAG_SHA1 
+   TAG_TAG_SHA1=`git rev-parse --verify tag-tag` 
git branch ref 
git checkout master 
echo modified a-blob 
@@ -54,6 +61,26 @@ test_expect_success 'ref^{tree}' '
test_must_fail git rev-parse blob-tag^{tree}
 '
 
+test_expect_success 'ref^{tag}' '
+   echo $BLOB_TAG_SHA1 expected 
+   git rev-parse --verify blob-tag^{tag} actual 
+   test_cmp expected actual 
+   echo $TREE_TAG_SHA1 expected 
+   git rev-parse --verify tree-tag^{tag} actual 
+   test_cmp expected actual 
+   echo $COMMIT_TAG_SHA1 expected 
+   git rev-parse --verify commit-tag^{tag} actual 
+   test_cmp expected actual 
+   echo $TAG_TAG_SHA1 expected 
+   git rev-parse --verify tag-tag^{tag} actual 
+   test_cmp expected actual 
+   test_must_fail git rev-parse --verify $BLOB_SHA1^{tag} 
+   test_must_fail git rev-parse --verify $TREE_SHA1^{tag} 
+   test_must_fail git rev-parse --verify $COMMIT_SHA1^{tag} 
+   test_must_fail git rev-parse --verify rev^{tag} 
+   true
+'
+
 test_expect_success 'ref^{/.}' '
git rev-parse master expected 
git rev-parse master^{/.} actual 
-- 
1.8.4

--
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

Re: [PATCH v2] peel_onion(): add support for rev^{tag}

2013-09-03 Thread Richard Hansen
On 2013-09-03 03:05, Jeff King wrote:
 FWIW, this makes sense to me.

Thank you for the feedback.  I posted a reroll of the patch that you've
already replied to, but for the benefit of others searching the mailing
list archive, v3 can be found at
http://thread.gmane.org/gmane.comp.version-control.git/233752.

I have a patch submission question:  Is it OK that I didn't use the
'--in-reply-to' argument to 'git send-email' when I sent the v3 reroll?
 Should I have marked it as a reply to the v2 email?  Or should I have
marked it as a reply to your review of the v2 email?

 You can already accomplish the same thing
 by checking the output of $(git cat-file -t $name), but this is a
 natural extension of the other ^{} rules, and I can see making some
 callers more natural.

Exactly.  I updated the commit message to explain this so that other
people know why it might be a good feature to have.

 Can you please add a test (probably in t1511) that checks the behavior,
 similar to what you wrote in the commit message?

Done.  I see by your reply that my fear of going a bit overboard in the
test was justified.  :)  I don't mind rerolling if you'd prefer a
simpler test.

For future reference, is there a preference for putting tests of a new
feature in a separate commit?  In the same commit?  Doesn't really matter?

 diff --git a/sha1_name.c b/sha1_name.c
 index 65ad066..6dc496d 100644
 --- a/sha1_name.c
 +++ b/sha1_name.c
 @@ -679,6 +679,8 @@ static int peel_onion(const char *name, int len, 
 unsigned char *sha1)
  sp++; /* beginning of type name, or closing brace for empty */
  if (!strncmp(commit_type, sp, 6)  sp[6] == '}')
  expected_type = OBJ_COMMIT;
 +else if (!strncmp(tag_type, sp, 3)  sp[3] == '}')
 +expected_type = OBJ_TAG;
 
 This is not a problem you are introducing to this code, but the use of
 opaque constants like commit_type along with the magic number 6
 assuming that it contains commit seems like a maintenance nightmare
 (the only thing saving us is that it will almost certainly never change
 from commit; but then why do we have the opaque type in the first
 place?).

I agree.  I didn't address this in the reroll.

 
 I wonder if we could do better with:
 
   #define COMMIT_TYPE commit
   ...
   if (!strncmp(COMMIT_TYPE, sp, strlen(COMMIT_TYPE))
sp[strlen(COMMIT_TYPE)] == '}')
 
 Any compiler worth its salt will optimize the strlen on a string
 constant into a constant itself. The length makes it a bit less
 readable, though.

True, and I'm not a huge fan of macros.

 
 I wonder if we could do even better with:
 
   const char *x;
   ...
   if ((x = skip_prefix(sp, commit_type))  *x == '}')
 
 which avoids the magic lengths altogether

Not bad, especially since skip_prefix() already exists.

 (though the compiler cannot
 optimize out the strlen call inside skip_prefix, because we declare
 commit_type and friends as an extern.  It probably doesn't matter in
 peel_onion, though, which should not generally be performance critical
 anyway).

Yeah, I can't see performance being a problem there.

There's also this awkward approach, which would avoid strlen() altogether:

commit.h:

extern const char *commit_type;
extern const size_t commit_type_len;

commit.c:

const char commit_type_array[] = commit;
const char *commit_type = commit_type_array[0];
const size_t commit_type_len = sizeof(commit_type_array) - 1;

sha1_name.c peel_onion():

if (!strncmp(commit_type, sp, commit_type_len)
 sp[commit_type_len] == '}')

but I prefer your skip_prefix() suggestion.

-Richard
--
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] peel_onion(): add support for rev^{tag}

2013-09-03 Thread Richard Hansen
On 2013-09-03 15:03, Eric Sunshine wrote:
 On Tue, Sep 3, 2013 at 1:37 PM, Richard Hansen rhan...@bbn.com wrote:
 diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
 index eaefc77..5771cbd 100755
 --- a/t/t1511-rev-parse-caret.sh
 +++ b/t/t1511-rev-parse-caret.sh
 @@ -54,6 +61,26 @@ test_expect_success 'ref^{tree}' '
 test_must_fail git rev-parse blob-tag^{tree}
  '

 +test_expect_success 'ref^{tag}' '
 +   echo $BLOB_TAG_SHA1 expected 
 +   git rev-parse --verify blob-tag^{tag} actual 
 +   test_cmp expected actual 
 +   echo $TREE_TAG_SHA1 expected 
 +   git rev-parse --verify tree-tag^{tag} actual 
 +   test_cmp expected actual 
 +   echo $COMMIT_TAG_SHA1 expected 
 +   git rev-parse --verify commit-tag^{tag} actual 
 +   test_cmp expected actual 
 +   echo $TAG_TAG_SHA1 expected 
 +   git rev-parse --verify tag-tag^{tag} actual 
 +   test_cmp expected actual 
 +   test_must_fail git rev-parse --verify $BLOB_SHA1^{tag} 
 +   test_must_fail git rev-parse --verify $TREE_SHA1^{tag} 
 +   test_must_fail git rev-parse --verify $COMMIT_SHA1^{tag} 
 +   test_must_fail git rev-parse --verify rev^{tag} 
 +   true
 +'
 
 The unnecessary trailing  true is unusual. Such form is not used
 elsewhere in this file, or in any script in the test suite.

True.  I can take it out, and while I'm at it simplify the test case to
what Peff suggested.

I'm in the habit of using that idiom because (1) I won't break things if
I forget to add/remove '' when I add/remove lines in a future commit,
and (2) it simplifies conflict resolution if two commits touch the same
list of stuff.

-Richard
--
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] peel_onion(): add support for rev^{tag}

2013-09-03 Thread Richard Hansen
Complete the rev^{type} family of object descriptors by having
rev^{tag} dereference rev until a tag object is found (or fail if
unable).

At first glance this may not seem very useful, as commits, trees, and
blobs cannot be peeled to a tag, and a tag would just peel to itself.
However, this can be used to ensure that rev names a tag object:

$ git rev-parse --verify v1.8.4^{tag}
04f013dc38d7512eadb915eba22efc414f18b869
$ git rev-parse --verify master^{tag}
error: master^{tag}: expected tag type, but the object dereferences to tree 
type
fatal: Needed a single revision

Users can already ensure that rev is a tag object by checking the
output of 'git cat-file -t rev', but:
  * users may expect rev^{tag} to exist given that rev^{commit},
rev^{tree}, and rev^{blob} all exist
  * this syntax is more convenient/natural in some circumstances

Signed-off-by: Richard Hansen rhan...@bbn.com
---
Changes from v3 (2013-09-03, see
http://thread.gmane.org/gmane.comp.version-control.git/233752):
  * Use Peff's simpler test case.

 Documentation/revisions.txt | 3 +++
 sha1_name.c | 2 ++
 t/t1511-rev-parse-caret.sh  | 7 +++
 3 files changed, 12 insertions(+)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index d477b3f..b3322ad 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -121,6 +121,9 @@ some output processing may assume ref names in UTF-8.
 object that exists, without requiring 'rev' to be a tag, and
 without dereferencing 'rev'; because a tag is already an object,
 it does not have to be dereferenced even once to get to an object.
++
+'rev{caret}\{tag\}' can be used to ensure that 'rev' identifies an
+existing tag object.
 
 'rev{caret}\{\}', e.g. 'v0.99.8{caret}\{\}'::
   A suffix '{caret}' followed by an empty brace pair
diff --git a/sha1_name.c b/sha1_name.c
index 65ad066..6dc496d 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -679,6 +679,8 @@ static int peel_onion(const char *name, int len, unsigned 
char *sha1)
sp++; /* beginning of type name, or closing brace for empty */
if (!strncmp(commit_type, sp, 6)  sp[6] == '}')
expected_type = OBJ_COMMIT;
+   else if (!strncmp(tag_type, sp, 3)  sp[3] == '}')
+   expected_type = OBJ_TAG;
else if (!strncmp(tree_type, sp, 4)  sp[4] == '}')
expected_type = OBJ_TREE;
else if (!strncmp(blob_type, sp, 4)  sp[4] == '}')
diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index eaefc77..15973f2 100755
--- a/t/t1511-rev-parse-caret.sh
+++ b/t/t1511-rev-parse-caret.sh
@@ -54,6 +54,13 @@ test_expect_success 'ref^{tree}' '
test_must_fail git rev-parse blob-tag^{tree}
 '
 
+test_expect_success 'ref^{tag}' '
+   test_must_fail git rev-parse HEAD^{tag} 
+   git rev-parse commit-tag expected 
+   git rev-parse commit-tag^{tag} actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'ref^{/.}' '
git rev-parse master expected 
git rev-parse master^{/.} actual 
-- 
1.8.4

--
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 7/7] glossary: fix and clarify the definition of 'ref'

2013-09-04 Thread Richard Hansen
Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index 3466ce9..7ad13e1 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -395,10 +395,20 @@ should not be combined with other pathspec.
to the result.
 
 [[def_ref]]ref::
-   A 40-byte hex representation of a def_SHA1,SHA-1 or a name that
-   denotes a particular def_object,object. They may be stored in
-   a file under `$GIT_DIR/refs/` directory, or
-   in the `$GIT_DIR/packed-refs` file.
+   A name that begins with `refs/` (e.g. `refs/heads/master`)
+   that points to an def_object_name,object name or another
+   ref (the latter is called a def_symref,symbolic ref).
+   For convenience, a ref can sometimes be abbreviated when used
+   as an argument to a Git command; see linkgit:gitrevisions[7]
+   for details.
+   Refs are stored in the def_repository,repository.
++
+The ref namespace is hierarchical.
+Different subhierarchies are used for different purposes (e.g. the
+`refs/heads/` hierarchy is used to represent local branches).
++
+There are a few special-purpose refs that do not begin with `refs/`.
+The most notable example is `HEAD`.
 
 [[def_reflog]]reflog::
A reflog shows the local history of a ref.  In other words,
-- 
1.8.4

--
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 6/7] revisions.txt: fix and clarify rev^{type}

2013-09-04 Thread Richard Hansen
If possible, rev will be dereferenced even if it is not a tag type
(e.g., commit dereferenced to a tree).

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/revisions.txt | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index d477b3f..b0f4284 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -111,10 +111,14 @@ some output processing may assume ref names in UTF-8.
 
 'rev{caret}\{type\}', e.g. 'v0.99.8{caret}\{commit\}'::
   A suffix '{caret}' followed by an object type name enclosed in
-  brace pair means the object
-  could be a tag, and dereference the tag recursively until an
-  object of that type is found or the object cannot be
-  dereferenced anymore (in which case, barf).  'rev{caret}0'
+  brace pair means dereference the object at 'rev' recursively until
+  an object of type 'type' is found or the object cannot be
+  dereferenced anymore (in which case, barf).
+  For example, if 'rev' is a commit-ish, 'rev{caret}\{commit\}'
+  describes the corresponding commit object.
+  Similarly, if 'rev' is a tree-ish, 'rev{caret}\{tree\}'
+  describes the corresponding tree object.
+  'rev{caret}0'
   is a short-hand for 'rev{caret}\{commit\}'.
 +
 'rev{caret}\{object\}' can be used to make sure 'rev' names an
-- 
1.8.4

--
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/7] documentation cleanups for rev^{type}

2013-09-04 Thread Richard Hansen
On 2013-09-03 18:46, Junio C Hamano wrote:
 I hate to say this after seeing you doing a thorough job in this
 series, but because tree-ish and commit-ish are both made-up
 words, I have a feeling that we are better off unifying to the
 dashed form, which unfortunately is the other way around from what
 your series does.

I thought you might say that so I held on to the commits that
standardize on tree-ish and commit-ish in my local repo.  :)

This series still says tree-ish (also treeish) and commit-ish (also
committish) in gitglossary(7).  Would you like me to eliminate the
(also ...)  part?

I'm not 100% confident that these don't break translations, although
it still builds and make test passes.

Changes since v2:
  * standardize on 'tree-ish' instead of 'treeish'
  * standardize on 'commit-ish' instead of 'committish'

Richard Hansen (7):
  glossary: mention 'treeish' as an alternative to 'tree-ish'
  glossary: define commit-ish (a.k.a. committish)
  use 'tree-ish' instead of 'treeish'
  use 'commit-ish' instead of 'committish'
  glossary: more precise definition of tree-ish (a.k.a. treeish)
  revisions.txt: fix and clarify rev^{type}
  glossary: fix and clarify the definition of 'ref'

 Documentation/RelNotes/1.7.11.2.txt  |  2 +-
 Documentation/config.txt |  4 +--
 Documentation/git-cat-file.txt   |  2 +-
 Documentation/git-describe.txt   | 14 -
 Documentation/git-fast-import.txt| 26 +++
 Documentation/git-merge-tree.txt |  2 +-
 Documentation/git-name-rev.txt   |  2 +-
 Documentation/git-push.txt   |  2 +-
 Documentation/gitcli.txt |  2 +-
 Documentation/glossary-content.txt   | 47 +++-
 Documentation/howto/revert-branch-rebase.txt |  2 +-
 Documentation/revisions.txt  | 12 ---
 builtin/describe.c   |  4 +--
 builtin/merge.c  |  2 +-
 contrib/examples/git-merge.sh|  2 +-
 fast-import.c| 20 ++--
 git-cvsserver.perl   |  2 +-
 po/da.po |  2 +-
 po/de.po |  6 ++--
 po/fr.po |  4 +--
 po/git.pot   |  4 +--
 po/it.po |  2 +-
 po/nl.po |  2 +-
 po/pt_PT.po  |  2 +-
 po/sv.po |  6 ++--
 po/vi.po |  6 ++--
 po/zh_CN.po  |  4 +--
 sha1_name.c  |  6 ++--
 t/t9300-fast-import.sh   |  6 ++--
 test-match-trees.c   |  4 +--
 30 files changed, 118 insertions(+), 83 deletions(-)

-- 
1.8.4

--
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/7] glossary: define commit-ish (a.k.a. committish)

2013-09-04 Thread Richard Hansen
Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index 0273095..47e901e 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -82,6 +82,18 @@ to point at the new commit.
to the top def_directory,directory of the stored
revision.
 
+[[def_commit-ish]]commit-ish (also committish)::
+   A def_commit_object,commit object or an
+   def_object,object that can be recursively dereferenced to
+   a commit object.
+   The following are all commit-ishes:
+   a commit object,
+   a def_tag_object,tag object that points to a commit
+   object,
+   a tag object that points to a tag object that points to a
+   commit object,
+   etc.
+
 [[def_core_git]]core Git::
Fundamental data structures and utilities of Git. Exposes only limited
source code management tools.
-- 
1.8.4

--
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/7] glossary: mention 'treeish' as an alternative to 'tree-ish'

2013-09-04 Thread Richard Hansen
The documentation contains a mix of the two spellings, so include both
in the glossary so that a search for either will lead to the
definition.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index dba5062..0273095 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -486,7 +486,7 @@ should not be combined with other pathspec.
with refs to the associated blob and/or tree objects. A
def_tree,tree is equivalent to a def_directory,directory.
 
-[[def_tree-ish]]tree-ish::
+[[def_tree-ish]]tree-ish (also treeish)::
A def_ref,ref pointing to either a def_commit_object,commit
object, a def_tree_object,tree object, or a def_tag_object,tag
object pointing to a tag or commit or tree object.
-- 
1.8.4

--
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 5/7] glossary: more precise definition of tree-ish (a.k.a. treeish)

2013-09-04 Thread Richard Hansen
A tree-ish isn't a ref.  Also, mention dereferencing, and that a
commit dereferences to a tree, to support gitrevisions(7) and
rev-parse's error messages.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index 47e901e..3466ce9 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -499,9 +499,18 @@ should not be combined with other pathspec.
def_tree,tree is equivalent to a def_directory,directory.
 
 [[def_tree-ish]]tree-ish (also treeish)::
-   A def_ref,ref pointing to either a def_commit_object,commit
-   object, a def_tree_object,tree object, or a def_tag_object,tag
-   object pointing to a tag or commit or tree object.
+   A def_tree_object,tree object or an def_object,object
+   that can be recursively dereferenced to a tree object.
+   Dereferencing a def_commit_object,commit object yields the
+   tree object corresponding to the def_revision,revision's
+   top def_directory,directory.
+   The following are all tree-ishes:
+   a def_commit-ish,commit-ish,
+   a tree object,
+   a def_tag_object,tag object that points to a tree object,
+   a tag object that points to a tag object that points to a tree
+   object,
+   etc.
 
 [[def_unmerged_index]]unmerged index::
An def_index,index which contains unmerged
-- 
1.8.4

--
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/7] use 'commit-ish' instead of 'committish'

2013-09-04 Thread Richard Hansen
Replace 'committish' in documentation and comments with 'commit-ish'
to match gitglossary(7) and to be consistent with 'tree-ish'.

The only remaining instances of 'committish' are:
  * variable, function, and macro names
  * (also committish) in the definition of commit-ish in
gitglossary[7]

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/config.txt |  4 ++--
 Documentation/git-describe.txt   | 14 +++---
 Documentation/git-fast-import.txt| 26 +-
 Documentation/git-name-rev.txt   |  2 +-
 Documentation/git-push.txt   |  2 +-
 Documentation/gitcli.txt |  2 +-
 Documentation/howto/revert-branch-rebase.txt |  2 +-
 builtin/describe.c   |  4 ++--
 builtin/merge.c  |  2 +-
 contrib/examples/git-merge.sh|  2 +-
 fast-import.c| 16 
 po/da.po |  2 +-
 po/de.po |  6 +++---
 po/fr.po |  4 ++--
 po/git.pot   |  4 ++--
 po/it.po |  2 +-
 po/nl.po |  2 +-
 po/pt_PT.po  |  2 +-
 po/sv.po |  6 +++---
 po/vi.po |  6 +++---
 po/zh_CN.po  |  4 ++--
 sha1_name.c  |  6 +++---
 t/t9300-fast-import.sh   |  6 +++---
 23 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..1ccec22 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -170,8 +170,8 @@ advice.*::
pushNeedsForce::
Shown when linkgit:git-push[1] rejects an update that
tries to overwrite a remote ref that points at an
-   object that is not a committish, or make the remote
-   ref point at an object that is not a committish.
+   object that is not a commit-ish, or make the remote
+   ref point at an object that is not a commit-ish.
statusHints::
Show directions on how to proceed from the current
state in the output of linkgit:git-status[1], in
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index 9439cd6..d20ca40 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -9,7 +9,7 @@ git-describe - Show the most recent tag that is reachable from 
a commit
 SYNOPSIS
 
 [verse]
-'git describe' [--all] [--tags] [--contains] [--abbrev=n] committish...
+'git describe' [--all] [--tags] [--contains] [--abbrev=n] commit-ish...
 'git describe' [--all] [--tags] [--contains] [--abbrev=n] --dirty[=mark]
 
 DESCRIPTION
@@ -26,8 +26,8 @@ see the -a and -s options to linkgit:git-tag[1].
 
 OPTIONS
 ---
-committish...::
-   Committish object names to describe.
+commit-ish...::
+   Commit-ish object names to describe.
 
 --dirty[=mark]::
Describe the working tree.
@@ -57,7 +57,7 @@ OPTIONS
 
 --candidates=n::
Instead of considering only the 10 most recent tags as
-   candidates to describe the input committish consider
+   candidates to describe the input commit-ish consider
up to n candidates.  Increasing n above 10 will take
slightly longer but may produce a more accurate result.
An n of 0 will cause only exact matches to be output.
@@ -145,7 +145,7 @@ be sufficient to disambiguate these commits.
 SEARCH STRATEGY
 ---
 
-For each committish supplied, 'git describe' will first look for
+For each commit-ish supplied, 'git describe' will first look for
 a tag which tags exactly that commit.  Annotated tags will always
 be preferred over lightweight tags, and tags with newer dates will
 always be preferred over tags with older dates.  If an exact match
@@ -154,12 +154,12 @@ is found, its name will be output and searching will stop.
 If an exact match was not found, 'git describe' will walk back
 through the commit history to locate an ancestor commit which
 has been tagged.  The ancestor's tag will be output along with an
-abbreviation of the input committish's SHA-1. If '--first-parent' was
+abbreviation of the input commit-ish's SHA-1. If '--first-parent' was
 specified then the walk will only consider the first parent of each
 commit.
 
 If multiple tags were found during the walk then the tag which
-has the fewest commits different from the input committish will be
+has the fewest commits different from the input commit-ish will be
 selected and output.  Here fewest commits different is defined as
 the number of commits which would be shown by `git log tag..input

[PATCH v3 3/7] use 'tree-ish' instead of 'treeish'

2013-09-04 Thread Richard Hansen
Replace 'treeish' in documentation and comments with 'tree-ish' to
match gitglossary(7).

The only remaining instances of 'treeish' are:
  * variable, function, and macro names
  * (also treeish) in the definition of tree-ish in gitglossary(7)

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/RelNotes/1.7.11.2.txt | 2 +-
 Documentation/git-cat-file.txt  | 2 +-
 Documentation/git-merge-tree.txt| 2 +-
 fast-import.c   | 4 ++--
 git-cvsserver.perl  | 2 +-
 test-match-trees.c  | 4 ++--
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/RelNotes/1.7.11.2.txt 
b/Documentation/RelNotes/1.7.11.2.txt
index a0d24d1..f0cfd02 100644
--- a/Documentation/RelNotes/1.7.11.2.txt
+++ b/Documentation/RelNotes/1.7.11.2.txt
@@ -31,7 +31,7 @@ Fixes since v1.7.11.1
  * git diff --no-index did not work with pagers correctly.
 
  * git diff COPYING HEAD:COPYING gave a nonsense error message that
-   claimed that the treeish HEAD did not have COPYING in it.
+   claimed that the tree-ish HEAD did not have COPYING in it.
 
  * When git log gets --simplify-merges/by-decoration together with
--first-parent, the combination of these options makes the
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 10fbc6a..e468ceb 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -54,7 +54,7 @@ OPTIONS
 
 --textconv::
Show the content as transformed by a textconv filter. In this case,
-   object has be of the form treeish:path, or :path in order
+   object has be of the form tree-ish:path, or :path in order
to apply the filter to the content recorded in the index at path.
 
 --batch::
diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
index c5f84b6..58731c1 100644
--- a/Documentation/git-merge-tree.txt
+++ b/Documentation/git-merge-tree.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-Reads three treeish, and output trivial merge results and
+Reads three tree-ish, and output trivial merge results and
 conflicting stages to the standard output.  This is similar to
 what three-way 'git read-tree -m' does, but instead of storing the
 results in the index, the command outputs the entries to the
diff --git a/fast-import.c b/fast-import.c
index 23f625f..019be11 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2957,7 +2957,7 @@ static struct object_entry *dereference(struct 
object_entry *oe,
case OBJ_TAG:
break;
default:
-   die(Not a treeish: %s, command_buf.buf);
+   die(Not a tree-ish: %s, command_buf.buf);
}
 
if (oe-pack_id != MAX_PACK_ID) {   /* in a pack being written */
@@ -3041,7 +3041,7 @@ static void parse_ls(struct branch *b)
struct tree_entry *root = NULL;
struct tree_entry leaf = {NULL};
 
-   /* ls SP (treeish SP)? path */
+   /* ls SP (tree-ish SP)? path */
p = command_buf.buf + strlen(ls );
if (*p == '') {
if (!b)
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index a0d796e..a9f6f8e 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -4338,7 +4338,7 @@ sub getAnyHead
 =head2 getRevisionDirMap
 
 A revision dir map contains all the plain-file filenames associated
-with a particular revision (treeish), organized by directory:
+with a particular revision (tree-ish), organized by directory:
 
   $type = $out-{$dir}{$fullName}
 
diff --git a/test-match-trees.c b/test-match-trees.c
index a3c4688..2ef725e 100644
--- a/test-match-trees.c
+++ b/test-match-trees.c
@@ -12,10 +12,10 @@ int main(int ac, char **av)
die(cannot parse %s as an object name, av[2]);
one = parse_tree_indirect(hash1);
if (!one)
-   die(not a treeish %s, av[1]);
+   die(not a tree-ish %s, av[1]);
two = parse_tree_indirect(hash2);
if (!two)
-   die(not a treeish %s, av[2]);
+   die(not a tree-ish %s, av[2]);
 
shift_tree(one-object.sha1, two-object.sha1, shifted, -1);
printf(shifted: %s\n, sha1_to_hex(shifted));
-- 
1.8.4

--
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/3] Reject non-ff pulls by default

2013-09-05 Thread Richard Hansen
On 2013-09-04 18:59, Junio C Hamano wrote:
 Philip Oakley philipoak...@iee.org writes:
 
 From: Junio C Hamano gits...@pobox.com
 John Keeping j...@keeping.me.uk writes:

 I think there are two distinct uses for pull, which boil down to:

 (1) git pull
 ...
 Peff already covered (1)---it is highly doubtful that a merge is
 almost always wrong.  In fact, if that _were_ the case, we should
 simply be defaulting to rebase, not failing the command and asking
 between merge and rebase like jc/pull-training-wheel topic did.

 We simply do not know what the user wants, as it heavily depends on
 the project, so we ask the user to choose one (and stick to it).

 We only offer a limited list. It won't be sufficient for all use
 cases. It wasn't for me.
 
 Very interesting. Tell us more.

I'm a bit late to the discussion, but I wanted to chime in.  I detest
'git pull' and discourage everyone I meet from using it.  See:
http://stackoverflow.com/questions/15316601/why-is-git-pull-considered-harmful
for my reasons.

Instead, I encourage people to do this:

   git config --global alias.up '!git remote update -p; git merge
--ff-only @{u}'

and tell them to run 'git up' whenever they would be tempted to use a
plain 'git pull'.

I usually work with a central repository with topic branches.  I follow
this rule of thumb:
  * When merging a same-named branch (e.g., origin/foo into foo, foo
into origin/foo), it should always be a fast-forward.  This may
require rebasing.
  * When merging a differently-named branch (e.g., feature.xyz into
master), it should never be a fast-forward.

In distributed workflows, I think of 'git pull collaborator-repo
their-branch' as merging a differently-named branch (I wouldn't be
merging if they hadn't told me that a separate feature they were working
on is complete), so I generally want the merge commit.  But when I do a
'git pull' without extra arguments, I'm updating a same-named branch so
I never want a merge.

When merging a differently-named branch, I prefer the merge --no-ff to
be preceded by a rebase to get a nice, pretty graph:

   * merge feature.xyz  - master
   |\
   | * xyz part 3/3
   | * xyz part 2/3
   | * xyz part 1/3
   |/
   * merge feature.foo
   |\
   | * foo part 2/2
   | * foo part 1/2
   |/
   * merge feature.bar
   |\
   ...

The explicit merge has several benefits:
  * It clearly communicates to others that the feature is done.
  * It makes it easier to revert the entire feature by reverting the
merge if necessary.
  * It allows our continuous integration tool to skip over the
work-in-progress commits and test only complete features.
  * It makes it easier to review the entire feature in one diff.
  * 'git log --first-parent' shows a high-level summary of the changes
over time, while a normal 'git log' shows the details.

 
 When git pull stops because what was fetched in FETCH_HEAD does
 not fast-forward, then what did _you_ do (and with the knowledge you
 currently have, what would you do)?

I stop and review what's going on, then make a decision:
  * usually it's a rebase
  * sometimes it's a rebase --onto (because the branch was
force-updated to undo a particularly bad commit)
  * sometimes it's a rebase -p (because there's an explicit merge of a
different branch that I want to keep)
  * sometimes it's a reset --hard (my changes were made obsolete by a
different upstream change)
  * sometimes it's a merge
  * sometimes I do nothing.  This is a fairly regular pattern:  I'm in
the middle of working on something that I know will conflict with
some changes that were just pushed upstream, and I want to finish
my changes before starting the rebase.  My collaborator contacts me
and asks, Would you take a look at the changes I just pushed?  If
I type 'git pull' out of habit to get the commits, then I'll make a
mess of my work-in-progress work tree.  If I type 'git up' out of
habit, then the merge --ff-only will fail as expected and I can
quickly review the commits without messing with my work tree or
HEAD.

Even if I always rebase or always merge, I want to briefly review what
changed in the remote branch *before* I start the rebase.  This helps me
understand the conflicts I might encounter.

Thus, ff-only always works for me.  I might have to type a second merge
or rebase command, but that's OK -- it gives me an opportunity to think
about what I want first.  Non-ff merges are rare enough that the
interruption isn't annoying at all.

 In a single project, would you
 choose to sometimes rebase and sometimes merge, and if so, what is
 the choice depend on?  When I am on these selected branches, I want
 to merge, but on other branches I want to rebase?

My choice depends on the circumstances of the divergence.  It's never as
simple as branch X always has this policy while branch Y has that policy.

 Are there cases where you do not want to either 

[PATCH] remote-bzr: reuse bzrlib transports when possible

2013-09-07 Thread Richard Hansen
Pass a list of open bzrlib.transport.Transport objects to each bzrlib
function that might create a transport.  This enables bzrlib to reuse
existing transports when possible, avoiding multiple concurrent
connections to the same remote server.

If the remote server is accessed via ssh, this fixes a couple of
problems:
  * If the user does not have keys loaded into an ssh agent, the user
may be prompted for a password multiple times.
  * If the user is using OpenSSH and the ControlMaster setting is set
to auto, git-remote-bzr might hang.  This is because bzrlib closes
the multiple ssh sessions in an undefined order and might try to
close the master ssh session before the other sessions.  The
master ssh process will not exit until the other sessions have
exited, causing a deadlock.  (The ssh sessions are closed in an
undefined order because bzrlib relies on the Python garbage
collector to trigger ssh session termination.)
---
 contrib/remote-helpers/git-remote-bzr | 33 +
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr 
b/contrib/remote-helpers/git-remote-bzr
index c3a3cac..1e0044b 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -674,7 +674,7 @@ def parse_reset(parser):
 parsed_refs[ref] = mark_to_rev(from_mark)
 
 def do_export(parser):
-global parsed_refs, dirname
+global parsed_refs, dirname, transports
 
 parser.next()
 
@@ -699,7 +699,8 @@ def do_export(parser):
 branch.generate_revision_history(revid, marks.get_tip(name))
 
 if name in peers:
-peer = bzrlib.branch.Branch.open(peers[name])
+peer = bzrlib.branch.Branch.open(peers[name],
+ 
possible_transports=transports)
 try:
 peer.bzrdir.push_branch(branch, revision_id=revid)
 except bzrlib.errors.DivergedBranches:
@@ -769,25 +770,28 @@ def do_list(parser):
 print
 
 def clone(path, remote_branch):
+global transports
 try:
-bdir = bzrlib.bzrdir.BzrDir.create(path)
+bdir = bzrlib.bzrdir.BzrDir.create(path, 
possible_transports=transports)
 except bzrlib.errors.AlreadyControlDirError:
-bdir = bzrlib.bzrdir.BzrDir.open(path)
+bdir = bzrlib.bzrdir.BzrDir.open(path, possible_transports=transports)
 repo = bdir.find_repository()
 repo.fetch(remote_branch.repository)
 return remote_branch.sprout(bdir, repository=repo)
 
 def get_remote_branch(name):
-global dirname, branches
+global dirname, branches, transports
 
-remote_branch = bzrlib.branch.Branch.open(branches[name])
+remote_branch = bzrlib.branch.Branch.open(branches[name],
+  possible_transports=transports)
 if isinstance(remote_branch.user_transport, 
bzrlib.transport.local.LocalTransport):
 return remote_branch
 
 branch_path = os.path.join(dirname, 'clone', name)
 
 try:
-branch = bzrlib.branch.Branch.open(branch_path)
+branch = bzrlib.branch.Branch.open(branch_path,
+   possible_transports=transports)
 except bzrlib.errors.NotBranchError:
 # clone
 branch = clone(branch_path, remote_branch)
@@ -821,17 +825,19 @@ def find_branches(repo):
 yield name, branch.base
 
 def get_repo(url, alias):
-global dirname, peer, branches
+global dirname, peer, branches, transports
 
 normal_url = bzrlib.urlutils.normalize_url(url)
-origin = bzrlib.bzrdir.BzrDir.open(url)
+origin = bzrlib.bzrdir.BzrDir.open(url, possible_transports=transports)
 is_local = isinstance(origin.transport, 
bzrlib.transport.local.LocalTransport)
 
 shared_path = os.path.join(gitdir, 'bzr')
 try:
-shared_dir = bzrlib.bzrdir.BzrDir.open(shared_path)
+shared_dir = bzrlib.bzrdir.BzrDir.open(shared_path,
+   possible_transports=transports)
 except bzrlib.errors.NotBranchError:
-shared_dir = bzrlib.bzrdir.BzrDir.create(shared_path)
+shared_dir = bzrlib.bzrdir.BzrDir.create(shared_path,
+ 
possible_transports=transports)
 try:
 shared_repo = shared_dir.open_repository()
 except bzrlib.errors.NoRepositoryPresent:
@@ -844,7 +850,8 @@ def get_repo(url, alias):
 else:
 # check and remove old organization
 try:
-bdir = bzrlib.bzrdir.BzrDir.open(clone_path)
+bdir = bzrlib.bzrdir.BzrDir.open(clone_path,
+ 
possible_transports=transports)
 bdir.destroy_repository()
 except bzrlib.errors.NotBranchError:
 pass
@@ -897,6 +904,7 @@ def main(args):
 global files_cache

Re: [PATCH] remote-bzr: reuse bzrlib transports when possible

2013-09-07 Thread Richard Hansen
On 2013-09-07 19:58, Richard Hansen wrote:
 Pass a list of open bzrlib.transport.Transport objects to each bzrlib
 function that might create a transport.  This enables bzrlib to reuse
 existing transports when possible, avoiding multiple concurrent
 connections to the same remote server.
 
 If the remote server is accessed via ssh, this fixes a couple of
 problems:
   * If the user does not have keys loaded into an ssh agent, the user
 may be prompted for a password multiple times.
   * If the user is using OpenSSH and the ControlMaster setting is set
 to auto, git-remote-bzr might hang.  This is because bzrlib closes
 the multiple ssh sessions in an undefined order and might try to
 close the master ssh session before the other sessions.  The
 master ssh process will not exit until the other sessions have
 exited, causing a deadlock.  (The ssh sessions are closed in an
 undefined order because bzrlib relies on the Python garbage
 collector to trigger ssh session termination.)

I forgot to mention:  I didn't add a Signed-off-by line because there is
no mention of a copyright license at the top of git-remote-bzr.

-Richard
--
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] remote-bzr: reuse bzrlib transports when possible

2013-09-07 Thread Richard Hansen
On 2013-09-07 20:30, Felipe Contreras wrote:
 On Sat, Sep 7, 2013 at 7:02 PM, Richard Hansen rhan...@bbn.com wrote:
 On 2013-09-07 19:58, Richard Hansen wrote:
 Pass a list of open bzrlib.transport.Transport objects to each bzrlib
 function that might create a transport.  This enables bzrlib to reuse
 existing transports when possible, avoiding multiple concurrent
 connections to the same remote server.

 If the remote server is accessed via ssh, this fixes a couple of
 problems:
   * If the user does not have keys loaded into an ssh agent, the user
 may be prompted for a password multiple times.
   * If the user is using OpenSSH and the ControlMaster setting is set
 to auto, git-remote-bzr might hang.  This is because bzrlib closes
 the multiple ssh sessions in an undefined order and might try to
 close the master ssh session before the other sessions.  The
 master ssh process will not exit until the other sessions have
 exited, causing a deadlock.  (The ssh sessions are closed in an
 undefined order because bzrlib relies on the Python garbage
 collector to trigger ssh session termination.)

 I forgot to mention:  I didn't add a Signed-off-by line because there is
 no mention of a copyright license at the top of git-remote-bzr.
 
 And why is that relevant? A signed-off-by line means you wrote the
 code and you are fine with the patch being applied, or you are
 responsible somehow.

The Developer's Certificate of Origin 1.1 in
Documentation/SubmittingPatches refers to the open source license
indicated in the file, but there is no such indication.

However, it looks like most of the files in the repository don't
indicate which license applies.  So I guess the license in the COPYING
file (GPLv2) applies by default?  I'll re-send with the Signed-off-by
line.

Perhaps SubmittingPatches should be updated to clarify what happens if
the file doesn't indicate which license applies to the file (see
example below).

-Richard


diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 7055576..c5ff744 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -227,13 +227,15 @@ the patch, which certifies that you wrote it or otherwise 
have
 the right to pass it on as a open-source patch.  The rules are
 pretty simple: if you can certify the below:

-Developer's Certificate of Origin 1.1
+Developer's Certificate of Origin 1.2

 By making a contribution to this project, I certify that:

 (a) The contribution was created in whole or in part by me and I
 have the right to submit it under the open source license
-indicated in the file; or
+indicated in the file (or, if no license is indicated in
+the file, the license in COPYING that accompanies the
+file); or

 (b) The contribution is based upon previous work that, to the best
 of my knowledge, is covered under an appropriate open source
@@ -241,7 +243,8 @@ pretty simple: if you can certify the below:
 work with modifications, whether created in whole or in part
 by me, under the same open source license (unless I am
 permitted to submit under a different license), as indicated
-in the file; or
+in the file (or, if no license is indicated in the file,
+the license in COPYING that accompanies the file); or

 (c) The contribution was provided directly to me by some other
 person who certified (a), (b) or (c) and I have not modified
--
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] remote-bzr: reuse bzrlib transports when possible

2013-09-07 Thread Richard Hansen
Pass a list of open bzrlib.transport.Transport objects to each bzrlib
function that might create a transport.  This enables bzrlib to reuse
existing transports when possible, avoiding multiple concurrent
connections to the same remote server.

If the remote server is accessed via ssh, this fixes a couple of
problems:
  * If the user does not have keys loaded into an ssh agent, the user
may be prompted for a password multiple times.
  * If the user is using OpenSSH and the ControlMaster setting is set
to auto, git-remote-bzr might hang.  This is because bzrlib closes
the multiple ssh sessions in an undefined order and might try to
close the master ssh session before the other sessions.  The
master ssh process will not exit until the other sessions have
exited, causing a deadlock.  (The ssh sessions are closed in an
undefined order because bzrlib relies on the Python garbage
collector to trigger ssh session termination.)

Signed-off-by: Richard Hansen rhan...@bbn.com
---
Changes from v1:
  * add Signed-off-by line

 contrib/remote-helpers/git-remote-bzr | 33 +
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr 
b/contrib/remote-helpers/git-remote-bzr
index c3a3cac..1e0044b 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -674,7 +674,7 @@ def parse_reset(parser):
 parsed_refs[ref] = mark_to_rev(from_mark)
 
 def do_export(parser):
-global parsed_refs, dirname
+global parsed_refs, dirname, transports
 
 parser.next()
 
@@ -699,7 +699,8 @@ def do_export(parser):
 branch.generate_revision_history(revid, marks.get_tip(name))
 
 if name in peers:
-peer = bzrlib.branch.Branch.open(peers[name])
+peer = bzrlib.branch.Branch.open(peers[name],
+ 
possible_transports=transports)
 try:
 peer.bzrdir.push_branch(branch, revision_id=revid)
 except bzrlib.errors.DivergedBranches:
@@ -769,25 +770,28 @@ def do_list(parser):
 print
 
 def clone(path, remote_branch):
+global transports
 try:
-bdir = bzrlib.bzrdir.BzrDir.create(path)
+bdir = bzrlib.bzrdir.BzrDir.create(path, 
possible_transports=transports)
 except bzrlib.errors.AlreadyControlDirError:
-bdir = bzrlib.bzrdir.BzrDir.open(path)
+bdir = bzrlib.bzrdir.BzrDir.open(path, possible_transports=transports)
 repo = bdir.find_repository()
 repo.fetch(remote_branch.repository)
 return remote_branch.sprout(bdir, repository=repo)
 
 def get_remote_branch(name):
-global dirname, branches
+global dirname, branches, transports
 
-remote_branch = bzrlib.branch.Branch.open(branches[name])
+remote_branch = bzrlib.branch.Branch.open(branches[name],
+  possible_transports=transports)
 if isinstance(remote_branch.user_transport, 
bzrlib.transport.local.LocalTransport):
 return remote_branch
 
 branch_path = os.path.join(dirname, 'clone', name)
 
 try:
-branch = bzrlib.branch.Branch.open(branch_path)
+branch = bzrlib.branch.Branch.open(branch_path,
+   possible_transports=transports)
 except bzrlib.errors.NotBranchError:
 # clone
 branch = clone(branch_path, remote_branch)
@@ -821,17 +825,19 @@ def find_branches(repo):
 yield name, branch.base
 
 def get_repo(url, alias):
-global dirname, peer, branches
+global dirname, peer, branches, transports
 
 normal_url = bzrlib.urlutils.normalize_url(url)
-origin = bzrlib.bzrdir.BzrDir.open(url)
+origin = bzrlib.bzrdir.BzrDir.open(url, possible_transports=transports)
 is_local = isinstance(origin.transport, 
bzrlib.transport.local.LocalTransport)
 
 shared_path = os.path.join(gitdir, 'bzr')
 try:
-shared_dir = bzrlib.bzrdir.BzrDir.open(shared_path)
+shared_dir = bzrlib.bzrdir.BzrDir.open(shared_path,
+   possible_transports=transports)
 except bzrlib.errors.NotBranchError:
-shared_dir = bzrlib.bzrdir.BzrDir.create(shared_path)
+shared_dir = bzrlib.bzrdir.BzrDir.create(shared_path,
+ 
possible_transports=transports)
 try:
 shared_repo = shared_dir.open_repository()
 except bzrlib.errors.NoRepositoryPresent:
@@ -844,7 +850,8 @@ def get_repo(url, alias):
 else:
 # check and remove old organization
 try:
-bdir = bzrlib.bzrdir.BzrDir.open(clone_path)
+bdir = bzrlib.bzrdir.BzrDir.open(clone_path,
+ 
possible_transports=transports)
 bdir.destroy_repository()
 except

Re: [PATCH 0/3] Reject non-ff pulls by default

2013-09-08 Thread Richard Hansen
On 2013-09-07 22:41, Felipe Contreras wrote:
 On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano gits...@pobox.com wrote:
 
 Which can be solved by adding the above fail option, and then
 renaming them to pull.integrate and branch.name.integrate to
 clarify what these variables are about (it is no longer do you
 rebase or not---if you choose not to rebase, by definition you are
 going to merge, as there is a third choice to fail), while
 retaining pull.rebase and branch.name.rebase as a deprecated
 synonym.
 
 All these names are completely unintuitive. First of all, why
 integrate? Integrate what to what? And then, why fail? Fail on
 what circumstances? Always?
 
 My proposal that does:
 
   pull.mode = merge/rebase/merge-ff-only
 
 Is way more intuitive.

+1

What about something like:

pull.mergeoptions (defaults to --ff-only)
pull.rebaseoptions (defaults to empty?  --preserve-merges?)
branch.name.pull.mergeoptions (defaults to pull.mergeoptions)
branch.name.pull.rebaseoptions (defaults to pull.rebaseoptions)

-Richard
--
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/3] Reject non-ff pulls by default

2013-09-08 Thread Richard Hansen
On 2013-09-08 14:10, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:
 What about something like:

 pull.mergeoptions (defaults to --ff-only)
 pull.rebaseoptions (defaults to empty?  --preserve-merges?)
 branch.name.pull.mergeoptions (defaults to pull.mergeoptions)
 branch.name.pull.rebaseoptions (defaults to pull.rebaseoptions)
 
[snip]
 But it does not help Philip's case, if I understand correctly, where
 running git pull on some branches is always a mistake and the user
 wants it to stop at fetch the history and objects needed to
 complete the history from the other side phase without proceeding
 to the then integrate the history from the other side and the
 history of your branch into one step, which may be done with either
 merge or rebase.

How about:

branch.name.pull.defaultIntegrationMode = merge | rebase | none
If 'merge', pull acts like 'git pull --merge' by default,
merging the other commits into this branch.
If 'rebase', pull acts like 'git pull --rebase' by default,
rebasing this branch onto the other commits.
If 'none', pull acts like 'git fetch' by default.
Default: whatever pull.defaultIntegrationMode is set to.

branch.name.pull.mergeoptions
Arguments to pass to 'git merge' during the merge phase of
'git pull --merge'.
Default: whatever pull.mergeoptions is set to.

branch.name.pull.rebaseoptions
Arguments to pass to 'git rebase' during the rebase phase of
'git pull --rebase'.
Default: whatever pull.rebaseoptions is set to.

pull.defaultIntegrationMode = rebase | merge | none
See branch.name.pull.defaultIntegrationMode.
Default: merge

pull.mergeoptions
See branch.name.pull.mergeoptions.
Default: empty, but warn that a future version will change
this to --ff-only.

pull.rebaseoptions
See branch.name.pull.rebaseoptions.
Default: empty, but warn that a future version will change
this to --preserve-merges?

There's probably a better alternative to the term 'defaultIntegrationMode'.

We could even add a defaultIntegrationMode = merge-there that
reverses the parent order (the other commits become the first parent,
the current branch becomes the second parent).

-Richard
--
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 1/5] pull: rename pull.rename to pull.mode

2013-09-09 Thread Richard Hansen
On 2013-09-08 21:23, Felipe Contreras wrote:
 The old configurations still work, but get deprecated.

Should some tests for the deprecated configs be added?  We wouldn't want
to accidentally break those.

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index ec57a15..9489a59 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -760,11 +760,11 @@ branch.name.mergeoptions::
   option values containing whitespace characters are currently not
   supported.
  
 -branch.name.rebase::
 - When true, rebase the branch name on top of the fetched branch,
 - instead of merging the default branch from the default remote when
 - git pull is run. See pull.rebase for doing this in a non
 - branch-specific manner.
 +branch.name.pullmode::
 + When git pull is run, this determines if it would either merge or
 + rebase the fetched branch. The possible values are 'merge' and
 + 'rebase'. See pull.mode for doing this in a non branch-specific
 + manner.
  +
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]
 @@ -1820,11 +1820,11 @@ pretty.name::
   Note that an alias with the same name as a built-in format
   will be silently ignored.
  
 -pull.rebase::
 - When true, rebase branches on top of the fetched branch, instead
 - of merging the default branch from the default remote when git
 - pull is run. See branch.name.rebase for setting this on a
 - per-branch basis.
 +pull.mode::
 + When git pull is run, this determines if it would either merge or
 + rebase the fetched branch. The possible values are 'merge' and
 + 'rebase'. See branch.name.pullmode for doing this in a non
 + branch-specific manner.
  +
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]

Somewhere something should mention what the default values are
(branch.name.pullmode defaults to pull.mode and pull.mode defaults to
merge).

 diff --git a/git-pull.sh b/git-pull.sh
 index f0df41c..de57c1d 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -43,10 +43,24 @@ log_arg= verbosity= progress= recurse_submodules= 
 verify_signatures=
  merge_args= edit=
  curr_branch=$(git symbolic-ref -q HEAD)
  curr_branch_short=${curr_branch#refs/heads/}
 -rebase=$(git config --bool branch.$curr_branch_short.rebase)
 +mode=$(git config branch.${curr_branch_short}.pullmode)
 +if test -z $mode
 +then
 + mode=$(git config pull.mode)
 +fi
 +test $mode == 'rebase'  rebase=true
  if test -z $rebase
  then
 - rebase=$(git config --bool pull.rebase)
 + rebase=$(git config --bool branch.$curr_branch_short.rebase)
 + if test -z $rebase
 + then
 + rebase=$(git config --bool pull.rebase)
 + fi
 + if test $rebase = 'true'
 + then
 + echo The configurations pull.rebase and branch.name.rebase 
 are deprecated.
 + echo Please use pull.mode and branch.name.pullmode instead.
 + fi
  fi
  dry_run=
  while :

These deprecation warning messages should be written to stderr, and
should probably be prefixed with WARNING: .

-Richard
--
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/3] Reject non-ff pulls by default

2013-09-09 Thread Richard Hansen
On 2013-09-09 16:44, Jeff King wrote:
 On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote:
 I think we need to make sure that we give instructions for how to go
 back if the default hasn't done what you wanted.  Something like this:

 Your pull did not fast-forward, so Git has merged '$upstream' into
 your branch, which may not be correct for your project.  If you
 would rather rebase your changes, run

 git rebase

 See pull.mode in git-config(1) to suppress this message in the
 future.
 
 Yes, that's a good point. I don't know if just git rebase is the right
 advice, though; it would depend on whether we were actually pulling from
 the upstream or not.

Another reason 'git rebase' might not be the right advice:  We don't
want to encourage users to flatten intentional merges.  For example:

$ git checkout master
$ git merge --no-ff just-finished-feature-branch
$ git push
 ! [rejected]master - master (non-fast-forward)
$ git pull
WARNING: Your pull did not fast-forward [...] run git rebase

If 'git rebase' is run here, the commits on just-finished-feature-branch
will be linearized onto @{u}, which is not what the user wants.

Perhaps one could argue that a user that gets into this situation and is
normally comfortable running 'git rebase' is already experienced enough
to know to ignore the advice to run 'git rebase'.

(Sidenote:  Unfortunately there's not an easy way to recover from this
case without adding another merge commit.  But that's a topic for
another thread.)

-Richard
--
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] remote-bzr: reuse bzrlib transports when possible

2013-09-12 Thread Richard Hansen
On 2013-09-10 18:01, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
 Richard Hansen rhan...@bbn.com writes:

  def do_export(parser):
 -global parsed_refs, dirname
 +global parsed_refs, dirname, transports

 As this has been acked by Felipe who knows the script the best, I'll
 apply this directly to 'master'.

 These additions of global transports however have trivial
 interactions with fc/contrib-bzr-hg-fixes topic Felipe posted
 earlier, which I was planning to start merging down to 'next' and
 then to 'master'.  Most funcions merely use the variable without
 assigning, so global transports can be removed, in line with the
 spirit of 641a2b5b (remote-helpers: cleanup more global variables,
 2013-08-28), except for the obvious initialisation in main(), I
 think.  Please double check the conflict resolution result in a
 commit on 'pu' with

 git show 'origin/pu^{/Merge fc/contrib-bzr}'

 when I push the result out.

 Thanks.
 
 Ping?  I'd like to merge fc/contrib-bzr.hg-fixes topic to 'next'
 (and fast track it to 'master' after that), and it would be helpful
 to get an Ack on the conflict resolution I have.

Sorry for the delay.

Looks good to me, and the tests still pass.

-Richard

--
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/RFC] Developer's Certificate of Origin: default to COPYING

2013-09-12 Thread Richard Hansen
The Developer's Certificate of Origin refers to the open source
license indicated in the file, but there is no such indication in
most files in the Git repository.

Update the text to indicate that the license in COPYING should be
assumed if a file doesn't excplicitly indicate which license applies
to the file.

The phrase accompanies the file was chosen to support different
default licenses in different subdirectories (e.g., 2-clause BSD for
vcs-svn/*, LGPL2.1+ for xdiff/*).

Signed-off-by: Richard Hansen rhan...@bbn.com
---
I'm bringing this up because, to this layman's eyes, it seems like a
potentially troublesome oversight.  IIUC, one of the purposes of the
Developer's Certificate of Origin is to make it easy for developers to
declare which license covers a contribution.  Requiring a license
declaration protects the project and its users from copyright
litigation.

What happens if the file(s) being modified do not indicate which
license applies to the file?  Is there no license?  Does it default to
the main project license in COPYING?  This lack of clarity makes me a
bit nervous (law is already too nondeterministic for my liking), so
I'd like to see a change that makes it explicit.

Notes:
  * I am not a lawyer.  (Maybe a lawyer should be consulted?)
  * This change might not be necessary.
  * This change might be wrong.
  * I hope I'm not just wasting everyone's time by bringing this up.

 Documentation/SubmittingPatches | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 7055576..c5ff744 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -227,13 +227,15 @@ the patch, which certifies that you wrote it or otherwise 
have
 the right to pass it on as a open-source patch.  The rules are
 pretty simple: if you can certify the below:
 
-Developer's Certificate of Origin 1.1
+Developer's Certificate of Origin 1.2
 
 By making a contribution to this project, I certify that:
 
 (a) The contribution was created in whole or in part by me and I
 have the right to submit it under the open source license
-indicated in the file; or
+indicated in the file (or, if no license is indicated in
+the file, the license in COPYING that accompanies the
+file); or
 
 (b) The contribution is based upon previous work that, to the best
 of my knowledge, is covered under an appropriate open source
@@ -241,7 +243,8 @@ pretty simple: if you can certify the below:
 work with modifications, whether created in whole or in part
 by me, under the same open source license (unless I am
 permitted to submit under a different license), as indicated
-in the file; or
+in the file (or, if no license is indicated in the file,
+the license in COPYING that accompanies the file); or
 
 (c) The contribution was provided directly to me by some other
 person who certified (a), (b) or (c) and I have not modified
-- 
1.8.4

--
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/RFC] Developer's Certificate of Origin: default to COPYING

2013-09-12 Thread Richard Hansen
On 2013-09-12 18:44, Linus Torvalds wrote:
 On Thu, Sep 12, 2013 at 3:30 PM, Junio C Hamano gits...@pobox.com wrote:
 Linus, this is not limited to us, so I am bothering you; sorry about
 that.

 My instinct tells me that some competent lawyers at linux-foundation
 helped you with the wording of DCO, and we amateurs shouldn't be
 mucking with the text like this patch does at all, but just in case
 you might find it interesting...
 
 There were lawyers involved, yes.
 
 I'm not sure there is any actual confusion, because the fact is,
 lawyers aren't robots or programmers, and they have the human
 qualities of understanding implications.

Well stated.  :)

 So I'm actually inclined to
 not change legal text unless a lawyer actually tells me that it's
 needed.

Is it worthwhile to poke a lawyer about this as a precaution?  (If so,
who?)  Or do we wait for a motivating event?

 
 Plus even if this change was needed, why would anybody point to
 COPYING. It's much better to just say the copyright license of the
 file, knowing that different projects have different rules about this
 all, and some projects mix files from different sources, where parts
 of the tree may be under different licenses that may be explained
 elsewhere..

I agree that your phrasing is better.

-Richard
--
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] peel_onion(): add support for rev^{tag}

2013-06-18 Thread Richard Hansen
gitrevisions(7) implies that rev^{tag} should work, but before now
it did not:

$ git rev-parse --verify v1.8.3.1^{tag}
fatal: Needed a single revision

This commit fixes the omission:

$ git rev-parse --verify v1.8.3.1^{tag}
4bc068950abec778a02ebf3ee73cf0735affabb4

Other object type names work as expected:

$ git rev-parse --verify v1.8.3.1^{commit}
362de916c06521205276acb7f51c99f47db94727
$ git rev-parse --verify v1.8.3.1^{tree}
c1fef48675edd74e9af19405339e8bdaebd56b0c
$ git rev-parse --verify v1.8.3.1^{blob}
error: v1.8.3.1^{blob}: expected blob type, but the object dereferences to 
tree type
fatal: Needed a single revision

Note that rev^{tag} is not the same as rev^{object} when rev is
not a tag:

$ git rev-parse --verify v1.8.3.1^{}^{object}
362de916c06521205276acb7f51c99f47db94727
$ git rev-parse --verify v1.8.3.1^{}^{tag}
error: v1.8.3.1^{}^{tag}: expected tag type, but the object dereferences to 
tree type
fatal: Needed a single revision

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 sha1_name.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sha1_name.c b/sha1_name.c
index 90419ef..68fd0e4 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -677,6 +677,8 @@ static int peel_onion(const char *name, int len, unsigned 
char *sha1)
sp++; /* beginning of type name, or closing brace for empty */
if (!strncmp(commit_type, sp, 6)  sp[6] == '}')
expected_type = OBJ_COMMIT;
+   else if (!strncmp(tag_type, sp, 3)  sp[3] == '}')
+   expected_type = OBJ_TAG;
else if (!strncmp(tree_type, sp, 4)  sp[4] == '}')
expected_type = OBJ_TREE;
else if (!strncmp(blob_type, sp, 4)  sp[4] == '}')
-- 
1.8.3.1

--
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] glossary: define committish (a.k.a. commit-ish)

2013-06-18 Thread Richard Hansen
Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index 01365d9..a3cc003 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -82,6 +82,17 @@ to point at the new commit.
to the top def_directory,directory of the stored
revision.
 
+[[def_committish]]committish (also commit-ish)::
+   A def_ref,ref pointing to an def_object,object that
+   can be recursively dereferenced to a
+   def_commit_object,commit object.
+   The following are all committishes:
+   a ref pointing to a commit object,
+   a ref pointing to a def_tag_object,tag object that points
+   to a commit object,
+   a ref pointing to a tag object that points to a tag object
+   that points to a commit object, etc.
+
 [[def_core_git]]core Git::
Fundamental data structures and utilities of Git. Exposes only limited
source code management tools.
-- 
1.8.3.1

--
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] documentation cleanups for rev^{type}

2013-06-18 Thread Richard Hansen
The documentation for the rev^{type} syntax (e.g.,
v1.8.3.1^{tree}) needed some fixing, and while I was at it I thought
I'd be pedantic about tree-ish.  Oh, and let's welcome committish to
the party!

-Richard


Richard Hansen (4):
  glossary: add 'treeish' as a synonym for 'tree-ish'
  glossary: define committish (a.k.a. commit-ish)
  glossary: more precise definition of tree-ish (a.k.a. treeish)
  revisions.txt: fix and clarify rev^{type}

 Documentation/glossary-content.txt | 29 +
 Documentation/revisions.txt| 12 
 2 files changed, 33 insertions(+), 8 deletions(-)

-- 
1.8.3.1

--
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/4] glossary: add 'treeish' as a synonym for 'tree-ish'

2013-06-18 Thread Richard Hansen
The documentation contains a mix of the two spellings, and including
both makes it possible for users to search the glossary with their
spelling of choice.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index db2a74d..01365d9 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -486,7 +486,7 @@ should not be combined with other pathspec.
with refs to the associated blob and/or tree objects. A
def_tree,tree is equivalent to a def_directory,directory.
 
-[[def_tree-ish]]tree-ish::
+[[def_tree-ish]]tree-ish (also treeish)::
A def_ref,ref pointing to either a def_commit_object,commit
object, a def_tree_object,tree object, or a def_tag_object,tag
object pointing to a tag or commit or tree object.
-- 
1.8.3.1

--
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] revisions.txt: fix and clarify rev^{type}

2013-06-18 Thread Richard Hansen
If possible, rev will be dereferenced even if it is not a tag type
(e.g., commit dereferenced to a tree).

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/revisions.txt | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 09896a3..bf563cf 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -114,10 +114,14 @@ some output processing may assume ref names in UTF-8.
 
 'rev{caret}\{type\}', e.g. 'v0.99.8{caret}\{commit\}'::
   A suffix '{caret}' followed by an object type name enclosed in
-  brace pair means the object
-  could be a tag, and dereference the tag recursively until an
-  object of that type is found or the object cannot be
-  dereferenced anymore (in which case, barf).  'rev{caret}0'
+  brace pair means dereference the object at 'rev' recursively until
+  an object of type 'type' is found or the object cannot be
+  dereferenced anymore (in which case, barf).
+  For example, if 'rev' is a committish, 'rev{caret}\{commit\}'
+  specifies the corresponding commit object.
+  Similarly, if 'rev' is a tree-ish, 'rev{caret}\{tree\}'
+  specifies the corresponding tree object.
+  'rev{caret}0'
   is a short-hand for 'rev{caret}\{commit\}'.
 +
 'rev{caret}\{object\}' can be used to make sure 'rev' names an
-- 
1.8.3.1

--
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] glossary: more precise definition of tree-ish (a.k.a. treeish)

2013-06-18 Thread Richard Hansen
Mention dereferencing, and that a commit dereferences to a tree, to
support gitrevisions(7) and rev-parse's error messages.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/glossary-content.txt | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index a3cc003..9a50fe1 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -498,9 +498,19 @@ should not be combined with other pathspec.
def_tree,tree is equivalent to a def_directory,directory.
 
 [[def_tree-ish]]tree-ish (also treeish)::
-   A def_ref,ref pointing to either a def_commit_object,commit
-   object, a def_tree_object,tree object, or a def_tag_object,tag
-   object pointing to a tag or commit or tree object.
+   A def_ref,ref pointing to an def_object,object that
+   can be recursively dereferenced to a
+   def_tree_object,tree object.
+   Dereferencing a def_commit_object,commit object yields the
+   tree object corresponding to the def_revision,revision's
+   top def_directory,directory.
+   The following are all tree-ishes:
+   a def_committish,committish,
+   a ref pointing to a tree object,
+   a ref pointing to a def_tag_object,tag object that points
+   to a tree object,
+   a ref pointing to a tag object that points to a tag object
+   that points to a tree object, etc.
 
 [[def_unmerged_index]]unmerged index::
An def_index,index which contains unmerged
-- 
1.8.3.1

--
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] glossary: define committish (a.k.a. commit-ish)

2013-06-18 Thread Richard Hansen
On 2013-06-19 00:19, Ramkumar Ramachandra wrote:
 Is master~3 a committish?  What about :/foomery?

Yes; as documented, both of those are refs that point to a commit.

 Look at the other forms in gitrevisions(7); master:quuxery,
 master^{tree} are notable exceptions.

gitrevisions(7) says that master:quuxery is a ref pointing to a blob or
tree, so it is not a committish.  However, if quuxery is a submodule, I
would expect master:quuxery to point to a commit object and thus be a
committish.  So perhaps the rev:path description in gitrevisions(7)
should be updated to accommodate submodules.

master^{tree} is guaranteed to be a tree (if such a tree exists), so it
is not a committish.

-Richard
--
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] glossary: define committish (a.k.a. commit-ish)

2013-06-19 Thread Richard Hansen
On 2013-06-19 01:56, Ramkumar Ramachandra wrote:
 From gitglossary(7):
 
 ref
 A 40-byte hex representation of a SHA-1 or a name that denotes a
 particular object. They may be stored in a file under $GIT_DIR/refs/
 directory, or in the $GIT_DIR/packed-refs file.
 
 Do master~3 and :/foomery qualify as refs?

Yes; they are names that denote a particular object.

 
 Look at the other forms in gitrevisions(7); master:quuxery,
 master^{tree} are notable exceptions.

 gitrevisions(7) says that master:quuxery is a ref pointing to a blob or
 tree, so it is not a committish.  However, if quuxery is a submodule, I
 would expect master:quuxery to point to a commit object and thus be a
 committish.  So perhaps the rev:path description in gitrevisions(7)
 should be updated to accommodate submodules.
 
 When quuxery is a submodule, master:quuxery refers to a commit object
 that does not exist in the parent repository.  I don't know what we
 gain by documenting a comittish you can't even `show`.

Fair point.

-Richard
--
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] glossary: define committish (a.k.a. commit-ish)

2013-06-19 Thread Richard Hansen
On 2013-06-19 11:31, Richard Hansen wrote:
 On 2013-06-19 01:56, Ramkumar Ramachandra wrote:
 From gitglossary(7):

 ref
 A 40-byte hex representation of a SHA-1 or a name that denotes a
 particular object. They may be stored in a file under $GIT_DIR/refs/
 directory, or in the $GIT_DIR/packed-refs file.

 Do master~3 and :/foomery qualify as refs?
 
 Yes; they are names that denote a particular object.

Hmm...  Maybe not.  There is no definition of name in gitglossary(7),
but there is object name, and that says:

object name
The unique identifier of an object.  The hash of the object's
contents using the Secure Hash Algorithm 1 and usually
represented by the 40 character hexadecimal encoding of the
hash of the object.

That definition excludes master~3 and :/foomery.  So perhaps we need a
clearer definition of ref, or add a separate definition of name that
is distinct from object name, or change the definition of object
name to be more general (and perhaps define object ID to take the
current definition of object name?).

In sha1_name.c, master~3 and :/foomery are considered to be names.  I
think it'd be a good idea if gitglossary(7) matched the code, because
that's the vocabulary Git developers and power users will use.
Unfortunately, in my mind name has a connotation that doesn't quite
match what sha1_name.c considers to be a name (I think of name as an
arbitrary, more-or-less semanticless label attached to something for the
purpose of convenient identification; the stuff in gitrevisions(7) are
more like operators on a name).

Maybe object specifier (objspec for short) could be used to refer to
all the ways one could specify an object?  Similarly, commit
specifier/commitspec, tree specifier/treespec, etc.  A treeish would
then be defined as a treespec or something that can be dereferenced to a
treespec.


BTW, I'm not a huge fan of the current definition of ref in
gitglossary(7) because to me a ref is ONLY something in .git/refs (or
HEAD, FETCH_HEAD, etc.) -- NOT a SHA1.  But I used ref in the
definition of committish because that's how the definition of
tree-ish was worded.

It's also unfortunate that gitrevisions(7) isn't just about specifying
revisions -- it's about specifying any object.


Anyway, although my patches aren't perfect, I think they improve the
current situation.  If there are no objections I would like to see them
committed.

Thanks,
Richard
--
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/4] glossary: add 'treeish' as a synonym for 'tree-ish'

2013-06-19 Thread Richard Hansen
On 2013-06-19 13:09, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:
 
 The documentation contains a mix of the two spellings, and including
 both makes it possible for users to search the glossary with their
 spelling of choice.
 
 Is it an option to instead find dashless form in our documentation
 and turn all of them into tree-ish form with a dash?  I personally
 find it cleaner that way.

I can s/treeish/tree-ish/g, although I'd still like to keep 'treeish' in
the glossary so that people can find it when they search for the
misspelled version.

Perhaps something like:

-[[def_tree-ish]]tree-ish::
+[[def_tree-ish]]tree-ish (sometimes misspelled treeish)::

would be satisfactory?

While we're on the topic, do you have a preference for commit-ish vs.
committish?  Grepping the code shows comittish to be the overwhelming
favorite, but it's inconsistent with tree-ish.

-Richard


 

 Signed-off-by: Richard Hansen rhan...@bbn.com
 ---
  Documentation/glossary-content.txt | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/Documentation/glossary-content.txt 
 b/Documentation/glossary-content.txt
 index db2a74d..01365d9 100644
 --- a/Documentation/glossary-content.txt
 +++ b/Documentation/glossary-content.txt
 @@ -486,7 +486,7 @@ should not be combined with other pathspec.
  with refs to the associated blob and/or tree objects. A
  def_tree,tree is equivalent to a def_directory,directory.
  
 -[[def_tree-ish]]tree-ish::
 +[[def_tree-ish]]tree-ish (also treeish)::
  A def_ref,ref pointing to either a def_commit_object,commit
  object, a def_tree_object,tree object, or a def_tag_object,tag
  object pointing to a tag or commit or tree object.
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

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


Re: [PATCH 2/4] glossary: define committish (a.k.a. commit-ish)

2013-06-19 Thread Richard Hansen
On 2013-06-19 13:14, Junio C Hamano wrote:
 object-type-ish does not have anything to do with a ref.  Even
 when an object is dangling in your object store without being
 reachable from any of your refs, it keeps its own ish-ness.

Ah, so your personal definition of ref matches my personal definition
of ref, and this definition doesn't match gitglossary(7).  :)

 
 ish-ness is a property of the object itself.
 
  * A commit object has a single top-level tree, and when a command
wants a tree object, you can often pass it a commit (historically
some commands were more strict and refused to work on a commit
when they wanted a tree).  In other words, a commit can be used
in place for a tree.  A commit object is a tree-ish.
 
  * A tag object, when it points (recursively) at a commit object,
can often be used in place for a commit object.  Such a tag
object is a commit-ish.
 
  * A tag object, when it points (recursively) at a tree object, can
often be used in place for a tree object.  Such a tag object is a
tree-ish.  Note that such a tag object cannot be a commit-ish.

I agree with all of this; the issue is the definition of ref in
gitglossary(7).  That definition should be fixed.  In the meantime, I'll
rework the patch series to avoid using the word ref when defining
committish and tree-ish.

-Richard
--
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] peel_onion(): add support for rev^{tag}

2013-06-19 Thread Richard Hansen
On 2013-06-19 14:38, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:
 
 gitrevisions(7) implies that rev^{tag} should work,...
 
 Does it?  Is it possible that that should be fixed?

Depends on whether you think ^{tag} is a useful feature or not; see below.

 
 What does it even _mean_ to peel something to a TAG?

It's the same as peeling something to any other object type:  If the
object is that type, done.  Otherwise dereference and try again.  If it
can't be dereferenced, barf.

 
 A commit, a tree or a blob cannot be peeled to a tag---none of them
 can contain a tag.

Right, so all of those would barf.

 
 When you have a tag that points at something else, what you have is
 already a tag, so that-tag^{tag} would be that-tag itself.

Exactly, just like object^{object} is object itself.

 
 Even more confusingly, when you have a tag that points at another
 tag, what does that-outer-tag^{tag} mean?  The outer tag itself,
 or do you need to peel at least once to reveal the inner-tag?  What
 if that inner-tag points at yet another tag?
 
 The patch does not touch peel_to_type(), so your answer to the above
 question seems to be if T is already a tag, T^{tag} is T itself,
 but then that operation does not look all that useful.

Barfing on non-tags is the feature this adds.  It's otherwise useless,
just like object^{object} is useless except to barf when object
doesn't exist.

It's a sometimes-convenient way to assert that an object specifier
refers to a tag object and not something else.  For example, instead of:

   fatal() { printf %s\\n ERROR: $* 2; exit 1; }

   type=$(git cat-file -t $1) || fatal $1 is not a valid object
   [ ${type} = tag ] || fatal $1 is not a tag object
   use $1 here

you can do:

   use $1^{tag} here

-Richard


 
 Confused...
 
 diff --git a/sha1_name.c b/sha1_name.c
 index 90419ef..68fd0e4 100644
 --- a/sha1_name.c
 +++ b/sha1_name.c
 @@ -677,6 +677,8 @@ static int peel_onion(const char *name, int len, 
 unsigned char *sha1)
  sp++; /* beginning of type name, or closing brace for empty */
  if (!strncmp(commit_type, sp, 6)  sp[6] == '}')
  expected_type = OBJ_COMMIT;
 +else if (!strncmp(tag_type, sp, 3)  sp[3] == '}')
 +expected_type = OBJ_TAG;
  else if (!strncmp(tree_type, sp, 4)  sp[4] == '}')
  expected_type = OBJ_TREE;
  else if (!strncmp(blob_type, sp, 4)  sp[4] == '}')
 

--
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/4] glossary: add 'treeish' as a synonym for 'tree-ish'

2013-06-19 Thread Richard Hansen
On 2013-06-19 13:09, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:
 
 The documentation contains a mix of the two spellings, and including
 both makes it possible for users to search the glossary with their
 spelling of choice.
 
 Is it an option to instead find dashless form in our documentation
 and turn all of them into tree-ish form with a dash?  I personally
 find it cleaner that way.

My preference is treeish and committish (instead of tree-ish and
commit-ish) because those are the spellings used in the source code.

-Richard

 

 Signed-off-by: Richard Hansen rhan...@bbn.com
 ---
  Documentation/glossary-content.txt | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/Documentation/glossary-content.txt 
 b/Documentation/glossary-content.txt
 index db2a74d..01365d9 100644
 --- a/Documentation/glossary-content.txt
 +++ b/Documentation/glossary-content.txt
 @@ -486,7 +486,7 @@ should not be combined with other pathspec.
  with refs to the associated blob and/or tree objects. A
  def_tree,tree is equivalent to a def_directory,directory.
  
 -[[def_tree-ish]]tree-ish::
 +[[def_tree-ish]]tree-ish (also treeish)::
  A def_ref,ref pointing to either a def_commit_object,commit
  object, a def_tree_object,tree object, or a def_tag_object,tag
  object pointing to a tag or commit or tree object.
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

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


Re: [PATCH 2/4] glossary: define committish (a.k.a. commit-ish)

2013-06-19 Thread Richard Hansen
On 2013-06-19 17:05, Junio C Hamano wrote:
 Richard Hansen rhan...@bbn.com writes:
 
 On 2013-06-19 13:14, Junio C Hamano wrote:
 object-type-ish does not have anything to do with a ref.  Even
 when an object is dangling in your object store without being
 reachable from any of your refs, it keeps its own ish-ness.

 Ah, so your personal definition of ref matches my personal definition
 of ref, and this definition doesn't match gitglossary(7).  :)
 
 Huh?  The only thing I I said was that *-ish does not have
 anything to do with a ref.  I didn't say anything about definition
 of ref.

The phrase

when an object is dangling in your object store without being
reachable from any of your refs

implies something about your definition of a ref that is inconsistent
with gitglossary(7).  See below.

 
 You are the one who brought ref into description of *-ish, with
 this:
 
 +[[def_committish]]committish (also commit-ish)::
 +A def_ref,ref pointing to an def_object,object that
 +can be recursively dereferenced to a

And I did that to be consistent with the definition of tree-ish, which
currently says:

tree-ish
A ref pointing to either a commit object, a tree object, or a
tag object pointing to a tag or commit or tree object.

Notice the term ref in the above definition.  This definition says
that a tree-ish is a particular kind of ref -- NOT a property of an
object as you claim.  I'm not saying you're wrong -- I actually agree
with you completely -- I'm just saying that your definition of ref
doesn't match the definition of ref in gitglossary(7).

The current definition of ref says:

ref
A 40-byte hex representation of a SHA-1 or a name that denotes
a particular object.  They may be stored in a file under
$GIT_DIR/refs/ directory, or in the $GIT_DIR/packed-refs file.

Depending on how one interprets name (which is not defined in
gitglossary(7)) in the above definition of ref, claiming that
master:README is a ref is consistent with gitglossary(7).  It is NOT,
however, consistent with what you -- or anyone else I know -- think of
as a ref.

 
 All I am saying is that an object does not have to be pointed by any
 ref to be any-ish.  ish-ness is an attribute of an object, not an
 ref.  You do not say refs/heads/master (which is a ref) is a
 commit-ish or a tree-ish.  The object pointed at by that ref is
 always a commit and is a commit-ish and a tree-ish.

I understand and agree completely and always have.

Here's what I'm trying to say:

  * Given the current definition of ref in gitglossary(7), claiming
that a foo-ish is a ref is not entirely incorrect.
  * If the definition of ref is altered to match the general
understanding of a ref, then claiming that a foo-ish is a ref is
wrong.  Very wrong.

I was trying to be minimal and consistent with my changes, but
unfortunately it seems like more changes are necessary.  When I next
have time, I'll send some revised patches to include the following changes:

  * replace the current definition of ref with something that matches
general understanding
  * eliminate the use of ref in the definitions of tag object, tree
object, and tree-ish
  * create a term that means a thing understood by rev-parse that
uniquely identifies an object (perhaps object specifier?) that
can be used in gitglossary(7) and elsewhere

-Richard
--
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] glossary: define committish (a.k.a. commit-ish)

2013-06-19 Thread Richard Hansen
On 2013-06-19 18:36, Junio C Hamano wrote:
 Ahh.  If you had quoted [...] a few exchanges ago I would have
 immediately understood what you were trying to say.

Sorry about that, my bad.

 In today's world (after packed-refs was introduced), probably
 
   A name that begins with refs/ (e.g. refs/heads/master) that
   can point at an object name.
 
 The namespace of refs is hierarchical and different
 subhierarchy is used for different purposes (e.g. the
 refs/heads/ hierarchy is used to represent local branches).
 
 is an appropriate rewrite of the above.

Some thoughts about the above definition:
  * Aren't HEAD, FETCH_HEAD, ORIG_HEAD also refs?
  * That definition excludes symrefs.
  * It may be worthwhile to mention that refs are part of the
repository.
  * Is a ref a name?  Or is it the binding of a name to an object/ref?

How about:

ref
A binding of a name to an object or other ref (in which case it
is a symref).  Refs are stored in the repository.

The ref namespace is hierarchical.  Different subhierarchies
are used for different purposes (e.g. the refs/heads/ hierarchy
is used to represent local branches).

 
 If we also want to explain the implementation details of refs, then
 additionally at the end of the first paragraph, add:
 
   ... at an object name, by storing its 40-byte hex
   representation.  They are implemented as either a file in
   $GIT_DIR/refs/ directory (called loose refs) or an entry
   in $GIT_DIR/packed-refs file (called packed refs); when a
   loose ref exists, a packed ref of the same name is ignored.

It would be good to document this somewhere, but I'm not sure the
glossary is the right place for it.  Maybe gitrepository-layout(5)?

-Richard
--
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-options.txt: restore `-e` option

2013-05-16 Thread Richard Hansen
It looks like commit f8246281af9adb0fdddbcc90d2e19cb5cd5217e5
unintentionally removed the documentation for the `-e` option.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/merge-options.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 34a8445..f192cd2 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -8,12 +8,13 @@ failed and do not autocommit, to give the user a chance to
 inspect and further tweak the merge result before committing.
 
 --edit::
+-e::
 --no-edit::
Invoke an editor before committing successful mechanical merge to
further edit the auto-generated merge message, so that the user
can explain and justify the merge. The `--no-edit` option can be
used to accept the auto-generated message (this is generally
-   discouraged). The `--edit` option is still useful if you are
+   discouraged). The `--edit` (or `-e`) option is still useful if you are
giving a draft message with the `-m` option from the command line
and want to edit it in the editor.
 +
-- 
1.8.2.3

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


Re: [PATCH v3 1/5] pull: rename pull.rename to pull.mode

2013-09-19 Thread Richard Hansen
On 2013-09-09 18:49, Felipe Contreras wrote:
 On Mon, Sep 9, 2013 at 4:23 PM, Richard Hansen rhan...@bbn.com wrote:
 On 2013-09-08 21:23, Felipe Contreras wrote:
 The old configurations still work, but get deprecated.

 Should some tests for the deprecated configs be added?  We wouldn't want
 to accidentally break those.
 
 Probably, but Junio is not picking this patch anyway.

It sounds to me like he would with some mods:
http://thread.gmane.org/gmane.comp.version-control.git/233554/focus=234488

-Richard
--
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/9] transport-helper: updates

2013-09-22 Thread Richard Hansen
On 2013-08-29 11:23, Felipe Contreras wrote:
 Here are the patches that allow transport helpers to be completely 
 transparent;
 renaming branches, deleting them, custom refspecs, --force, --dry-run,
 reporting forced update, everything works.

What is the status of these patches?

I would like to be able to 'git push old:new' and 'git push -f' to a
bzr repo, and these are a prerequisite.  I imagine changes to
git-remote-bzr will also be required; I'm willing write up such
changes to git-remote-bzr, but only if there's interest in picking up
this patch series.

Thanks,
Richard
--
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 8/9] transport-helper: add support to delete branches

2013-09-22 Thread Richard Hansen
On 2013-08-29 11:23, Felipe Contreras wrote:
 For remote-helpers that use 'export' to push.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  t/t5801-remote-helpers.sh |  8 
  transport-helper.c| 11 ++-
  2 files changed, 14 insertions(+), 5 deletions(-)
 
[...]
 diff --git a/transport-helper.c b/transport-helper.c
 index c7135ef..5490796 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -844,18 +844,19 @@ static int push_refs_with_export(struct transport 
 *transport,
   }
   free(private);
  
 - if (ref-deletion)
 - die(remote-helpers do not support ref deletion);
 -

The above deleted lines actually appear twice in transport-helper.c due
to an incorrect merge conflict resolution in
99d9ec090677c925c534001f01cbaf303a31cb82.  The other copy of those lines should
also be deleted:

diff --git a/transport-helper.c b/transport-helper.c
index 859131f..bbf4e7c 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -874,9 +874,6 @@ static int push_refs_with_export(struct transport 
*transport,
char *private;
unsigned char sha1[20];

-   if (ref-deletion)
-   die(remote-helpers do not support ref deletion);
-
private = apply_refspecs(data-refspecs, data-refspec_nr, 
ref-name);
if (private  !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
--
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 4/9] fast-export: add new --refspec option

2013-09-22 Thread Richard Hansen
On 2013-08-29 11:23, Felipe Contreras wrote:
 So that we can covert the exported ref names.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/git-fast-export.txt |  4 
  builtin/fast-export.c | 30 ++
  t/t9350-fast-export.sh|  7 +++
  3 files changed, 41 insertions(+)
 
 diff --git a/Documentation/git-fast-export.txt 
 b/Documentation/git-fast-export.txt
 index 85f1f30..221506b 100644
 --- a/Documentation/git-fast-export.txt
 +++ b/Documentation/git-fast-export.txt
 @@ -105,6 +105,10 @@ marks the same across runs.
   in the commit (as opposed to just listing the files which are
   different from the commit's first parent).
  
 +--refspec::
 + Apply the specified refspec to each ref exported. Multiple of them can
 + be specified.
 +

Do you mean '--refspec=refspec' and/or '--refspec refspec'?

How are the multiple refspecs specified?  Space/comma/colon separated
list?  Or multiple '--refspec' arguments with one refspec per '--refspec'?

 diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
 index 34c2d8f..dcf 100755
 --- a/t/t9350-fast-export.sh
 +++ b/t/t9350-fast-export.sh
 @@ -504,4 +504,11 @@ test_expect_success 'refs are updated even if no commits 
 need to be exported' '
   test_cmp expected actual
  '
  
 +test_expect_success 'use refspec' '
 + git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
 + grep ^commit  | sort | uniq  actual 
 + echo commit refs/heads/foobar  expected 
 + test_cmp expected actual
 +'
 +
  test_done
 

I think it'd be good to add a test for multiple refspecs.

-Richard
--
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/9] transport-helper: add 'force' to 'export' helpers

2013-09-22 Thread Richard Hansen
On 2013-08-29 11:23, Felipe Contreras wrote:
 Otherwise they cannot know when to force the push or not (other than
 hacks).
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  transport-helper.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/transport-helper.c b/transport-helper.c
 index 63cabc3..62051a6 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -814,6 +814,9 @@ static int push_refs_with_export(struct transport 
 *transport,
   die(helper %s does not support dry-run, data-name);
   }
  
 + if (flags  TRANSPORT_PUSH_FORCE)
 + set_helper_option(transport, force, true);

Should the return value of set_helper_option() be checked?

Also, should there be a #define TRANS_OPT_FORCE force with
TRANS_OPT_FORCE added to boolean_options[]?

Thanks,
Richard
--
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 1/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Richard Hansen
On 2013-10-11 19:56, Felipe Contreras wrote:
 Matthieu Moy wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 On Mon, Sep 9, 2013 at 9:21 PM, Jeff King p...@peff.net wrote:
 On Mon, Sep 09, 2013 at 05:49:36PM -0500, Felipe Contreras wrote:

 These deprecation warning messages should be written to stderr, and
 should probably be prefixed with WARNING: .

 Is there any deprecation warning that works this way?

 The ones in C code typically use warning(), which will prefix with
 warning: and write to stderr. They do not use all-caps, though.

 Try git log --grep=deprecate -Swarning for some examples.

 I'm asking about the ones in shell, because this is a shell script.

 No user cares whether git pull is written in shell. shell Vs C is an
 implementation detail, stdout Vs stderr is user-visible.
 
 You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, in the
 meantime no shell script does that, and that's no reason to reject this patch
 series.

Sure, but is is a reason to reroll the patch series.  Maybe it's not a
strong enough reason on its own to reroll, but if you're going to reroll
anyway, this should be fixed.

-Richard
--
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 1/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Richard Hansen
On 2013-10-11 22:08, Felipe Contreras wrote:
 I'm fine with 'echo warning: foo 2', but still, if you really
 cared about consistency, there would be a warn() function

So add one!  It's only one simple line:

warning() { printf %s\\n warning: $* 2; }

So much discussion for something so trivial...

-Richard
--
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 11/10] fixup! transport-helper: add support to delete branches

2013-10-26 Thread Richard Hansen
Patch 2/10 (transport-helper: fix extra lines) deleted one copy of the
lines; patch 9/10 (transport-helper: add support to delete branches)
should delete the other copy of the lines.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 transport-helper.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index 10db28e..23526de 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -878,9 +878,6 @@ static int push_refs_with_export(struct transport 
*transport,
char *private;
unsigned char sha1[20];
 
-   if (ref-deletion)
-   die(remote-helpers do not support ref deletion);
-
private = apply_refspecs(data-refspecs, data-refspec_nr, 
ref-name);
if (private  !get_sha1(private, sha1)) {
strbuf_addf(buf, ^%s, private);
-- 
1.8.4.1

--
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 00/10] transport-helper: updates

2013-10-27 Thread Richard Hansen
On 10/12/2013 03:05 AM, Felipe Contreras wrote:
 Hi,
 
 Here are the patches that allow transport helpers to be completely 
 transparent;
 renaming branches, deleting them, custom refspecs, --force, --dry-run,
 reporting forced update, everything works.

These patches don't cleanly apply to master anymore; would you be
willing to rebase and post a new version?

I wanted to test these changes via git-remote-bzr on a bzr repository
I'm working on, but unfortunately git-remote-bzr doesn't yet support
force pushes.  I may look into adding force push support to
git-remote-bzr, unless you beat me to it.  :)

In general these patches look good to me and I'd like to see them merged
to master.

Thanks,
Richard
--
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 11/10] fixup! transport-helper: add 'force' to 'export' helpers

2013-10-27 Thread Richard Hansen
document the new 'force' option

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 Documentation/gitremote-helpers.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index f1f4ca9..e75699c 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -437,6 +437,10 @@ set by Git if the remote helper has the 'option' 
capability.
 'option check-connectivity' \{'true'|'false'\}::
Request the helper to check connectivity of a clone.
 
+'option force' \{'true'|'false'\}::
+   Request the helper to perform a force update.  Defaults to
+   'false'.
+
 SEE ALSO
 
 linkgit:git-remote[1]
-- 
1.8.4.1.610.g2fe5c0e

--
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 12/10] git-remote-testgit: support the new 'force' option

2013-10-27 Thread Richard Hansen
Signed-off-by: Richard Hansen rhan...@bbn.com
---
 git-remote-testgit.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 6d2f282..80546c1 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -6,6 +6,7 @@ url=$2
 
 dir=$GIT_DIR/testgit/$alias
 prefix=refs/testgit/$alias
+forcearg=
 
 default_refspec=refs/heads/*:${prefix}/heads/*
 
@@ -39,6 +40,7 @@ do
fi
test -n $GIT_REMOTE_TESTGIT_SIGNED_TAGS  echo signed-tags
test -n $GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE  echo 
no-private-update
+   echo 'option'
echo
;;
list)
@@ -93,6 +95,7 @@ do
before=$(git for-each-ref --format=' %(refname) %(objectname) ')
 
git fast-import \
+   ${forcearg} \
${testgitmarks:+--import-marks=$testgitmarks} \
${testgitmarks:+--export-marks=$testgitmarks} \
--quiet
@@ -115,6 +118,21 @@ do
 
echo
;;
+   option\ *)
+   read cmd opt val EOF
+${line}
+EOF
+   case ${opt} in
+   force)
+   case ${val} in
+   true) forcearg=--force; echo 'ok';;
+   false) forcearg=; echo 'ok';;
+   *) printf %s\\n error '${val}'\
+ is not a valid value for option ${opt};;
+   esac;;
+   *) echo unsupported;;
+   esac
+   ;;
'')
exit
;;
-- 
1.8.4.1.614.ga09cf56

--
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 13/10] test: remote-helper: add test for force pushes

2013-10-27 Thread Richard Hansen
Signed-off-by: Richard Hansen rhan...@bbn.com
---
 t/t5801-remote-helpers.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index be543c0..93a7d34 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -102,6 +102,19 @@ test_expect_success 'push delete branch' '
 rev-parse --verify refs/heads/new-name
 '
 
+test_expect_success 'force push' '
+   (cd local 
+git checkout -b force-test 
+echo content file 
+git commit -a -m eight 
+git push origin force-test 
+echo content file 
+git commit -a --amend -m eight-modified 
+git push --force origin force-test
+   ) 
+   compare_refs local refs/heads/force-test server refs/heads/force-test
+'
+
 test_expect_success 'cloning without refspec' '
GIT_REMOTE_TESTGIT_REFSPEC= \
git clone testgit::${PWD}/server local2 2error 
-- 
1.8.4.1.614.ga09cf56

--
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 v4 12/10] git-remote-testgit: support the new 'force' option

2013-11-10 Thread Richard Hansen
On 2013-10-29 04:41, Felipe Contreras wrote:
 Richard Hansen wrote:
 Signed-off-by: Richard Hansen rhan...@bbn.com
 ---
  git-remote-testgit.sh | 18 ++
  1 file changed, 18 insertions(+)

 diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
 index 6d2f282..80546c1 100755
 --- a/git-remote-testgit.sh
 +++ b/git-remote-testgit.sh
 @@ -6,6 +6,7 @@ url=$2
  
  dir=$GIT_DIR/testgit/$alias
  prefix=refs/testgit/$alias
 +forcearg=
  
  default_refspec=refs/heads/*:${prefix}/heads/*
  
 @@ -39,6 +40,7 @@ do
  fi
  test -n $GIT_REMOTE_TESTGIT_SIGNED_TAGS  echo signed-tags
  test -n $GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE  echo 
 no-private-update
 +echo 'option'
  echo
  ;;
  list)
 @@ -93,6 +95,7 @@ do
  before=$(git for-each-ref --format=' %(refname) %(objectname) ')
  
  git fast-import \
 +${forcearg} \
  ${testgitmarks:+--import-marks=$testgitmarks} \
  ${testgitmarks:+--export-marks=$testgitmarks} \
  --quiet
 @@ -115,6 +118,21 @@ do
  
  echo
  ;;
 +option\ *)
 +read cmd opt val EOF
 +${line}
 +EOF
 
 We can do -EOF to align this properly.

Good point.  I personally avoid tabs whenever possible, and - only
works with tabs, so I'm in the habit of doing EOF.

 
 Also, I don't see why all the variables are ${foo} instead of $foo.

I'm in the habit of doing ${foo} because I like the consistency --
sometimes you need them to disambiguate, and sometimes you need special
expansions like ${foo##bar} or ${foo:-bar}.

In this case it's actually less consistent to do ${foo} because the rest
of the file doesn't use {} when not needed, so I agree with your change.

 
 +case ${opt} in
 +force)
 
 I think the convention is to align these:
 
 case $opt in
 force)

The existing case statement in this file indents the patterns the same
amount as the case statement, so this should be aligned to match.

In general I rarely see the case patterns indented at the same level as
the case statement, possibly because Emacs shell-mode indents the
patterns more than the case statement (by default).  The POSIX spec
contains a mix of styles:
  * the normative text documenting the format of a 'case' construct
indents the patterns more than the 'case' statement
  * two of the four non-normative examples indent the patterns
more than the 'case' statements; the other two do not

 
 +case ${val} in
 +true) forcearg=--force; echo 'ok';;
 +false) forcearg=; echo 'ok';;
 +*) printf %s\\n error '${val}'\
 + is not a valid value for option ${opt};;
 
 I think this is packing a lot of stuff and it's not that readable.
 
 Moreover, this is not for production purposes, it's for testing purposes and a
 guideline, I think this suffices.
 
 
   option\ *)
   read cmd opt val -EOF
   $line
   EOF
   case $opt in
   force)
   test $val = true  force=true || force=
   echo ok
   ;;
   *)
   echo unsupported
   ;;
   esac
   ;;

Works for me.

 
 But this is definetly good to have, will merge.

Thanks,
Richard

--
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/7] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

2013-11-10 Thread Richard Hansen
Change 'git push' to 'git push -u remote branch' in one of the
test-bzr.sh tests to ensure that the test continues to pass when the
default value of push.default changes to simple.

Also, explicitly set push.default to simple to silence warnings when
using --verbose.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 contrib/remote-helpers/test-bzr.sh | 5 -
 contrib/remote-helpers/test-hg.sh  | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-bzr.sh 
b/contrib/remote-helpers/test-bzr.sh
index 094062c..ea597b0 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -28,6 +28,9 @@ check () {
 
 bzr whoami A U Thor aut...@example.com
 
+# silence warnings
+git config --global push.default simple
+
 test_expect_success 'cloning' '
(
bzr init bzrrepo 
@@ -379,7 +382,7 @@ test_expect_success 'export utf-8 authors' '
git add content 
git commit -m one 
git remote add bzr bzr::../bzrrepo 
-   git push bzr
+   git push -u bzr master
) 
 
(
diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index dbe0eec..53f2bba 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -102,6 +102,9 @@ setup () {
GIT_AUTHOR_DATE=2007-01-01 00:00:00 +0230 
GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE 
export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
+
+   # silence warnings
+   git config --global push.default simple
 }
 
 setup
-- 
1.8.5.rc1.207.gc17dd22

--
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/7] test-hg.sh: eliminate 'local' bashism

2013-11-10 Thread Richard Hansen
Unlike bash, POSIX shell does not specify a 'local' command for
declaring function-local variable scope.  Except for IFS, the variable
names are not used anywhere else in the script so simply remove the
'local'.  For IFS, move the assignment to the 'read' command to
prevent it from affecting code outside the function.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 contrib/remote-helpers/test-hg.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 53f2bba..558a656 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -54,14 +54,14 @@ check_bookmark () {
 }
 
 check_push () {
-   local expected_ret=$1 ret=0 ref_ret=0 IFS=':'
+   expected_ret=$1 ret=0 ref_ret=0
 
shift
git push origin $@ 2error
ret=$?
cat error
 
-   while read branch kind
+   while IFS=':' read branch kind
do
case $kind in
'new')
-- 
1.8.5.rc1.207.gc17dd22

--
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/7] remote-hg, remote-bzr fixes

2013-11-10 Thread Richard Hansen
A handful of fixes for the git-remote-hg and git-remote-bzr remote
helpers and their unit tests.

Richard Hansen (7):
  remote-hg:  don't decode UTF-8 paths into Unicode objects
  test-bzr.sh, test-hg.sh: allow running from any dir
  test-bzr.sh, test-hg.sh: prepare for change to push.default=simple
  test-hg.sh: eliminate 'local' bashism
  test-hg.sh: avoid obsolete 'test' syntax
  test-hg.sh: help user correlate verbose output with email test
  remote-bzr, remote-hg: fix email address regular expression

 contrib/remote-helpers/git-remote-bzr |  7 +++
 contrib/remote-helpers/git-remote-hg  |  9 -
 contrib/remote-helpers/test-bzr.sh|  6 +-
 contrib/remote-helpers/test-hg.sh | 31 ++-
 4 files changed, 30 insertions(+), 23 deletions(-)

-- 
1.8.5.rc1.207.gc17dd22

--
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/7] test-bzr.sh, test-hg.sh: allow running from any dir

2013-11-10 Thread Richard Hansen
cd to the t/ subdirectory so that the user doesn't already have to be
in the test directory to run these test scripts.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 contrib/remote-helpers/test-bzr.sh | 1 +
 contrib/remote-helpers/test-hg.sh  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/contrib/remote-helpers/test-bzr.sh 
b/contrib/remote-helpers/test-bzr.sh
index 5c50251..094062c 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -5,6 +5,7 @@
 
 test_description='Test remote-bzr'
 
+cd ${0%/*}/../../t || exit 1
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON
diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 72f745d..dbe0eec 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -8,6 +8,7 @@
 
 test_description='Test remote-hg'
 
+cd ${0%/*}/../../t || exit 1
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON
-- 
1.8.5.rc1.207.gc17dd22

--
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/7] remote-hg: don't decode UTF-8 paths into Unicode objects

2013-11-10 Thread Richard Hansen
The internal mercurial API expects ordinary 8-bit string objects, not
Unicode string objects.  With this change, the test-hg.sh unit tests
pass again.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 contrib/remote-helpers/git-remote-hg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 3222afd..c6026b9 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -747,7 +747,7 @@ def parse_commit(parser):
 f = { 'deleted' : True }
 else:
 die('Unknown file command: %s' % line)
-path = c_style_unescape(path).decode('utf-8')
+path = c_style_unescape(path)
 files[path] = f
 
 # only export the commits if we are on an internal proxy repo
-- 
1.8.5.rc1.207.gc17dd22

--
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 5/7] test-hg.sh: avoid obsolete 'test' syntax

2013-11-10 Thread Richard Hansen
The POSIX spec says that the '-a', '-o', and parentheses operands to
the 'test' utility are obsolete extensions due to the potential for
ambiguity.  Replace '-o' with '|| test' to avoid unspecified behavior.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 contrib/remote-helpers/test-hg.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 558a656..84c67ff 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -83,7 +83,7 @@ check_push () {
test $ref_ret -ne 0  echo match for '$branch' failed  
break
done
 
-   if test $expected_ret -ne $ret -o $ref_ret -ne 0
+   if test $expected_ret -ne $ret || test $ref_ret -ne 0
then
return 1
fi
-- 
1.8.5.rc1.207.gc17dd22

--
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 6/7] test-hg.sh: help user correlate verbose output with email test

2013-11-10 Thread Richard Hansen
It's hard to tell which author conversion test failed when the email
addresses look similar.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 contrib/remote-helpers/test-hg.sh | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 84c67ff..5eda265 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -209,16 +209,16 @@ test_expect_success 'authors' '
 
../expected 
author_test alpha  H G Wells we...@example.com 
-   author_test beta test test unknown 
-   author_test beta test t...@example.com (comment) test 
t...@example.com 
-   author_test gamma t...@example.com Unknown t...@example.com 
-   author_test delta namet...@example.com name t...@example.com 
-   author_test epsilon name t...@example.com name t...@example.com 

-   author_test zeta  test  test unknown 
-   author_test eta test  t...@example.com  test t...@example.com 
-   author_test theta test t...@example.com test t...@example.com 
-   author_test iota test  test at example dot com test unknown 

-   author_test kappa t...@example.com Unknown t...@example.com
+   author_test beta beta beta unknown 
+   author_test beta beta t...@example.com (comment) beta 
t...@example.com 
+   author_test gamma ga...@example.com Unknown ga...@example.com 
+   author_test delta deltat...@example.com delta t...@example.com 

+   author_test epsilon epsilon t...@example.com epsilon 
t...@example.com 
+   author_test zeta  zeta  zeta unknown 
+   author_test eta eta  t...@example.com  eta t...@example.com 
+   author_test theta theta t...@example.com theta t...@example.com 

+   author_test iota iota  test at example dot com iota unknown 

+   author_test kappa ka...@example.com Unknown ka...@example.com
) 
 
git clone hg::hgrepo gitrepo 
-- 
1.8.5.rc1.207.gc17dd22

--
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 7/7] remote-bzr, remote-hg: fix email address regular expression

2013-11-10 Thread Richard Hansen
Before, strings like foo@example.com would be converted to
foo. b...@example.com when they should be unknown
foo@example.com.

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 contrib/remote-helpers/git-remote-bzr | 7 +++
 contrib/remote-helpers/git-remote-hg  | 7 +++
 contrib/remote-helpers/test-hg.sh | 3 ++-
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr 
b/contrib/remote-helpers/git-remote-bzr
index 054161a..7e34532 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -44,8 +44,8 @@ import StringIO
 import atexit, shutil, hashlib, urlparse, subprocess
 
 NAME_RE = re.compile('^([^]+)')
-AUTHOR_RE = re.compile('^([^]+?)? ?([^]*)$')
-EMAIL_RE = re.compile('^([^]+[^ \\\t])?\\b(?:[ \\t]*?)\\b([^ \\t]+@[^ 
\\t]+)')
+AUTHOR_RE = re.compile('^([^]+?)? ?[]([^]*)(?:$|)')
+EMAIL_RE = re.compile(r'([^ \t]+@[^ \t]+)')
 RAW_AUTHOR_RE = re.compile('^(\w+) (.+)? (.*) (\d+) ([+-]\d+)')
 
 def die(msg, *args):
@@ -193,8 +193,7 @@ def fixup_user(user):
 else:
 m = EMAIL_RE.match(user)
 if m:
-name = m.group(1)
-mail = m.group(2)
+mail = m.group(1)
 else:
 m = NAME_RE.match(user)
 if m:
diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index c6026b9..30402d5 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -51,8 +51,8 @@ import time as ptime
 #
 
 NAME_RE = re.compile('^([^]+)')
-AUTHOR_RE = re.compile('^([^]+?)? ?([^]*)$')
-EMAIL_RE = re.compile('^([^]+[^ \\\t])?\\b(?:[ \\t]*?)\\b([^ \\t]+@[^ 
\\t]+)')
+AUTHOR_RE = re.compile('^([^]+?)? ?[]([^]*)(?:$|)')
+EMAIL_RE = re.compile(r'([^ \t]+@[^ \t]+)')
 AUTHOR_HG_RE = re.compile('^(.*?) ?(.*?)(?:(.+)?)?$')
 RAW_AUTHOR_RE = re.compile('^(\w+) (?:(.+)? )?(.*) (\d+) ([+-]\d+)')
 
@@ -316,8 +316,7 @@ def fixup_user_git(user):
 else:
 m = EMAIL_RE.match(user)
 if m:
-name = m.group(1)
-mail = m.group(2)
+mail = m.group(1)
 else:
 m = NAME_RE.match(user)
 if m:
diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 5eda265..9f5066b 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -218,7 +218,8 @@ test_expect_success 'authors' '
author_test eta eta  t...@example.com  eta t...@example.com 
author_test theta theta t...@example.com theta t...@example.com 

author_test iota iota  test at example dot com iota unknown 

-   author_test kappa ka...@example.com Unknown ka...@example.com
+   author_test kappa ka...@example.com Unknown ka...@example.com 
+   author_test lambda lambda.lam...@example.com Unknown 
lambda.lam...@example.com
) 
 
git clone hg::hgrepo gitrepo 
-- 
1.8.5.rc1.207.gc17dd22

--
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] fixup! transport-helper: add 'force' to 'export' helpers

2013-11-10 Thread Richard Hansen
defend against force=foo in the user's environment

Signed-off-by: Richard Hansen rhan...@bbn.com
---
 git-remote-testgit.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 6d2f282..1cfdea2 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -15,6 +15,8 @@ test -z $refspec  prefix=refs
 
 export GIT_DIR=$url/.git
 
+force=
+
 mkdir -p $dir
 
 if test -z $GIT_REMOTE_TESTGIT_NO_MARKS
-- 
1.8.5.rc1.207.gc17dd22

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


[PATCH v5 12/10] remote-bzr: support the new 'force' option

2013-11-10 Thread Richard Hansen
Signed-off-by: Richard Hansen rhan...@bbn.com
---
 contrib/remote-helpers/git-remote-bzr | 34 +-
 contrib/remote-helpers/test-bzr.sh| 22 +-
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr 
b/contrib/remote-helpers/git-remote-bzr
index 7e34532..ba693d1 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -42,6 +42,7 @@ import json
 import re
 import StringIO
 import atexit, shutil, hashlib, urlparse, subprocess
+import types
 
 NAME_RE = re.compile('^([^]+)')
 AUTHOR_RE = re.compile('^([^]+?)? ?[]([^]*)(?:$|)')
@@ -684,7 +685,8 @@ def do_export(parser):
 peer = bzrlib.branch.Branch.open(peers[name],
  
possible_transports=transports)
 try:
-peer.bzrdir.push_branch(branch, revision_id=revid)
+peer.bzrdir.push_branch(branch, revision_id=revid,
+overwrite=force)
 except bzrlib.errors.DivergedBranches:
 print error %s non-fast forward % ref
 continue
@@ -718,8 +720,34 @@ def do_capabilities(parser):
 print *import-marks %s % path
 print *export-marks %s % path
 
+print option
 print
 
+class InvalidOptionValue(Exception):
+pass
+
+def do_option(parser):
+(opt, val) = parser[1:3]
+handler = globals().get('do_option_' + opt)
+if handler and type(handler) == types.FunctionType:
+try:
+handler(val)
+except InvalidOptionValue:
+print error '%s' is not a valid value for option '%s' % (val, 
opt)
+else:
+print unsupported
+
+def do_bool_option(val):
+if val == 'true': ret = True
+elif val == 'false': ret = False
+else: raise InvalidOptionValue()
+print ok
+return ret
+
+def do_option_force(val):
+global force
+force = do_bool_option(val)
+
 def ref_is_valid(name):
 return not True in [c in name for c in '~^: \\']
 
@@ -882,6 +910,7 @@ def main(args):
 global is_tmp
 global branches, peers
 global transports
+global force
 
 alias = args[1]
 url = args[2]
@@ -895,6 +924,7 @@ def main(args):
 branches = {}
 peers = {}
 transports = []
+force = False
 
 if alias[5:] == url:
 is_tmp = True
@@ -930,6 +960,8 @@ def main(args):
 do_import(parser)
 elif parser.check('export'):
 do_export(parser)
+elif parser.check('option'):
+do_option(parser)
 else:
 die('unhandled command: %s' % line)
 sys.stdout.flush()
diff --git a/contrib/remote-helpers/test-bzr.sh 
b/contrib/remote-helpers/test-bzr.sh
index ea597b0..7d7778f 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -69,13 +69,33 @@ test_expect_success 'pushing' '
test_cmp expected actual
 '
 
+test_expect_success 'forced pushing' '
+   (
+   cd gitrepo 
+   echo three-new content 
+   git commit -a --amend -m three-new 
+   git push -f
+   ) 
+
+   (
+   cd bzrrepo 
+   # the forced update overwrites the bzr branch but not the bzr
+   # working directory (it tries to merge instead)
+   bzr revert
+   ) 
+
+   echo three-new expected 
+   cat bzrrepo/content actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'roundtrip' '
(
cd gitrepo 
git pull 
git log --format=%s -1 origin/master actual
) 
-   echo three expected 
+   echo three-new expected 
test_cmp expected actual 
 
(cd gitrepo  git push  git pull) 
-- 
1.8.5.rc1.207.gc17dd22

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


Re: [PATCH v5 12/10] remote-bzr: support the new 'force' option

2013-11-11 Thread Richard Hansen
On 2013-11-11 06:51, Felipe Contreras wrote:
 Richard Hansen wrote:
 Signed-off-by: Richard Hansen rhan...@bbn.com
 ---
  contrib/remote-helpers/git-remote-bzr | 34 
 +-
  contrib/remote-helpers/test-bzr.sh| 22 +-
  2 files changed, 54 insertions(+), 2 deletions(-)

 diff --git a/contrib/remote-helpers/git-remote-bzr 
 b/contrib/remote-helpers/git-remote-bzr
 index 7e34532..ba693d1 100755
 --- a/contrib/remote-helpers/git-remote-bzr
 +++ b/contrib/remote-helpers/git-remote-bzr
 @@ -42,6 +42,7 @@ import json
  import re
  import StringIO
  import atexit, shutil, hashlib, urlparse, subprocess
 +import types
  
  NAME_RE = re.compile('^([^]+)')
  AUTHOR_RE = re.compile('^([^]+?)? ?[]([^]*)(?:$|)')
 @@ -684,7 +685,8 @@ def do_export(parser):
  peer = bzrlib.branch.Branch.open(peers[name],
   
 possible_transports=transports)
  try:
 -peer.bzrdir.push_branch(branch, revision_id=revid)
 +peer.bzrdir.push_branch(branch, revision_id=revid,
 +overwrite=force)
  except bzrlib.errors.DivergedBranches:
  print error %s non-fast forward % ref
  continue
 @@ -718,8 +720,34 @@ def do_capabilities(parser):
  print *import-marks %s % path
  print *export-marks %s % path
  
 +print option
  print
  
 +class InvalidOptionValue(Exception):
 +pass
 +
 +def do_option(parser):
 +(opt, val) = parser[1:3]
 +handler = globals().get('do_option_' + opt)
 +if handler and type(handler) == types.FunctionType:
 +try:
 +handler(val)
 +except InvalidOptionValue:
 +print error '%s' is not a valid value for option '%s' % (val, 
 opt)
 +else:
 +print unsupported
 +
 +def do_bool_option(val):
 +if val == 'true': ret = True
 +elif val == 'false': ret = False
 +else: raise InvalidOptionValue()
 +print ok
 +return ret
 +
 +def do_option_force(val):
 +global force
 +force = do_bool_option(val)
 +
 
 While this organization has merit, I think it's overkill for a single option,
 or just a couple of them. If in the future we add more, we might revisit this,
 for the moment something like this would suffice:

OK, I'll reroll.

 
 class InvalidOptionValue(Exception):
   pass
 
 def get_bool_option(val):
   if val == 'true':
   return True
   elif val == 'false':
   return False
   else:
   raise InvalidOptionValue()
 
 def do_option(parser):
   global force
   _, key, value = parser.line.split(' ')

I'm surprised you prefer this over 'key, val = parser[1:3]' or even
'_, key, val = parser[:]'.  Are you intending to eventually remove
Parser.__getitem__()?

Thanks,
Richard


   try:
   if key == 'force':
   force = get_bool_option(value)
   print 'ok'
   else:
   print 'unsupported'
   except InvalidOptionValue:
   print error '%s' is not a valid value for option '%s' % (value, 
 key)
 
 Cheers.

--
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/7] remote-hg: don't decode UTF-8 paths into Unicode objects

2013-11-11 Thread Richard Hansen
On 2013-11-11 06:04, Felipe Contreras wrote:
 Richard Hansen wrote:
 The internal mercurial API expects ordinary 8-bit string objects, not
 Unicode string objects.  With this change, the test-hg.sh unit tests
 pass again.
 
 This makes sense to me, but the tests are already passing for me. How are they
 failing for you?

$ hg --version | head -n 1
Mercurial Distributed SCM (version 2.2.2)
$ cd ~/git/t
$ ../contrib/remote-helpers/test-hg.sh --verbose --immediate
...
Traceback (most recent call last):
  File ~/git/contrib/remote-helpers/git-remote-hg, line 1246, in module
sys.exit(main(sys.argv))
  File ~/git/contrib/remote-helpers/git-remote-hg, line 1230, in main
do_export(parser)
  File ~/git/contrib/remote-helpers/git-remote-hg, line 1031, in do_export
parse_commit(parser)
  File ~/git/contrib/remote-helpers/git-remote-hg, line 822, in
parse_commit
node = hghex(repo.commitctx(ctx))
  File /usr/lib/python2.7/dist-packages/mercurial/localrepo.py, line
1270, in commitctx
p2.manifestnode(), (new, drop))
  File /usr/lib/python2.7/dist-packages/mercurial/manifest.py, line
197, in add
cachedelta = (self.rev(p1), addlistdelta(addlist, delta))
  File /usr/lib/python2.7/dist-packages/mercurial/manifest.py, line
124, in addlistdelta
addlist[start:end] = array.array('c', content)
TypeError: array item must be char
not ok 4 - update bookmark

I can put the above in the commit message if people would like it there.

-Richard
--
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 6/7] test-hg.sh: help user correlate verbose output with email test

2013-11-11 Thread Richard Hansen
On 2013-11-11 06:42, Felipe Contreras wrote:
 Richard Hansen wrote:
 It's hard to tell which author conversion test failed when the email
 addresses look similar.

 Signed-off-by: Richard Hansen rhan...@bbn.com
 ---
  contrib/remote-helpers/test-hg.sh | 20 ++--
  1 file changed, 10 insertions(+), 10 deletions(-)

 diff --git a/contrib/remote-helpers/test-hg.sh 
 b/contrib/remote-helpers/test-hg.sh
 index 84c67ff..5eda265 100755
 --- a/contrib/remote-helpers/test-hg.sh
 +++ b/contrib/remote-helpers/test-hg.sh
 @@ -209,16 +209,16 @@ test_expect_success 'authors' '
  
  ../expected 
  author_test alpha  H G Wells we...@example.com 
 -author_test beta test test unknown 
 -author_test beta test t...@example.com (comment) test 
 t...@example.com 

Notice the two betas here in the original code.

 -author_test gamma t...@example.com Unknown t...@example.com 
 -author_test delta namet...@example.com name t...@example.com 
 -author_test epsilon name t...@example.com name t...@example.com 
 
 -author_test zeta  test  test unknown 
 -author_test eta test  t...@example.com  test t...@example.com 
 -author_test theta test t...@example.com test t...@example.com 
 -author_test iota test  test at example dot com test unknown 
 
 -author_test kappa t...@example.com Unknown t...@example.com
 +author_test beta beta beta unknown 
 +author_test beta beta t...@example.com (comment) beta 
 t...@example.com 
 
 Two betas?

See above.  I can change them to beta1 and beta2, or if you'd prefer I
can change them to beta and gamma and increment the subsequent entries.

Thanks,
Richard

 
 +author_test gamma ga...@example.com Unknown ga...@example.com 
 +author_test delta deltat...@example.com delta t...@example.com 
 
 +author_test epsilon epsilon t...@example.com epsilon 
 t...@example.com 
 +author_test zeta  zeta  zeta unknown 
 +author_test eta eta  t...@example.com  eta t...@example.com 
 +author_test theta theta t...@example.com theta t...@example.com 
 
 +author_test iota iota  test at example dot com iota unknown 
 
 +author_test kappa ka...@example.com Unknown ka...@example.com
  ) 
  
  git clone hg::hgrepo gitrepo 
 

--
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   >