security vulnerability disclosure procedure?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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'
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'
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}
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'
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}
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}
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}
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}
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}
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'
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}
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}
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)
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'
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)
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'
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'
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
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
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
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
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
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
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
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
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
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
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
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
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}
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)
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}
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'
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}
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)
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)
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)
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)
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'
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)
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}
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'
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)
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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