Re: [PATCH] Add support for a 'pre-push' hook
On Sat, Nov 17, 2012 at 7:39 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
On 11/16/2012 09:30 PM, Junio C Hamano wrote:
Aske Olsson askeols...@gmail.com writes:
If the script .git/hooks/pre-push exists and is executable it will be
called before a `git push` command, and when the script exits with a
non-zero status the push will be aborted.
The hook can be overridden by passing the '--no-verify' option to
`git push`.
The pre-push hook is usefull to run tests etc. before push. Or to make
sure that if a binary solution like git-media, git-annex or git-bin is
used the binaries are uploaded before the push, so when others do a
fetch the binaries will be available already. This also reduces the
need for introducing extra (git) commands to e.g. sync binaries.
Signed-off-by: Aske Olsson askeols...@gmail.com
---
...
+[[pre-push]]
+pre-push
+
+
+This hook is invoked by 'git push' and can be bypassed with the
+`--no-verify` option. It takes no parameter, and is invoked before
+the push happens.
+Exiting with a non-zero status from this script causes 'git push'
+to abort.
...
+ if (!no_verify run_hook(NULL, pre-push)) {
+ die(_(pre-push hook failed: exiting\n));
+ }
NAK, at least in the current form. At least the system should tell
the hook where it is pushing and what is being pushed.
I agree.
Yes, I also agree that is a nice piece of information to pass to the hook.
Will work on that.
Besides, there are five valid reasons to add a new hook to the
system, but your version of pre-push does not satisfy any of them:
http://thread.gmane.org/gmane.comp.version-control.git/94111/focus=71069
Here I disagree. I think it satisfies (1):
(1) A hook that countermands the normal decision made by the
underlying command. Examples of this class are the update
hook and the pre-commit hook.
pre-push would be very similar in spirit to pre-commit: pre-commit is a
filter that can prevent a bad commit from getting into the local
repository; pre-push is similarly a filter between the local repo and
remote repositories.
Yes, I was thinking of a pre-push hook in the same way, preventing bad
commits from leaving the repo, but I guess you could argue that a bad
commit shouldn't happen in the first place due to the pre-commit hook.
I also think it satisfies (2) and/or (5b):
(2) A hook that operates on data generated after the command
starts to run. [...]
(5) [...] Another example is the post-checkout
hook that gets information that is otherwise harder to get
(namely, if it was a branch checkout or file checkout --
you can figure it out by examining the command line but
that already is part of the processing git-checkout does
anyway, so no need to force duplicating that code in the
userland).
It would not be trivial for a wrapper script to figure out what branches
and commits are about to be pushed. But git push could tell the hook
script what branches are to be pushed. And if the pre-push hook is run
after negotiation between client and server of what commits need to be
transfered, the hook could also be provided that information and use it
to figure out which commits it should vet.
Although a pre-receive script on the remote repository could do most of
the same things as a pre-push script, the latter would sometimes have
advantages because it is run client-side:
* When the user doesn't have the ability to change the pre-receive
script on the server (think public git hosting services).
Yeah, and for my case I would like to sync some binaries to a remote
storage before pushing the commits that contains references to these.
I know a wrapper script could easily do this, but I'm not to sure that it will
work in an organization with a lot of developers. Sooner or later one of them
will have some git problem and search the net, find an answer which (of
course) doesn't include the commands in the wrapper script. Then he might
end up pushing something that none of the other devs can get hold of and
that might cause builds to break etc.
And we can't currently run pre-receive hooks to deny the commit.
* For user-specific actions that are not wanted by all users pushing to
the same server (for example, maybe I add the string WIP to commit
messages for commits that are not ready to be pushed; my pre-push script
could verify that I don't push such a commit by accident).
* Preventing secret info (password files, proprietary branches) from
being pushed. Even if the remote repo were taught to reject them, they
would have already traversed the internet.
Yes we have also seen a couple of these, would be nice to have a way
do deny them.
-Aske
--
To unsubscribe from this list: send the line 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] Add support for a 'pre-push' hook
On Fri, Nov 16, 2012 at 9:25 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Aske Olsson askeols...@gmail.com writes: +--no-verify:: + This option bypasses the pre-push hook. + See also linkgit:githooks[5]. + -q:: --quiet:: Suppress all output, including the listing of updated refs, Here, and below: you seem to have whitespace damage. Somebody replaced tabs with spaces I guess. Using git send-email helps avoiding this. I had some firewall problems at work, so ended up sending from gmail. Will fix ;) +D=`pwd` Unused variable, left from previous hacking I guess. Yep! I'd add a touch hook-ran in the script, a rm -f hook-ran before launching git-push, and test the file existance after the hook to make sure it was ran. Yea' that would probably be a good idea! I don't think you need to re-create the repos for each tests. Tests are good, but they're better when they're fast so avoiding useless operations is good. We try to write tests so that one test failure does not imply failures of the next tests (i.e. stopping in the middle should not not leave garbage that would prevent the next test from running), but do not necessarily write 100% self-contained tests. Ok, I'll speed it up. + echo one foo git add foo git commit -m one test_commit ? Cool, I'll use that! It would be cool to actually check that the push was not performed (verify that the target repo is still pointing to the old revision). Otherwise, an implementation that would call run_hook after pushing would pass the tests. [...] These two tests are not testing much. The hook is not executable, so it shouldn't be ran, but you do not check whether the hook is ran or not. At least make the exit 0 an exit 1 in the hook. All very good points, thanks. I'll get back to hacking. -Aske -- To unsubscribe from this list: send the line 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: Auto-repo-repair
Hi, You can't reliably just grab the broken objects, because most transports don't support grabbing arbitrary objects (you can do it if you have shell access to a known-good repository, but it's not automated). can we introduce a new or extend existing transports to support that ? cu -- Mit freundlichen Grüßen / Kind regards Enrico Weigelt VNC - Virtual Network Consult GmbH Head Of Development Pariser Platz 4a, D-10117 Berlin Tel.: +49 (30) 3464615-20 Fax: +49 (30) 3464615-59 enrico.weig...@vnc.biz; www.vnc.de -- To unsubscribe from this list: send the line 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] tcsh-completion re-using git-completion.bash
On Fri, Nov 16, 2012 at 10:46:16PM +0100, Felipe Contreras wrote:
On Fri, Nov 16, 2012 at 10:22 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Fri, Nov 16, 2012 at 10:03:41PM +0100, Felipe Contreras wrote:
As I understand the main issues with using the completion script with
zsh are the various little incompatibilities between the two shells
and bugs in zsh's emulation of Bash's completion-related builtins.
Running the completion script under Bash and using its results in zsh
would solve these issues at the root. And would allow as to remove
some if [[ -n ${ZSH_VERSION-} ]] code.
We can remove that code already, because we now have code that is
superior than zsh's bash completion emulation:
http://article.gmane.org/gmane.comp.version-control.git/208173
Which depends on the completion script having a wrapper function
around compgen filling COMPREPLY.
No, it does not. Previous incarnations didn't have this dependency:
http://article.gmane.org/gmane.comp.version-control.git/196720
Good.
However, COMPREPLY will be soon
filled by hand-rolled code to prevent expansion issues with compgen,
and there will be no such wrapper.
I'm still waiting to see a resemblance of that code, but my bet would
be that there will be a way to fill both COMPREPLY, and call zsh's
compadd. But it's hard to figure that out without any code. Which is
why I'm thinking on doing it myself.
But even in that case, if push comes to shoves, this zsh wrapper can
ultimately read COMPREPLY and figure things backwards, as even more
previous versions did:
http://article.gmane.org/gmane.comp.version-control.git/189310
Even better. I was just going to propose that zsh's completion could
just read the contents of COMPREPLY at the end of _git() and _gitk(),
because this way no zsh-induced helper functions and changes would be
needed to the completion script at all.
However, running the completion script with Bash would also prevent
possible issues caused by incompatibilities between the two shells
mentioned below.
This is the equivalent of what Marc is doing, except that zsh has no
problems running bash's code. Note there's a difference with zsh's
emulation bash (or rather bourne shell, or k shell), and zsh's
emulation of bash's _completion_. The former is fine, the later is
not.
There are a couple of constructs supported by Bash but not by zsh,
which we usually try to avoid.
Yes, and is that a big deal?
Not that big, but I wanted to point out that it's not fine either.
Just a slight maintenance burden, because we have to pay attention not
to use such constructs.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1
[Wow, that's quite the Cc-list above. I wonder why e.g. Robert ended
up there, when all his sins were to add a couple of 'git svn'
options back in 2009.]
On Sat, Nov 17, 2012 at 02:38:18AM +0100, Felipe Contreras wrote:
It's only used by __gitcomp, so move some code there and avoid going
through the loop again.
We could get rid of it altogether, but it's useful for zsh's completion
wrapper.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-completion.bash | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index ad3e1fe..d92d11e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -58,15 +58,12 @@ __gitdir ()
__gitcomp_1 ()
{
- local c IFS=$' \t\n'
- for c in $1; do
- c=$c$2
- case $c in
- --*=*|*.) ;;
- *) c=$c ;;
- esac
- printf '%s\n' $c
- done
+ local c=$1
+ case $c in
+ --*=*|*.) ;;
+ *) c=$c ;;
+ esac
+ printf '%s\n' $c
}
# The following function is based on code from:
@@ -249,10 +246,16 @@ __gitcomp ()
--*=)
;;
*)
- local IFS=$'\n'
- __gitcompadd $(__gitcomp_1 ${1-} ${4-}) ${2-} $cur_
+ local c IFS=$' \t\n'
+ for c in ${1-}; do
+ c=`__gitcomp_1 $c${4-}`
1. Backticks.
2. A subshell for every word in the wordlist?
+ if [[ $c = $cur_* ]]; then
+ COMPREPLY+=(${2-}$c)
This is the first time we use the append operator in the completion
script. When it came up last time the question was whether the
benefit of using it is large enough for worrying about supported Bash
versions.
http://article.gmane.org/gmane.comp.version-control.git/206525
+ fi
+ done
;;
esac
+
}
# Generates completion reply with compgen from newline-separated possible
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/5] completion: trivial test improvement
On Sat, Nov 17, 2012 at 02:38:16AM +0100, Felipe Contreras wrote:
Instead of passing a dummy , let's check if the last character is a
space, and then move the _cword accordingly.
Apparently we were passing all the way to compgen, which fortunately
expanded it to nothing.
Glad you noticed it, too.
I posted an alternative fix (without any new conditions in the code
path) a while ago:
http://article.gmane.org/gmane.comp.version-control.git/206525
Will repost it as part of my series shortly.
Lets do the right thing though.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t9902-completion.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index cbd0fb6..0b5e1f5 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -51,6 +51,7 @@ run_completion ()
local _cword
_words=( $1 )
(( _cword = ${#_words[@]} - 1 ))
+ test ${1: -1} == ' ' (( _cword += 1 ))
__git_wrap__git_main print_comp
}
@@ -156,7 +157,7 @@ test_expect_success '__gitcomp - suffix' '
'
test_expect_success 'basic' '
- run_completion git \\
+ run_completion git
# built-in
grep -q ^add \$ out
# script
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/5] completion: get rid of compgen
On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
The functionality we use is very simple, plus, this fixes a known
breakage 'complete tree filename with metacharacters'.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-completion.bash | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 975ae13..ad3e1fe 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -227,7 +227,11 @@ fi
__gitcompadd ()
{
- COMPREPLY=($(compgen -W $1 -P $2 -S $4 -- $3))
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
The whole point of creating __gitcomp_nl() back then was to fill
COMPREPLY without iterating through all words in the wordlist, making
completion faster for large number of words, e.g. a lot of refs, or
later a lot of symbols for 'git grep' in a larger project.
The loop here kills that optimization.
}
# Generates completion reply with compgen, appending a space to possible
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] completion: fix expansion issues with compgen
This series fixes the expansion issues with compgen reported by Jeroen Meijer a while ago in http://article.gmane.org/gmane.comp.version-control.git/201596 The problem is that the compgen Bash-builtin performs expansion on all words in the wordlist given to its -W option, breaking Git's completion script when e.g. refs or filenames contain expandable substrings. Since compgen has no option to turn off this expansion and we only use a small subset of compgen's functionality, this series replaces compgen in __gitcomp() and __gitcomp_nl() by some shell logic and a small awk script, respectively. Patches 1 and 2 are test enhancements and fixes, while 3 and 4 add new tests to make sure I don't break anything and to demonstrate the issues, respectively. The actual bugfixes are in patches 5 and 6. Finally, patch 7 is a cleanup made possible by patch 6 (could be squashed into 6). In the future we might want/need to fill COMPREPLY directly when completing a path in the tree:path notation instead using __gitcomp_nl() there, because paths can contain newlines, too. Compared to Felipe's series from yesterday, this series has more tests, more descriptive commit messages, and it's faster. However, it doesn't include his 1/5 (completion: get rid of empty COMPREPLY assignments). Footnote: check the patchlist below, and notice how format-patch put patches 4 and 5 into the same line. SZEDER Gábor (7): completion: make the 'basic' test more tester-friendly completion: fix args of run_completion() test helper completion: add tests for the __gitcomp_nl() completion helper function completion: add tests for invalid variable name among completion words completion: fix expansion issues in __gitcomp_nl() completion: fix expansion issue in __gitcomp() completion: remove the now unused __gitcomp_1() internal helper function contrib/completion/git-completion.bash | 57 --- t/t9902-completion.sh | 123 ++--- 2 files changed, 147 insertions(+), 33 deletions(-) -- 1.8.0.220.g4d14ece -- To unsubscribe from this list: send the line 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] completion: fix args of run_completion() test helper
To simulate that the user hit 'git TAB, the 'basic' test sets up the
rather strange command line containing the two words
git
i.e. the second word on the command line consists of two double
quotes. This is not what happens for real, however, because after
'git TAB' the second word on the command line is just an empty
string. Luckily, the test works nevertheless.
Fix this by passing the command line to run_completion() as separate
words.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index b56759f7..3af75872 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -49,7 +49,7 @@ run_completion ()
{
local -a COMPREPLY _words
local _cword
- _words=( $1 )
+ _words=( $@ )
(( _cword = ${#_words[@]} - 1 ))
__git_wrap__git_main print_comp
}
@@ -57,7 +57,7 @@ run_completion ()
test_completion ()
{
test $# -gt 1 echo $2 expected
- run_completion $@
+ run_completion $1
test_cmp expected out
}
@@ -156,7 +156,7 @@ test_expect_success '__gitcomp - suffix' '
'
test_expect_success 'basic' '
- run_completion git \\
+ run_completion git
# built-in
echo add expected
sed -n /^add \$/p out out2
@@ -170,7 +170,7 @@ test_expect_success 'basic' '
sed -n /^ls-files \$/p out out2
test_cmp expected out2
- run_completion git f
+ run_completion git f
expected
sed -n /^[^f]/p out out2
test_cmp expected out2
--
1.8.0.220.g4d14ece
--
To unsubscribe from this list: send the line 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] completion: add tests for the __gitcomp_nl() completion helper function
Test __gitcomp_nl()'s basic functionality, i.e. that trailing space, prefix, and suffix are added correctly. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- t/t9902-completion.sh | 84 +++ 1 file changed, 84 insertions(+) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 3af75872..32b3e8c4 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -155,6 +155,90 @@ test_expect_success '__gitcomp - suffix' ' test_cmp expected out ' +test_expect_success '__gitcomp_nl - trailing space' ' + sed -e s/Z$// expected -\EOF + maint Z + master Z + EOF + ( + declare -a COMPREPLY + refs=$(cat -\EOF + maint + master + next + pu + EOF + ) + cur=m + __gitcomp_nl $refs + print_comp + ) + test_cmp expected out +' + +test_expect_success '__gitcomp_nl - prefix' ' + sed -e s/Z$// expected -\EOF + --fixup=maint Z + --fixup=master Z + EOF + ( + declare -a COMPREPLY + refs=$(cat -\EOF + maint + master + next + pu + EOF + ) + cur=--fixup=m + __gitcomp_nl $refs --fixup= m + print_comp + ) + test_cmp expected out +' + +test_expect_success '__gitcomp_nl - suffix' ' + sed -e s/Z$// expected -\EOF + branch.maint.Z + branch.master.Z + EOF + ( + declare -a COMPREPLY + refs=$(cat -\EOF + maint + master + next + pu + EOF + ) + cur=branch.ma + __gitcomp_nl $refs branch. ma . + print_comp + ) + test_cmp expected out +' + +test_expect_success '__gitcomp_nl - no suffix' ' + sed -e s/Z$// expected -\EOF + maintZ + masterZ + EOF + ( + declare -a COMPREPLY + refs=$(cat -\EOF + maint + master + next + pu + EOF + ) + cur=ma + __gitcomp_nl $refs ma + print_comp + ) + test_cmp expected out +' + test_expect_success 'basic' ' run_completion git # built-in -- 1.8.0.220.g4d14ece -- To unsubscribe from this list: send the line 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] completion: make the 'basic' test more tester-friendly
The 'basic' test uses 'grep -q' to filter the resulting possible completion words while looking for the presence or absence of certain git commands, and relies on grep's exit status to indicate a failure. This works fine as long as there are no errors. However, in case of a failure it doesn't give any indication whatsoever about the reason of the failure, i.e. which condition failed. To make testers' life easier provide some output about the failed condition: store the results of the filtering in a file and compare its contents to the expected results by the good old test_cmp helper. However, to actually get output from test_cmp in case of an error we must make sure that test_cmp is always executed. Since in case of an error grep's exit code aborts the test immediately, before running the subsequent test_cmp, do the filtering using sed instead of grep. Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- t/t9902-completion.sh | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 8fa025f9..b56759f7 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -158,14 +158,22 @@ test_expect_success '__gitcomp - suffix' ' test_expect_success 'basic' ' run_completion git \\ # built-in - grep -q ^add \$ out + echo add expected + sed -n /^add \$/p out out2 + test_cmp expected out2 # script - grep -q ^filter-branch \$ out + echo filter-branch expected + sed -n /^filter-branch \$/p out out2 + test_cmp expected out2 # plumbing - ! grep -q ^ls-files \$ out + expected + sed -n /^ls-files \$/p out out2 + test_cmp expected out2 run_completion git f - ! grep -q -v ^f out + expected + sed -n /^[^f]/p out out2 + test_cmp expected out2 ' test_expect_success 'double dash git itself' ' -- 1.8.0.220.g4d14ece -- To unsubscribe from this list: send the line 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] completion: add tests for invalid variable name among completion words
The compgen Bash-builtin performs expansion on all words in the
wordlist given to its -W option, breaking Git's completion script in
various ways if one of those words can be expanded. The breakage can
be simply bogus possible completion words, but it can also be a
failure:
$ git branch '${foo.bar}'
$ git checkout TAB
bash: ${foo.bar}: bad substitution
${foo.bar} is an invalid variable name, which triggers the failure
when compgen attempts to expand it, completely breaking refs
completion. The same applies to e.g. completing the tree:path
notation when a filename contains a similar expandable substring.
Since both __gitcomp() and __gitcomp_nl() rely on compgen, both are
affected by this issue. So add a simple test for each of them to
check that such a word doesn't cause failures (but don't check the
resulting possible completion words for now, because that should be
quoted properly, and that's a separate topic).
Reported-by: Jeroen Meijer jjgmei...@hotmail.com
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
t/t9902-completion.sh | 13 +
1 file changed, 13 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 32b3e8c4..a108ec1c 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -71,6 +71,7 @@ test_completion_long ()
}
newline=$'\n'
+invalid_variable_name=${foo.bar}
test_expect_success '__gitcomp - trailing space - options' '
sed -e s/Z$// expected -\EOF
@@ -155,6 +156,12 @@ test_expect_success '__gitcomp - suffix' '
test_cmp expected out
'
+test_expect_failure '__gitcomp - doesnt fail because of invalid variable name'
'
+ (
+ __gitcomp $invalid_variable_name
+ )
+'
+
test_expect_success '__gitcomp_nl - trailing space' '
sed -e s/Z$// expected -\EOF
maint Z
@@ -239,6 +246,12 @@ test_expect_success '__gitcomp_nl - no suffix' '
test_cmp expected out
'
+test_expect_failure '__gitcomp_nl - doesnt fail because of invalid variable
name' '
+ (
+ __gitcomp_nl $invalid_variable_name
+ )
+'
+
test_expect_success 'basic' '
run_completion git
# built-in
--
1.8.0.220.g4d14ece
--
To unsubscribe from this list: send the line 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] completion: fix expansion issue in __gitcomp()
The compgen Bash-builtin performs expansion on all words in the
wordlist given to its -W option, breaking Git's completion script when
words passed to __gitcomp() contain expandable substrings.
In __gitcomp() we only use a small subset ot compgen's functionality,
namely the filtering of matching possible completion words and adding
a prefix to each of those words; suffix is added by the __gitcomp_1()
helper function instead. Now, since we already have to iterate over
all words in the wordlist to append a trailing space if necessary, we
can check in the same loop whether a word matches the word to be
completed and store matching words with possible prefix and/or suffix
in the COMPREPLY array. This way we achieve the same functionality
without the compgen builtin and, more importantly, without it's
problematic expansions.
An additional benefit, especially for Windows/MSysGit users, is the
loss of two subshells, one to run __gitcomp_1() and the other to run
the compgen builtin.
Note, that like the previous patch for __gitcomp_nl(), this patch
doesn't quote expandable words either, but that could be implemented
later on top by unquoting $cur and then quoting what get stored in
COMPREPLY.
Also update the function's description a bit.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-completion.bash | 27 ---
t/t9902-completion.sh | 2 +-
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 65196ddd..283ef99b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,12 +225,13 @@ _get_comp_words_by_ref ()
fi
fi
-# Generates completion reply with compgen, appending a space to possible
-# completion words, if necessary.
+# Generates completion reply for the word in $cur, appending a space to
+# possible completion words, if necessary.
# It accepts 1 to 4 arguments:
# 1: List of possible completion words.
# 2: A prefix to be added to each possible completion word (optional).
-# 3: Generate possible completion matches for this word (optional).
+# 3: Generate possible completion matches for this word instead of $cur
+#(optional).
# 4: A suffix to be appended to each possible completion word (optional).
__gitcomp ()
{
@@ -241,10 +242,22 @@ __gitcomp ()
COMPREPLY=()
;;
*)
- local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} \
- -W $(__gitcomp_1 ${1-} ${4-}) \
- -- $cur_))
+ local i=0 c IFS=$' \t\n'
+ for c in $1; do
+ case $c in
+ $cur_*)
+ c=$c${4-}
+ case $c in
+ --*=*|*.) ;;
+ *) c=$c ;;
+ esac
+ COMPREPLY[$i]=${2-}$c
+ i=$((++i))
+ ;;
+ *)
+ ;;
+ esac
+ done
;;
esac
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index fa289324..d08f4259 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -156,7 +156,7 @@ test_expect_success '__gitcomp - suffix' '
test_cmp expected out
'
-test_expect_failure '__gitcomp - doesnt fail because of invalid variable name'
'
+test_expect_success '__gitcomp - doesnt fail because of invalid variable name'
'
(
__gitcomp $invalid_variable_name
)
--
1.8.0.220.g4d14ece
--
To unsubscribe from this list: send the line 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] completion: remove the now unused __gitcomp_1() internal helper function
Since the __gitcomp_1() helper is basically only an implementation
detail of __gitcomp() that exists solely for performance reasons (see
ab02dfe5 (bash completion: Improve responsiveness of git-log
completion, 2008-07-13)), it's quite unlikely that anyone uses or ever
used it besides __gitcomp(). And now __gitcomp() doesn't need it
anymore either, so delete it.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
contrib/completion/git-completion.bash | 13 -
1 file changed, 13 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 283ef99b..a281b28d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -56,19 +56,6 @@ __gitdir ()
fi
}
-__gitcomp_1 ()
-{
- local c IFS=$' \t\n'
- for c in $1; do
- c=$c$2
- case $c in
- --*=*|*.) ;;
- *) c=$c ;;
- esac
- printf '%s\n' $c
- done
-}
-
# The following function is based on code from:
#
# bash_completion - programmable completion functions for bash 3.2+
--
1.8.0.220.g4d14ece
--
To unsubscribe from this list: send the line 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] completion: fix expansion issues in __gitcomp_nl()
The compgen Bash-builtin performs expansion on all words in the
wordlist given to its -W option, breaking Git's completion script when
refs or filenames passed to __gitcomp_nl() contain expandable
substrings. At least one user can't use ref completion at all in a
repository, which contains tags with metacharacters like
Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721
Such a ref causes a failure in compgen as it tries to expand the
variable with invalid name.
Unfortunately, compgen has no option to turn off this expansion.
However, in __gitcomp_nl() we use only a small subset of compgen's
functionality, namely the filtering of matching possible completion
words and adding a prefix and/or suffix to each of those words. So,
to avoid compgen's problematic expansion we get rid of compgen, and
implement the equivalent functionality in a small awk script. The
reason for using awk instead of sed is that awk allows plain (i.e.
non-regexp) string comparison, making quoting of regexp characters in
the current word unnecessary.
Note, that such expandable words need quoting to be of any use on the
command line. This patch doesn't perform that quoting, but it could
be implemented later on top by extending the awk script to unquote
$cur and to quote matching words. Nonetheless, this patch still a
step forward, because at least refs can be completed even in the
presence of one with metacharacters.
Also update the function's description a bit.
Reported-by: Jeroen Meijer jjgmei...@hotmail.com
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
awk doesn't have a prefixcmp(). I'm no awk wizard, so I'm not sure this
is the right way to do it.
contrib/completion/git-completion.bash | 17 +
t/t9902-completion.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index bc0657a2..65196ddd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -249,19 +249,28 @@ __gitcomp ()
esac
}
-# Generates completion reply with compgen from newline-separated possible
-# completion words by appending a space to all of them.
+# Generates completion reply for the word in $cur from newline-separated
+# possible completion words, appending a space to all of them.
# It accepts 1 to 4 arguments:
# 1: List of possible completion words, separated by a single newline.
# 2: A prefix to be added to each possible completion word (optional).
-# 3: Generate possible completion matches for this word (optional).
+# 3: Generate possible completion matches for this word instead of $cur
+#(optional).
# 4: A suffix to be appended to each possible completion word instead of
#the default space (optional). If specified but empty, nothing is
#appended.
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
+ COMPREPLY=($(awk -v pfx=${2-} -v sfx=${4- } -v cur=${3-$cur} '
+ BEGIN {
+ FS=\n;
+ len=length(cur);
+ }
+ {
+ if (cur == substr($1, 1, len))
+ print pfx$1sfx;
+ }' $1 ))
}
__git_heads ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a108ec1c..fa289324 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -246,7 +246,7 @@ test_expect_success '__gitcomp_nl - no suffix' '
test_cmp expected out
'
-test_expect_failure '__gitcomp_nl - doesnt fail because of invalid variable
name' '
+test_expect_success '__gitcomp_nl - doesnt fail because of invalid variable
name' '
(
__gitcomp_nl $invalid_variable_name
)
@@ -383,7 +383,7 @@ test_expect_success 'complete tree filename with spaces' '
EOF
'
-test_expect_failure 'complete tree filename with metacharacters' '
+test_expect_success 'complete tree filename with metacharacters' '
echo content name with \${meta}
git add .
git commit -m meta
--
1.8.0.220.g4d14ece
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1
On Sat, Nov 17, 2012 at 11:58 AM, SZEDER Gábor sze...@ira.uka.de wrote:
# The following function is based on code from:
@@ -249,10 +246,16 @@ __gitcomp ()
--*=)
;;
*)
- local IFS=$'\n'
- __gitcompadd $(__gitcomp_1 ${1-} ${4-}) ${2-} $cur_
+ local c IFS=$' \t\n'
+ for c in ${1-}; do
+ c=`__gitcomp_1 $c${4-}`
1. Backticks.
2. A subshell for every word in the wordlist?
Fine, lets make it hard for zsh then:
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -56,19 +56,6 @@ __gitdir ()
fi
}
-__gitcomp_1 ()
-{
- local c IFS=$' \t\n'
- for c in $1; do
- c=$c$2
- case $c in
- --*=*|*.) ;;
- *) c=$c ;;
- esac
- printf '%s\n' $c
- done
-}
-
# The following function is based on code from:
#
# bash_completion - programmable completion functions for bash 3.2+
@@ -241,12 +228,22 @@ __gitcomp ()
COMPREPLY=()
;;
*)
- local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} \
- -W $(__gitcomp_1 ${1-} ${4-}) \
- -- $cur_))
+ local c i IFS=$' \t\n'
+ i=0
+ for c in ${1-}; do
+ c=$c${4-}
+ case $c in
+ --*=*|*.) ;;
+ *) c=$c ;;
+ esac
+ if [[ $c = $cur_* ]]; then
+ (( i++ ))
+ COMPREPLY[$i]=${2-}$c
+ fi
+ done
;;
esac
+
}
# Generates completion reply with compgen from newline-separated possible
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/5] completion: get rid of compgen
On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
The functionality we use is very simple, plus, this fixes a known
breakage 'complete tree filename with metacharacters'.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-completion.bash | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 975ae13..ad3e1fe 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -227,7 +227,11 @@ fi
__gitcompadd ()
{
- COMPREPLY=($(compgen -W $1 -P $2 -S $4 -- $3))
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
The whole point of creating __gitcomp_nl() back then was to fill
COMPREPLY without iterating through all words in the wordlist, making
completion faster for large number of words, e.g. a lot of refs, or
later a lot of symbols for 'git grep' in a larger project.
The loop here kills that optimization.
So your solution is to move the loop to awk? I fail to see how that
could bring more optimization, specially since it includes an extra
fork now.
--
Felipe Contreras
--
To unsubscribe from this list: send the line 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] tcsh-completion re-using git-completion.bash
On Sat, Nov 17, 2012 at 11:56 AM, SZEDER Gábor sze...@ira.uka.de wrote: On Fri, Nov 16, 2012 at 10:46:16PM +0100, Felipe Contreras wrote: But even in that case, if push comes to shoves, this zsh wrapper can ultimately read COMPREPLY and figure things backwards, as even more previous versions did: http://article.gmane.org/gmane.comp.version-control.git/189310 Even better. I was just going to propose that zsh's completion could just read the contents of COMPREPLY at the end of _git() and _gitk(), because this way no zsh-induced helper functions and changes would be needed to the completion script at all. I would rather modify the __gitcomp function. Parsing COMPREPLY is too cumbersome. However, running the completion script with Bash would also prevent possible issues caused by incompatibilities between the two shells mentioned below. It could, but it doesn't now. This is the equivalent of what Marc is doing, except that zsh has no problems running bash's code. Note there's a difference with zsh's emulation bash (or rather bourne shell, or k shell), and zsh's emulation of bash's _completion_. The former is fine, the later is not. There are a couple of constructs supported by Bash but not by zsh, which we usually try to avoid. Yes, and is that a big deal? Not that big, but I wanted to point out that it's not fine either. Just a slight maintenance burden, because we have to pay attention not to use such constructs. Do we have to pay attention? I say when we encounter one of such maintenance burden issues _then_ we think about it. In the meantime for all we know sourcing bash's script from zsh is fine. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor sze...@ira.uka.de wrote:
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
+ COMPREPLY=($(awk -v pfx=${2-} -v sfx=${4- } -v cur=${3-$cur} '
+ BEGIN {
+ FS=\n;
+ len=length(cur);
+ }
+ {
+ if (cur == substr($1, 1, len))
+ print pfx$1sfx;
+ }' $1 ))
}
Does this really perform better than my alternative?
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git-credential-gnome-keyring fails at multilib
Downstream bug report: https://bugs.gentoo.org/443634 The git-credential-gnome-keyring Makefile doesn't allow overriding its variables, making for spectacular link failure if you use CFLAGS for aught but decoration. gcc -g -O2 -Wall -I/usr/include/gnome-keyring-1 -I/usr/include/glib-2.0 -I/usr/lib32/glib-2.0/include -o git-credential-gnome-keyring.o -c git-credential-gnome-keyring.c gcc -o git-credential-gnome-keyring -Wl,--as-needed -Wl,--hash-style=gnu -m32 git-credential-gnome-keyring.o -lgnome-keyring -lglib-2.0 /usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/../../../../x86_64-pc-linux-gnu/bin/ld: i386:x86-64 architecture of input file `git-credential-gnome-keyring.o' is incompatible with i386 output /usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/../../../../x86_64-pc-linux-gnu/bin/ld: git-credential-gnome-keyring.o: file class ELFCLASS64 incompatible with ELFCLASS32 /usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/../../../../x86_64-pc-linux-gnu/bin/ld: final link failed: File in wrong format collect2: error: ld returned 1 exit status Attached patch fixes it. /Peter git-1.8.0-gnome-keyring-multilib.patch Description: Binary data
Re: [PATCH 6/7] completion: fix expansion issue in __gitcomp()
On Sat, Nov 17, 2012 at 12:39:24PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor sze...@ira.uka.de wrote:
-# Generates completion reply with compgen, appending a space to possible
-# completion words, if necessary.
+# Generates completion reply for the word in $cur, appending a space to
+# possible completion words, if necessary.
# It accepts 1 to 4 arguments:
# 1: List of possible completion words.
# 2: A prefix to be added to each possible completion word (optional).
-# 3: Generate possible completion matches for this word (optional).
+# 3: Generate possible completion matches for this word instead of $cur
+#(optional).
# 4: A suffix to be appended to each possible completion word (optional).
__gitcomp ()
{
@@ -241,10 +242,22 @@ __gitcomp ()
COMPREPLY=()
;;
*)
- local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} \
- -W $(__gitcomp_1 ${1-} ${4-}) \
- -- $cur_))
+ local i=0 c IFS=$' \t\n'
+ for c in $1; do
+ case $c in
+ $cur_*)
+ c=$c${4-}
+ case $c in
+ --*=*|*.) ;;
+ *) c=$c ;;
+ esac
+ COMPREPLY[$i]=${2-}$c
+ i=$((++i))
+ ;;
+ *)
+ ;;
+ esac
+ done
This is not quite the same as before, is it? Before the suffix would
be taken into consideration for the comparison with $cur_, but not any
more.
That's a good catch, thanks.
I remember it puzzled me that the suffix is considered in the
comparison (and that a trailing space would be appended even after a
given suffix, too, so there seems to be no way to disable the trailing
space). However, currently it doesn't make a difference for us,
because afaics we never specify a suffix for __gitcomp(). There were
two instances in _git_config() where we specified . as suffix, but
those two were converted to __gitcomp_nl(). I changed those callsites
back to __gitcomp() and tried to provoke wrong behavior with the above
patch, but couldn't.
Anyway, it's better to err on the safe side, so here's the fixup.
-- 8 --
Subject: [PATCH] fixup! completion: fix expansion issue in __gitcomp()
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index a1bf732f..29818fb5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -231,9 +231,9 @@ __gitcomp ()
*)
local i=0 c IFS=$' \t\n'
for c in $1; do
+ c=$c${4-}
case $c in
$cur_*)
- c=$c${4-}
case $c in
--*=*|*.) ;;
*) c=$c ;;
--
1.8.0.220.g4d14ece
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/5] completion: get rid of compgen
On Sat, Nov 17, 2012 at 12:42:38PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
The functionality we use is very simple, plus, this fixes a known
breakage 'complete tree filename with metacharacters'.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-completion.bash | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 975ae13..ad3e1fe 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -227,7 +227,11 @@ fi
__gitcompadd ()
{
- COMPREPLY=($(compgen -W $1 -P $2 -S $4 -- $3))
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
The whole point of creating __gitcomp_nl() back then was to fill
COMPREPLY without iterating through all words in the wordlist, making
completion faster for large number of words, e.g. a lot of refs, or
later a lot of symbols for 'git grep' in a larger project.
The loop here kills that optimization.
So your solution is to move the loop to awk? I fail to see how that
could bring more optimization, specially since it includes an extra
fork now.
This patch didn't aim for more optimization, but it was definitely a
goal not to waste what we gained by creating __gitcomp_nl() in
a31e6262 (completion: optimize refs completion, 2011-10-15). However,
as it turns out the new version with awk is actually faster than
current master with compgen:
Before:
$ refs=$(for i in {0..} ; do echo branch$i ; done)
$ time __gitcomp_nl $refs
real0m0.242s
user0m0.220s
sys 0m0.028s
After:
$ time __gitcomp_nl $refs
real0m0.109s
user0m0.096s
sys 0m0.012s
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1
On Sat, Nov 17, 2012 at 12:27:40PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 11:58 AM, SZEDER Gábor sze...@ira.uka.de wrote:
# The following function is based on code from:
@@ -249,10 +246,16 @@ __gitcomp ()
--*=)
;;
*)
- local IFS=$'\n'
- __gitcompadd $(__gitcomp_1 ${1-} ${4-}) ${2-}
$cur_
+ local c IFS=$' \t\n'
+ for c in ${1-}; do
+ c=`__gitcomp_1 $c${4-}`
1. Backticks.
2. A subshell for every word in the wordlist?
Fine, lets make it hard for zsh then:
No, it's about keeping it usable. With this change offering the
approximately 170 commands for 'git help TAB' would take more than 4
seconds on Windows.
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -56,19 +56,6 @@ __gitdir ()
fi
}
-__gitcomp_1 ()
-{
- local c IFS=$' \t\n'
- for c in $1; do
- c=$c$2
- case $c in
- --*=*|*.) ;;
- *) c=$c ;;
- esac
- printf '%s\n' $c
- done
-}
-
# The following function is based on code from:
#
# bash_completion - programmable completion functions for bash 3.2+
@@ -241,12 +228,22 @@ __gitcomp ()
COMPREPLY=()
;;
*)
- local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} \
- -W $(__gitcomp_1 ${1-} ${4-}) \
- -- $cur_))
+ local c i IFS=$' \t\n'
+ i=0
+ for c in ${1-}; do
+ c=$c${4-}
+ case $c in
+ --*=*|*.) ;;
+ *) c=$c ;;
+ esac
+ if [[ $c = $cur_* ]]; then
+ (( i++ ))
+ COMPREPLY[$i]=${2-}$c
+ fi
+ done
;;
esac
+
}
# Generates completion reply with compgen from newline-separated possible
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
On Sat, Nov 17, 2012 at 12:50:39PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor sze...@ira.uka.de wrote:
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
+ COMPREPLY=($(awk -v pfx=${2-} -v sfx=${4- } -v cur=${3-$cur} '
+ BEGIN {
+ FS=\n;
+ len=length(cur);
+ }
+ {
+ if (cur == substr($1, 1, len))
+ print pfx$1sfx;
+ }' $1 ))
}
Does this really perform better than my alternative?
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
It does:
My version:
$ refs=$(for i in {0..} ; do echo branch$i ; done)
$ time __gitcomp_nl $refs
real0m0.109s
user0m0.096s
sys 0m0.012s
Yours:
$ time __gitcomp_nl $refs
real0m0.321s
user0m0.312s
sys 0m0.008s
--
To unsubscribe from this list: send the line 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] tcsh-completion re-using git-completion.bash
On Sat, Nov 17, 2012 at 12:46:27PM +0100, Felipe Contreras wrote: On Sat, Nov 17, 2012 at 11:56 AM, SZEDER Gábor sze...@ira.uka.de wrote: On Fri, Nov 16, 2012 at 10:46:16PM +0100, Felipe Contreras wrote: But even in that case, if push comes to shoves, this zsh wrapper can ultimately read COMPREPLY and figure things backwards, as even more previous versions did: http://article.gmane.org/gmane.comp.version-control.git/189310 Even better. I was just going to propose that zsh's completion could just read the contents of COMPREPLY at the end of _git() and _gitk(), because this way no zsh-induced helper functions and changes would be needed to the completion script at all. I would rather modify the __gitcomp function. Parsing COMPREPLY is too cumbersome. Each element of COMPREPLY contains a possible completion word. What parsing is needed to use that, that is so cumbersome? However, running the completion script with Bash would also prevent possible issues caused by incompatibilities between the two shells mentioned below. It could, but it doesn't now. This is the equivalent of what Marc is doing, except that zsh has no problems running bash's code. Note there's a difference with zsh's emulation bash (or rather bourne shell, or k shell), and zsh's emulation of bash's _completion_. The former is fine, the later is not. There are a couple of constructs supported by Bash but not by zsh, which we usually try to avoid. Yes, and is that a big deal? Not that big, but I wanted to point out that it's not fine either. Just a slight maintenance burden, because we have to pay attention not to use such constructs. Do we have to pay attention? Unless you don't mind possible breakages of zsh completion, yes. I say when we encounter one of such maintenance burden issues _then_ we think about it. In the meantime for all we know sourcing bash's script from zsh is fine. That's a cool argument, will remember it when it again comes to refactoring the __gitcomp() tests. For now those tests work just fine. When we encounter maintenance burden issues, like fixing a bug requiring the same modification to all of those tests, then we'll think about it. ;) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
using multiple version of git simultaneously
Hi, I'm trying to use different version of git simultaneously. So how can i append some suffix (like --program-suffix=git1.8) so that i can distinguish between different versions. -- Cheers arif -- To unsubscribe from this list: send the line 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] Rename V15_MINGW_HEADERS into CYGWIN_OLD_WINSOCK_HEADERS
On 11/17/2012 02:09 AM, Torsten Bögershausen wrote: See commit 380a4d927bff693c42fc6b22c3547bdcaac4bdc3: Update cygwin.c for new mingw-64 win32 api headers Cygwin up to 1.7.16 uses some header file from the WINE project Cygwin 1.7.17 uses some header file from the mingw-64 project As the old cygwin (like 1.5) never used mingw, the name V15_MINGW_HEADERS is confusing. Rename it into CYGWIN_OLD_WINSOCK_HEADERS diff --git a/Makefile b/Makefile index c3edf8c..c2ea735 100644 --- a/Makefile +++ b/Makefile @@ -1089,7 +1089,7 @@ ifeq ($(uname_O),Cygwin) NO_SYMLINK_HEAD = YesPlease NO_IPV6 = YesPlease OLD_ICONV = UnfortunatelyYes - V15_MINGW_HEADERS = YesPlease + CYGWIN_OLD_WINSOCK_HEADERS = YesPlease WINSOCK is certainly a part of the win32 api implementation, but it is is the entire win32api that changed, not just the tiny bit dealing with sockets. Basically, WINDOWS.h, and everything it includes, and all of the dlls it touches, and the .o files, changed. Calling it OLD is not helpful, what happens in the future with the next change? The only version info we really have is the dll version. We are switching between the win32 api implementation shipped with cygwin dll version 1.5.x and the one that is now current. And, the shift is not tied to any particular cygwin 1.7.x dll version either (there are no cross dependencies between the win32api implementation and any particular dll in the 1.7.x series, just a coincidence in time as to what packages got updated when). So my suggestion in the bike shedding category is to s/V15_MINGW_HEADERS/CYGWIN_V15_WIN32API/ /end of bike shedding. If this is really worth a second patch, I'll be happy to send one :^) Mark -- To unsubscribe from this list: send the line 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: using multiple version of git simultaneously
On Sat, 17 Nov 2012 20:25:21 +0600, arif aft...@gmail.com wrote: I'm trying to use different version of git simultaneously. So how can i append some suffix (like --program-suffix=git1.8) so that i can distinguish between different versions. Install each version into its own prefix (~/git/1.8.0/, ~/git/1.7.0/ etc). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
Hi, sorry for the late reply but my git time is limited. On Sat, Nov 10, 2012 at 02:02:32PM -0500, W. Trevor King wrote: On Fri, Nov 09, 2012 at 05:29:27PM +0100, Heiko Voigt wrote: I think we should agree on a behavior for this option and implement it the same time when add learns about it. When we were discussing floating submodules as an important option for the gerrit people I already started to implement a proof of concept. Please have a look here: https://github.com/hvoigt/git/commits/hv/floating_submodules After skimming through this, something like $ git submodule update --pull would probably be better than introducing a new command: Yeah along the lines of that, but one thing to keep in mind: We already have --rebase and --merge which do slightly different things (I think). Adding --pull here should behave similar to them. Like fetch and merge is the same to pull without submodules. If I am understanding your goal correctly your --pull would be different. On the other hand: A --pull makes no sense if we apply it to the existing --merge option since it merges the recorded sha1 into the current HEAD. Just a fetch would not really make a difference. Thinking along the existing options I would probably still expect --pull to merge something into the current HEAD. So maybe we have to iron out where this command/option should go. But changing that once we have a patch to discuss should not be that much work. So please proceed with --pull and once we know exactly what it does we can polish that. On Sat, Nov 10, 2012 at 01:44:37PM -0500, W. Trevor King wrote: $ git submodule pull-branch I think floating submodules is a misleading name for this feature though, since the checkout SHA is explicitly specified. We're just making it more convenient to explicitly update the SHA. How about tracking submodules? Until now we have always called this workflow floating submodules. I imaging since the submodule floats to the newest revision (whatever the user chooses that to be) instead of staying at the recorded sha1. tracking submodules sounds strange to me since the term tracked in git is mainly used in combination with exact recorded history (e.g. tracking branch). Since it is about *not* checking out the recorded sha1 but something that can change I think that could cause confusion. I think floating is a more unambiguous term and already known on the list. Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
Hi, On Sun, Nov 11, 2012 at 10:00:48AM -0500, W. Trevor King wrote: On Sun, Nov 11, 2012 at 02:33:45AM -0800, Junio C Hamano wrote: In order to avoid losing (or creating) local-only submodule commits, I'll probably bail (with an error) on non-fast-forward pulls. Can anyone else think of other safety concerns? That sounds like a good thing to do. We can allow more flexibility later if people come up with usecases. This means that I'll probably drop Phil's $submodule_* export in v4, because the only explicit use we have for it is this branch tracking. I still think it is a useful idea, but it may not be useful enough to be worth the complexity. Yes lets concentrate on the branch following first. (2) git diff [$path] and friends in the superproject compares the HEAD of the checkout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject. Hmm. ???git diff??? compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like: $ git submodule foreach 'git diff origin/$submodule_branch' Perhaps this is enough motivation for keeping $submodule_* exports? and the option were called something like --follow-branch=$branch, ??? I am not sure if hiding changes to the recorded SHA1 from the user is such a useful thing. In the first step I would like it if it was kept simple and only the submodule update machinery learned to follow a branch. If that results in local changes that should be shown. The user is still in charge of recording the updated SHA1 in his commit. From what I have heard of projects using this: They usually still have something that records the SHA1s on a regular basis. Thinking further, why not record them in git? We could add an option to update which creates such a commit. Since git is all about changes I am hesitant to hide them from the user. I'll replace -r/--record with --follow-branch in v4. Sounds good. Cheers Heiko -- To unsubscribe from this list: send the line 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] tcsh-completion re-using git-completion.bash
On Fri, Nov 16, 2012 at 4:56 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Fri, Nov 16, 2012 at 10:20 PM, Junio C Hamano gits...@pobox.com wrote:
The point is not about the quality of zsh's emulation
of (k)sh when it is run under that mode, but is about not having to
have that logic in bash-only part in the first place.
As I said, that logic can be moved away _if_ my wrapper is merged. But
then again, that would cause regressions to existing users.
Please forgive me as I don't know the background of the efforts for
zsh git-completion or
the syntax for zsh completion, but I thought I'd mention another
approach I tried for tcsh
which may work for zsh.
I gather that using a wrapper for zsh causes concerns about
backwards-compatibility.
So, what could be done is have the bash script do both jobs: setup the
zsh completion
commands, and output the git completion using bash itself. At the top
of git-completion.bash
(or it could be even pushed at the bottom using if/else) we could use:
if [[ -n ${ZSH_VERSION-} ]]; then
# replace below by zsh completion commands calling `bash
${HOME}/.git-completion.bash`
complete git 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
complete gitk 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
exit
fi
That way the zsh user would still simply do 'source
~/.git-completion.bash' which would
only execute the two zsh completion setup commands. Then, when completion is
triggered, it calls `bash ${HOME}/.git-completion.bash ${COMMAND_LINE}` and
processes the output like tcsh does. This limits the zsh-specific
code to 2 lines for
the entire script.
I got this to work for tcsh (solution (B)) adding the following a the top of
git-completion.bash:
test $tcsh != \
complete git 'p,*,`${HOME}/.git-completion.sh
${COMMAND_LINE}|\sort|\uniq`,' \
complete gitk 'p,*,`${HOME}/.git-completion.sh
${COMMAND_LINE}|\sort|\uniq`,' \
exit
but I didn't think people would go for that since those lines have to
work in both bash
and tcsh syntax. I thought this made the script a bit brittle.
Just a thought.
Marc
--
To unsubscribe from this list: send the line 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] tcsh-completion re-using git-completion.bash
On Sat, Nov 17, 2012 at 6:15 PM, Marc Khouzam marc.khou...@gmail.com wrote:
On Fri, Nov 16, 2012 at 4:56 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Fri, Nov 16, 2012 at 10:20 PM, Junio C Hamano gits...@pobox.com wrote:
The point is not about the quality of zsh's emulation
of (k)sh when it is run under that mode, but is about not having to
have that logic in bash-only part in the first place.
As I said, that logic can be moved away _if_ my wrapper is merged. But
then again, that would cause regressions to existing users.
Please forgive me as I don't know the background of the efforts for
zsh git-completion or
the syntax for zsh completion, but I thought I'd mention another
approach I tried for tcsh
which may work for zsh.
I gather that using a wrapper for zsh causes concerns about
backwards-compatibility.
I don't see any concerns.
if [[ -n ${ZSH_VERSION-} ]]; then
# replace below by zsh completion commands calling `bash
${HOME}/.git-completion.bash`
complete git 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
complete gitk 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
That doesn't work in zsh. It might be possible to do something
similar, but it would probably require many more lines.
And we can achieve the same by essentially moving the relevant code of
my wrapper:
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -23,10 +23,6 @@
#3) Consider changing your PS1 to also show the current branch,
# see git-prompt.sh for details.
-if [[ -n ${ZSH_VERSION-} ]]; then
- autoload -U +X bashcompinit bashcompinit
-fi
-
case $COMP_WORDBREAKS in
*:*) : great ;;
*) COMP_WORDBREAKS=$COMP_WORDBREAKS:
@@ -2404,6 +2400,32 @@ __gitk_main ()
__git_complete_revlist
}
+if [[ -n ${ZSH_VERSION-} ]]; then
+ emulate -L zsh
+
+ __gitcompadd ()
+ {
+ compadd -Q -S $4 -P ${(M)cur#*[=:]} -p $2 --
${=1} _ret=0
+ }
+
+ _git ()
+ {
+ local _ret=1
+ () {
+ emulate -L ksh
+ local cur cword prev
+ cur=${words[CURRENT-1]}
+ prev=${words[CURRENT-2]}
+ let cword=CURRENT-1
+ __${service}_main
+ }
+ let _ret _default -S '' _ret=0
+ return _ret
+ }
+ compdef _git git gitk
+ return
+fi
+
__git_func_wrap ()
{
if [[ -n ${ZSH_VERSION-} ]]; then
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
On Sat, Nov 17, 2012 at 3:14 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 12:50:39PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor sze...@ira.uka.de wrote:
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 --
${3-$cur}))
+ COMPREPLY=($(awk -v pfx=${2-} -v sfx=${4- } -v cur=${3-$cur}
'
+ BEGIN {
+ FS=\n;
+ len=length(cur);
+ }
+ {
+ if (cur == substr($1, 1, len))
+ print pfx$1sfx;
+ }' $1 ))
}
Does this really perform better than my alternative?
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
It does:
My version:
$ refs=$(for i in {0..} ; do echo branch$i ; done)
$ time __gitcomp_nl $refs
real0m0.109s
user0m0.096s
sys 0m0.012s
Yours:
$ time __gitcomp_nl $refs
real0m0.321s
user0m0.312s
sys 0m0.008s
Yeah, for 1 refs, which is not the common case:
== 1 ==
SZEDER:
real0m0.007s
user0m0.003s
sys 0m0.000s
felipec:
real0m0.000s
user0m0.000s
sys 0m0.000s
== 10 ==
SZEDER:
real0m0.004s
user0m0.003s
sys 0m0.001s
felipec:
real0m0.000s
user0m0.000s
sys 0m0.000s
== 100 ==
SZEDER:
real0m0.005s
user0m0.002s
sys 0m0.002s
felipec:
real0m0.002s
user0m0.002s
sys 0m0.000s
== 1000 ==
SZEDER:
real0m0.010s
user0m0.008s
sys 0m0.001s
felipec:
real0m0.018s
user0m0.017s
sys 0m0.001s
== 1 ==
SZEDER:
real0m0.062s
user0m0.060s
sys 0m0.003s
felipec:
real0m0.175s
user0m0.174s
sys 0m0.000s
== 10 ==
SZEDER:
real0m0.595s
user0m0.593s
sys 0m0.021s
felipec:
real0m1.848s
user0m1.843s
sys 0m0.003s
== 100 ==
SZEDER:
real0m6.258s
user0m6.241s
sys 0m0.215s
felipec:
real0m18.191s
user0m18.115s
sys 0m0.045s
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
On Sat, Nov 17, 2012 at 04:04:42PM +0100, Heiko Voigt wrote: On Sat, Nov 10, 2012 at 01:44:37PM -0500, W. Trevor King wrote: $ git submodule pull-branch I think floating submodules is a misleading name for this feature though, since the checkout SHA is explicitly specified. We're just making it more convenient to explicitly update the SHA. How about tracking submodules? Until now we have always called this workflow floating submodules. I imaging since the submodule floats to the newest revision (whatever the user chooses that to be) instead of staying at the recorded sha1. tracking submodules sounds strange to me since the term tracked in git is mainly used in combination with exact recorded history (e.g. tracking branch). Since it is about *not* checking out the recorded sha1 but something that can change I think that could cause confusion. I think floating is a more unambiguous term and already known on the list. I had been getting the impression that floating submodules would automatically update without explicit user intervention. After re-reading your initial floating submodules post, it looks like we do match up after the mapping: GitHeiko Trevor - - - update update --checkout update update update --pull So I'll go back to floating ;). On Sat, Nov 17, 2012 at 04:30:07PM +0100, Heiko Voigt wrote: (2) git diff [$path] and friends in the superproject compares the HEAD of the checkout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject. Hmm. ???git diff??? compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like: $ git submodule foreach 'git diff origin/$submodule_branch' Perhaps this is enough motivation for keeping $submodule_* exports? and the option were called something like --follow-branch=$branch, ??? I am not sure if hiding changes to the recorded SHA1 from the user is such a useful thing. In the first step I would like it if it was kept simple and only the submodule update machinery learned to follow a branch. If that results in local changes that should be shown. The user is still in charge of recording the updated SHA1 in his commit. I understand what you're warning against here, or what it has to do with git diff. From what I have heard of projects using this: They usually still have something that records the SHA1s on a regular basis. Thinking further, why not record them in git? We could add an option to update which creates such a commit. I think it's best to have users craft their own commit messages explaining why the branch was updated. That said, an auto-generated hint (a la git merge) would probably be a useful extra feature. -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
On Sat, Nov 17, 2012 at 8:08 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sat, Nov 17, 2012 at 3:14 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 12:50:39PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor sze...@ira.uka.de wrote:
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 --
${3-$cur}))
+ COMPREPLY=($(awk -v pfx=${2-} -v sfx=${4- } -v
cur=${3-$cur} '
+ BEGIN {
+ FS=\n;
+ len=length(cur);
+ }
+ {
+ if (cur == substr($1, 1, len))
+ print pfx$1sfx;
+ }' $1 ))
}
Does this really perform better than my alternative?
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
It does:
My version:
$ refs=$(for i in {0..} ; do echo branch$i ; done)
$ time __gitcomp_nl $refs
real0m0.109s
user0m0.096s
sys 0m0.012s
Yours:
$ time __gitcomp_nl $refs
real0m0.321s
user0m0.312s
sys 0m0.008s
Yeah, for 1 refs, which is not the common case:
And this beats both in every case:
while read -r x; do
if [[ $x == $3* ]]; then
COMPREPLY+=($2$x$4)
fi
done $1
== 1 ==
one:
real0m0.004s
user0m0.003s
sys 0m0.000s
two:
real0m0.000s
user0m0.000s
sys 0m0.000s
== 10 ==
one:
real0m0.005s
user0m0.002s
sys 0m0.002s
two:
real0m0.000s
user0m0.000s
sys 0m0.000s
== 100 ==
one:
real0m0.005s
user0m0.004s
sys 0m0.000s
two:
real0m0.001s
user0m0.000s
sys 0m0.000s
== 1000 ==
one:
real0m0.010s
user0m0.008s
sys 0m0.001s
two:
real0m0.004s
user0m0.004s
sys 0m0.000s
== 1 ==
one:
real0m0.061s
user0m0.057s
sys 0m0.005s
two:
real0m0.045s
user0m0.044s
sys 0m0.000s
== 10 ==
one:
real0m0.582s
user0m0.575s
sys 0m0.022s
two:
real0m0.487s
user0m0.482s
sys 0m0.004s
== 100 ==
one:
real0m6.305s
user0m6.284s
sys 0m0.216s
two:
real0m5.268s
user0m5.194s
sys 0m0.065s
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/5] completion: get rid of compgen
On Sat, Nov 17, 2012 at 3:12 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 12:42:38PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
The functionality we use is very simple, plus, this fixes a known
breakage 'complete tree filename with metacharacters'.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-completion.bash | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 975ae13..ad3e1fe 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -227,7 +227,11 @@ fi
__gitcompadd ()
{
- COMPREPLY=($(compgen -W $1 -P $2 -S $4 -- $3))
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
The whole point of creating __gitcomp_nl() back then was to fill
COMPREPLY without iterating through all words in the wordlist, making
completion faster for large number of words, e.g. a lot of refs, or
later a lot of symbols for 'git grep' in a larger project.
The loop here kills that optimization.
So your solution is to move the loop to awk? I fail to see how that
could bring more optimization, specially since it includes an extra
fork now.
This patch didn't aim for more optimization, but it was definitely a
goal not to waste what we gained by creating __gitcomp_nl() in
a31e6262 (completion: optimize refs completion, 2011-10-15). However,
as it turns out the new version with awk is actually faster than
current master with compgen:
Before:
$ refs=$(for i in {0..} ; do echo branch$i ; done)
$ time __gitcomp_nl $refs
real0m0.242s
user0m0.220s
sys 0m0.028s
After:
$ time __gitcomp_nl $refs
real0m0.109s
user0m0.096s
sys 0m0.012s
This one is even faster:
while read -r x; do
if [[ $x == $3* ]]; then
COMPREPLY+=($2$x$4)
fi
done $1
== 1 ==
one:
real0m0.148s
user0m0.134s
sys 0m0.025s
two:
real0m0.055s
user0m0.054s
sys 0m0.000s
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git mv` has ambiguous error message for non-existing target
Patrick Lehner lehner.patr...@gmx.de writes: But just because mv's error essage isnt very good, does that mean git mv's error message mustn't be better? Did I say the error message from 'mv' was not very good in the message you are responding to (by the way, this is why you should never top-post when you are responding to a message on this list)? I meant to say that the message from 'mv' is good enough, so is the one given by 'git mv'. I wouldn't reject a patch that updates our message to something more informative without looking at it, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/5] push: update remote tags only with force
This patch set can be divided into two sets: 1. Provide useful advice for rejected tag references. push: return reject reasons via a mask push: add advice for rejected tag reference Recommending a merge to resolve a rejected tag update seems nonsensical since the tag does not come along for the ride. These patches change the advice for rejected tags to suggest using push -f. 2. Require force when updating tag references, even on a fast-forward. push: flag updates push: flag updates that require force push: update remote tags only with force This is in response to the following thread: http://thread.gmane.org/gmane.comp.version-control.git/208354 This solution prevents fast-forwards if the reference is of the refs/tags/* hierarchy or if the old object is not a commit. These patches contain the following updates since the v3 set: * builtin/push.c: Remove push --force suggestion from advice. * remote.c: Only require old object to be a commit to be forwardable. I added the check for object types based comments from Peff in original thread, and I think this implementation is actually what he intended. If the new object is a tag, the operation is not destructive so there is no reason to block it (at least within the scope of these changes) as was done in the previous iteration. * t/t5516-fetch-push.sh: Create separate tests for the lightweight and annotated cases. Do the annotated tests outside of refs/tags/ so that it actually tests different functionality. Chris Rorvick (5): push: return reject reasons via a mask push: add advice for rejected tag reference push: flag updates push: flag updates that require force push: update remote tags only with force Documentation/git-push.txt | 10 +- builtin/push.c | 24 +++- builtin/send-pack.c| 9 +++-- cache.h| 7 ++- remote.c | 46 -- send-pack.c| 1 + t/t5516-fetch-push.sh | 30 +- transport-helper.c | 6 ++ transport.c| 25 +++-- transport.h| 10 ++ 10 files changed, 126 insertions(+), 42 deletions(-) -- 1.8.0.155.g3a063ad.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/5] push: return reject reasons via a mask
Pass all rejection reasons back from transport_push(). The logic is
simpler and more flexible with regard to providing useful feedback.
Signed-off-by: Chris Rorvick ch...@rorvick.com
---
builtin/push.c | 13 -
builtin/send-pack.c | 4 ++--
transport.c | 17 -
transport.h | 9 +
4 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..eaeaf7e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -244,7 +244,7 @@ static void advise_checkout_pull_push(void)
static int push_with_options(struct transport *transport, int flags)
{
int err;
- int nonfastforward;
+ unsigned int reject_mask;
transport_set_verbosity(transport, verbosity, progress);
@@ -257,7 +257,7 @@ static int push_with_options(struct transport *transport,
int flags)
if (verbosity 0)
fprintf(stderr, _(Pushing to %s\n), transport-url);
err = transport_push(transport, refspec_nr, refspec, flags,
-nonfastforward);
+reject_mask);
if (err != 0)
error(_(failed to push some refs to '%s'), transport-url);
@@ -265,18 +265,13 @@ static int push_with_options(struct transport *transport,
int flags)
if (!err)
return 0;
- switch (nonfastforward) {
- default:
- break;
- case NON_FF_HEAD:
+ if (reject_mask NON_FF_HEAD) {
advise_pull_before_push();
- break;
- case NON_FF_OTHER:
+ } else if (reject_mask NON_FF_OTHER) {
if (default_matching_used)
advise_use_upstream();
else
advise_checkout_pull_push();
- break;
}
return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index d342013..fda28bc 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -85,7 +85,7 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
int send_all = 0;
const char *receivepack = git-receive-pack;
int flags;
- int nonfastforward = 0;
+ unsigned int reject_mask;
int progress = -1;
argv++;
@@ -223,7 +223,7 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
ret |= finish_connect(conn);
if (!helper_status)
- transport_print_push_status(dest, remote_refs, args.verbose, 0,
nonfastforward);
+ transport_print_push_status(dest, remote_refs, args.verbose, 0,
reject_mask);
if (!args.dry_run remote) {
struct ref *ref;
diff --git a/transport.c b/transport.c
index 9932f40..ae9fda8 100644
--- a/transport.c
+++ b/transport.c
@@ -714,7 +714,7 @@ static int print_one_push_status(struct ref *ref, const
char *dest, int count, i
}
void transport_print_push_status(const char *dest, struct ref *refs,
- int verbose, int porcelain, int
*nonfastforward)
+ int verbose, int porcelain, unsigned int
*reject_mask)
{
struct ref *ref;
int n = 0;
@@ -733,18 +733,17 @@ void transport_print_push_status(const char *dest, struct
ref *refs,
if (ref-status == REF_STATUS_OK)
n += print_one_push_status(ref, dest, n, porcelain);
- *nonfastforward = 0;
+ *reject_mask = 0;
for (ref = refs; ref; ref = ref-next) {
if (ref-status != REF_STATUS_NONE
ref-status != REF_STATUS_UPTODATE
ref-status != REF_STATUS_OK)
n += print_one_push_status(ref, dest, n, porcelain);
- if (ref-status == REF_STATUS_REJECT_NONFASTFORWARD
- *nonfastforward != NON_FF_HEAD) {
+ if (ref-status == REF_STATUS_REJECT_NONFASTFORWARD) {
if (!strcmp(head, ref-name))
- *nonfastforward = NON_FF_HEAD;
+ *reject_mask |= NON_FF_HEAD;
else
- *nonfastforward = NON_FF_OTHER;
+ *reject_mask |= NON_FF_OTHER;
}
}
}
@@ -1031,9 +1030,9 @@ static void die_with_unpushed_submodules(struct
string_list *needs_pushing)
int transport_push(struct transport *transport,
int refspec_nr, const char **refspec, int flags,
- int *nonfastforward)
+ unsigned int *reject_mask)
{
- *nonfastforward = 0;
+ *reject_mask = 0;
transport_verify_remote_names(refspec_nr, refspec);
if (transport-push) {
@@ -1099,7 +1098,7 @@ int transport_push(struct transport *transport,
if (!quiet || err)
transport_print_push_status(transport-url, remote_refs,
[PATCH v4 4/5] push: flag updates that require force
Add a flag for indicating an update to a reference requires force.
Currently the nonfastforward flag of a ref is used for this when
generating status the status message. A separate flag insulates the
status logic from the details of set_ref_status_for_push().
Signed-off-by: Chris Rorvick ch...@rorvick.com
---
cache.h | 4 +++-
remote.c| 11 ---
transport.c | 2 +-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index 0f53d11..f410d94 100644
--- a/cache.h
+++ b/cache.h
@@ -999,7 +999,9 @@ struct ref {
unsigned char old_sha1[20];
unsigned char new_sha1[20];
char *symref;
- unsigned int force:1,
+ unsigned int
+ force:1,
+ requires_force:1,
merge:1,
nonfastforward:1,
is_a_tag:1,
diff --git a/remote.c b/remote.c
index fe89869..1e263b0 100644
--- a/remote.c
+++ b/remote.c
@@ -1285,6 +1285,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int
send_mirror,
struct ref *ref;
for (ref = remote_refs; ref; ref = ref-next) {
+ int force_ref_update = ref-force || force_update;
+
if (ref-peer_ref)
hashcpy(ref-new_sha1, ref-peer_ref-new_sha1);
else if (!send_mirror)
@@ -1327,9 +1329,12 @@ void set_ref_status_for_push(struct ref *remote_refs,
int send_mirror,
!has_sha1_file(ref-old_sha1)
|| !ref_newer(ref-new_sha1, ref-old_sha1);
- if (ref-nonfastforward !ref-force
!force_update) {
- ref-status = REF_STATUS_REJECT_NONFASTFORWARD;
- continue;
+ if (ref-nonfastforward) {
+ ref-requires_force = 1;
+ if (!force_ref_update) {
+ ref-status =
REF_STATUS_REJECT_NONFASTFORWARD;
+ continue;
+ }
}
}
}
diff --git a/transport.c b/transport.c
index 5fcaea8..60a7421 100644
--- a/transport.c
+++ b/transport.c
@@ -659,7 +659,7 @@ static void print_ok_ref_status(struct ref *ref, int
porcelain)
const char *msg;
strcpy(quickref, status_abbrev(ref-old_sha1));
- if (ref-nonfastforward) {
+ if (ref-requires_force) {
strcat(quickref, ...);
type = '+';
msg = forced update;
--
1.8.0.155.g3a063ad.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 5/5] push: update remote tags only with force
References are allowed to update from one commit-ish to another if the
former is a ancestor of the latter. This behavior is oriented to
branches which are expected to move with commits. Tag references are
expected to be static in a repository, though, thus an update to a
tag (lightweight and annotated) should be rejected unless the update is
forced.
To enable this functionality, the following checks have been added to
set_ref_status_for_push() for updating refs (i.e, not new or deletion)
to restrict fast-forwarding in pushes:
1) The old and new references must be commits. If this fails,
it is not a valid update for a branch.
2) The reference name cannot start with refs/tags/. This
catches lightweight tags which (usually) point to commits
and therefore would not be caught by (1).
If either of these checks fails, then it is flagged (by default) with a
status indicating the update is being rejected due to the reference
already existing in the remote. This can be overridden by passing
--force to git push.
Signed-off-by: Chris Rorvick ch...@rorvick.com
---
Documentation/git-push.txt | 10 +-
builtin/push.c | 2 +-
builtin/send-pack.c| 5 +
cache.h| 3 ++-
remote.c | 23 +++
send-pack.c| 1 +
t/t5516-fetch-push.sh | 30 +-
transport-helper.c | 6 ++
transport.c| 8 ++--
9 files changed, 74 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index fe46c42..479e25f 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -51,11 +51,11 @@ be named. If `:`dst is omitted, the same ref as src
will be
updated.
+
The object referenced by src is used to update the dst reference
-on the remote side, but by default this is only allowed if the
-update can fast-forward dst. By having the optional leading `+`,
-you can tell git to update the dst ref even when the update is not a
-fast-forward. This does *not* attempt to merge src into dst. See
-EXAMPLES below for details.
+on the remote side. By default this is only allowed if the update is
+a branch, and then only if it can fast-forward dst. By having the
+optional leading `+`, you can tell git to update the dst ref even when
+the update is not a branch or it is not a fast-forward. This does *not*
+attempt to merge src into dst. See EXAMPLES below for details.
+
`tag tag` means the same as `refs/tags/tag:refs/tags/tag`.
+
diff --git a/builtin/push.c b/builtin/push.c
index 0e13e11..cd7aa3f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -222,7 +222,7 @@ static const char message_advice_checkout_pull_push[] =
static const char message_advice_ref_already_exists[] =
N_(Updates were rejected because a matching reference already exists
in\n
- the remote and the update is not a fast-forward.);
+ the remote.);
static void advise_pull_before_push(void)
{
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index fda28bc..1eabf42 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -44,6 +44,11 @@ static void print_helper_status(struct ref *ref)
msg = non-fast forward;
break;
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ res = error;
+ msg = already exists;
+ break;
+
case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = error;
diff --git a/cache.h b/cache.h
index f410d94..127e504 100644
--- a/cache.h
+++ b/cache.h
@@ -1004,13 +1004,14 @@ struct ref {
requires_force:1,
merge:1,
nonfastforward:1,
- is_a_tag:1,
+ forwardable:1,
update:1,
deletion:1;
enum {
REF_STATUS_NONE = 0,
REF_STATUS_OK,
REF_STATUS_REJECT_NONFASTFORWARD,
+ REF_STATUS_REJECT_ALREADY_EXISTS,
REF_STATUS_REJECT_NODELETE,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
diff --git a/remote.c b/remote.c
index 1e263b0..4fcd22c 100644
--- a/remote.c
+++ b/remote.c
@@ -1311,14 +1311,23 @@ void set_ref_status_for_push(struct ref *remote_refs,
int send_mirror,
* to overwrite it; you would not know what you are losing
* otherwise.
*
-* (3) if both new and old are commit-ish, and new is a
-* descendant of old, it is OK.
+* (3) if new is commit-ish and old is a commit, new is a
+* descendant of old, and the reference is not of the
+* refs/tags/ hierarchy it is OK.
*
* (4) regardless of all of the
[PATCH v4 3/5] push: flag updates
If the reference exists on the remote and the the update is not a
delete, then mark as an update. This is in preparation for handling
tags and branches differently when pushing.
Signed-off-by: Chris Rorvick ch...@rorvick.com
---
cache.h | 1 +
remote.c | 18 +++---
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 82dead1..0f53d11 100644
--- a/cache.h
+++ b/cache.h
@@ -1003,6 +1003,7 @@ struct ref {
merge:1,
nonfastforward:1,
is_a_tag:1,
+ update:1,
deletion:1;
enum {
REF_STATUS_NONE = 0,
diff --git a/remote.c b/remote.c
index 186814d..fe89869 100644
--- a/remote.c
+++ b/remote.c
@@ -1318,15 +1318,19 @@ void set_ref_status_for_push(struct ref *remote_refs,
int send_mirror,
ref-is_a_tag = !prefixcmp(ref-name, refs/tags/);
- ref-nonfastforward =
+ ref-update =
!ref-deletion
- !is_null_sha1(ref-old_sha1)
- (!has_sha1_file(ref-old_sha1)
- || !ref_newer(ref-new_sha1, ref-old_sha1));
+ !is_null_sha1(ref-old_sha1);
- if (ref-nonfastforward !ref-force !force_update) {
- ref-status = REF_STATUS_REJECT_NONFASTFORWARD;
- continue;
+ if (ref-update) {
+ ref-nonfastforward =
+ !has_sha1_file(ref-old_sha1)
+ || !ref_newer(ref-new_sha1, ref-old_sha1);
+
+ if (ref-nonfastforward !ref-force
!force_update) {
+ ref-status = REF_STATUS_REJECT_NONFASTFORWARD;
+ continue;
+ }
}
}
}
--
1.8.0.155.g3a063ad.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/6] completion: consolidate test_completion*() tests
On Sat, Nov 17, 2012 at 12:41 AM, Junio C Hamano gits...@pobox.com wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
No need to have two versions; if a second argument is specified, use
that, otherwise use stdin.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
t/t9902-completion.sh | 30 +-
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 204c92a..59cdbfd 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -60,19 +60,15 @@ run_completion ()
# 2: expected completion
test_completion ()
{
- test $# -gt 1 echo $2 expected
+ if [ $# -gt 1 ]; then
+ echo $2 expected
+ else
+ sed -e 's/Z$//' expected
+ fi
As $2 could begin with dash, end with \c, etc. that possibly can
be misinterpred by echo, I'd rewrite this as
printf '%s\n' $2 expected
Otherwise looked fine; thanks.
But that was the case before. I would do that in a separate patch.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
On Sat, Nov 17, 2012 at 02:20:27PM -0500, W. Trevor King wrote: On Sat, Nov 17, 2012 at 04:30:07PM +0100, Heiko Voigt wrote: (2) git diff [$path] and friends in the superproject compares the HEAD of thecheckout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject. Hmm. ???git diff??? compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like: $ git submodule foreach 'git diff origin/$submodule_branch' Perhaps this is enough motivation for keeping $submodule_* exports? and the option were called something like --follow-branch=$branch, ??? I am not sure if hiding changes to the recorded SHA1 from the user is such a useful thing. In the first step I would like it if it was kept simple and only the submodule update machinery learned to follow a branch. If that results in local changes that should be shown. The user is still in charge of recording the updated SHA1 in his commit. I understand what you're warning against here, or what it has to do with git diff. Is there a not missing here? Reads somehow like that. What I am talking about is the suggestion of Junio. Instead of showing a diff if the SHA1 is different we show a diff if the checkout in the worktree is different from the tip of the configured branch. That would hide the fact that a submodule has changed during a submodule update operation. From what I have heard of projects using this: They usually still have something that records the SHA1s on a regular basis. Thinking further, why not record them in git? We could add an option to update which creates such a commit. I think it's best to have users craft their own commit messages explaining why the branch was updated. That said, an auto-generated hint (a la git merge) would probably be a useful extra feature. I have the same opinion. Commits should always be created by humans so you have someone to blame/ask why. But I guess there are people that expect this to be automatic. One argument somehow goes along the lines: I already created a commit in the submodule why do I need to create another one in the superproject? Just follow the HEAD revision! They think in subversions submodules which are merely pointers to other svn repositories without any revision information. I am unsure if its good to support this the same way. Another use case is big projects that have so many submodules that creating superproject commits would create to much maintenance work. They want to have their integration server make those commits. That would already be supported with update checking out the branch tips and the commit is just one extra thing to do by the integration server. So I think it should be fine just to teach update to checkout the configured branch tips (or forward them to their tracking branch tips) and leave the rest to the user. Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4.1 5/5] push: update remote tags only with force
References are allowed to update from one commit-ish to another if the
former is a ancestor of the latter. This behavior is oriented to
branches which are expected to move with commits. Tag references are
expected to be static in a repository, though, thus an update to a
tag (lightweight and annotated) should be rejected unless the update is
forced.
To enable this functionality, the following checks have been added to
set_ref_status_for_push() for updating refs (i.e, not new or deletion)
to restrict fast-forwarding in pushes:
1) The old and new references must be commits. If this fails,
it is not a valid update for a branch.
2) The reference name cannot start with refs/tags/. This
catches lightweight tags which (usually) point to commits
and therefore would not be caught by (1).
If either of these checks fails, then it is flagged (by default) with a
status indicating the update is being rejected due to the reference
already existing in the remote. This can be overridden by passing
--force to git push.
Signed-off-by: Chris Rorvick ch...@rorvick.com
---
Fix C99 comment.
Documentation/git-push.txt | 10 +-
builtin/push.c | 2 +-
builtin/send-pack.c| 5 +
cache.h| 3 ++-
remote.c | 24
send-pack.c| 1 +
t/t5516-fetch-push.sh | 42 +-
transport-helper.c | 6 ++
transport.c| 8 ++--
9 files changed, 87 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index fe46c42..479e25f 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -51,11 +51,11 @@ be named. If `:`dst is omitted, the same ref as src
will be
updated.
+
The object referenced by src is used to update the dst reference
-on the remote side, but by default this is only allowed if the
-update can fast-forward dst. By having the optional leading `+`,
-you can tell git to update the dst ref even when the update is not a
-fast-forward. This does *not* attempt to merge src into dst. See
-EXAMPLES below for details.
+on the remote side. By default this is only allowed if the update is
+a branch, and then only if it can fast-forward dst. By having the
+optional leading `+`, you can tell git to update the dst ref even when
+the update is not a branch or it is not a fast-forward. This does *not*
+attempt to merge src into dst. See EXAMPLES below for details.
+
`tag tag` means the same as `refs/tags/tag:refs/tags/tag`.
+
diff --git a/builtin/push.c b/builtin/push.c
index 0e13e11..cd7aa3f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -222,7 +222,7 @@ static const char message_advice_checkout_pull_push[] =
static const char message_advice_ref_already_exists[] =
N_(Updates were rejected because a matching reference already exists
in\n
- the remote and the update is not a fast-forward.);
+ the remote.);
static void advise_pull_before_push(void)
{
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index fda28bc..1eabf42 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -44,6 +44,11 @@ static void print_helper_status(struct ref *ref)
msg = non-fast forward;
break;
+ case REF_STATUS_REJECT_ALREADY_EXISTS:
+ res = error;
+ msg = already exists;
+ break;
+
case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_REMOTE_REJECT:
res = error;
diff --git a/cache.h b/cache.h
index f410d94..127e504 100644
--- a/cache.h
+++ b/cache.h
@@ -1004,13 +1004,14 @@ struct ref {
requires_force:1,
merge:1,
nonfastforward:1,
- is_a_tag:1,
+ forwardable:1,
update:1,
deletion:1;
enum {
REF_STATUS_NONE = 0,
REF_STATUS_OK,
REF_STATUS_REJECT_NONFASTFORWARD,
+ REF_STATUS_REJECT_ALREADY_EXISTS,
REF_STATUS_REJECT_NODELETE,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
diff --git a/remote.c b/remote.c
index 44e72d6..a723f7a 100644
--- a/remote.c
+++ b/remote.c
@@ -1311,14 +1311,24 @@ void set_ref_status_for_push(struct ref *remote_refs,
int send_mirror,
* to overwrite it; you would not know what you are losing
* otherwise.
*
-* (3) if both new and old are commit-ish, and new is a
-* descendant of old, it is OK.
+* (3) if new is commit-ish and old is a commit, new is a
+* descendant of old, and the reference is not of the
+* refs/tags/ hierarchy it is OK.
*
*
Re: Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
On Sat, Nov 17, 2012 at 10:31:30PM +0100, Heiko Voigt wrote: On Sat, Nov 17, 2012 at 02:20:27PM -0500, W. Trevor King wrote: On Sat, Nov 17, 2012 at 04:30:07PM +0100, Heiko Voigt wrote: (2) git diff [$path] and friends in the superproject compares the HEAD of thecheckout of the submodule at $path with the tip of the branch named by submodule.$name.branch in .gitmodules of the superproject, instead of the commit that is recorded in the index of the superproject. Hmm. ???git diff??? compares the working tree with the local HEAD (just a SHA for submodules), so I don't think it should care about the status of a remote branch. This sounds like you want something like: $ git submodule foreach 'git diff origin/$submodule_branch' Perhaps this is enough motivation for keeping $submodule_* exports? and the option were called something like --follow-branch=$branch, ??? I am not sure if hiding changes to the recorded SHA1 from the user is such a useful thing. In the first step I would like it if it was kept simple and only the submodule update machinery learned to follow a branch. If that results in local changes that should be shown. The user is still in charge of recording the updated SHA1 in his commit. I understand what you're warning against here, or what it has to do with git diff. Is there a not missing here? Thanks. I'd meant to say I don't understand…. What I am talking about is the suggestion of Junio. Instead of showing a diff if the SHA1 is different we show a diff if the checkout in the worktree is different from the tip of the configured branch. That would hide the fact that a submodule has changed during a submodule update operation. Ahh, now I understand. I agree that comparing to the remote tip is a bad idea. -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/7] completion: make the 'basic' test more tester-friendly
SZEDER Gábor wrote: The 'basic' test uses 'grep -q' to filter the resulting possible completion words while looking for the presence or absence of certain git commands, and relies on grep's exit status to indicate a failure. [...] To make testers' life easier provide some output about the failed condition: store the results of the filtering in a file and compare its contents to the expected results by the good old test_cmp helper. Looks good. I wonder if this points to the need for a test_grep helper more generally. [...] run_completion git f - ! grep -q -v ^f out + expected + sed -n /^[^f]/p out out2 + test_cmp expected out2 Functional change: before, this would fail if out contained a blank line, while afterward it does not. I doubt that matters, though. Thanks and hope that helps, Jonathan -- To unsubscribe from this list: send the line 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/7] completion: fix args of run_completion() test helper
SZEDER Gábor wrote: Fix this by passing the command line to run_completion() as separate words. Good catch. The change makes sense, the code looks saner after the fix, and since this is test code any breakage should be caught quickly, so Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] completion: add tests for the __gitcomp_nl() completion helper function
SZEDER Gábor wrote: --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -155,6 +155,90 @@ test_expect_success '__gitcomp - suffix' ' test_cmp expected out ' +test_expect_success '__gitcomp_nl - trailing space' ' + sed -e s/Z$// expected -\EOF '$' is usually a shell metacharacter, so it would be more comfortable to read a version that escapes it: sed -e s/Z\$// expected -\EOF Since '$/' is not a valid parameter expansion, if I understand correctly then POSIX leaves the meaning of the quoted string s/Z$// undefined (XCU §2.2.3). Luckily every shell I've tried, including bash, keeps the $ unmolested. Other parts of the file already use the same style, so I wouldn't suggest changing it in this patch. Thanks for some nice tests. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line 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/7] completion: add tests for invalid variable name among completion words
SZEDER Gábor wrote:
The breakage can
be simply bogus possible completion words, but it can also be a
failure:
$ git branch '${foo.bar}'
$ git checkout TAB
bash: ${foo.bar}: bad substitution
Or arbitrary code execution:
$ git branch '$(kilroy-was-here)'
$ git checkout TAB
$ ls -l kilroy-was-here
-rw-rw-r-- 1 jrn jrn 0 nov 17 15:40 kilroy-was-here
The final version of this patch should go to maint. Thanks for
catching it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/5] completion: get rid of compgen
On Sat, Nov 17, 2012 at 8:33 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sat, Nov 17, 2012 at 3:12 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 12:42:38PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
The functionality we use is very simple, plus, this fixes a known
breakage 'complete tree filename with metacharacters'.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
contrib/completion/git-completion.bash | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 975ae13..ad3e1fe 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -227,7 +227,11 @@ fi
__gitcompadd ()
{
- COMPREPLY=($(compgen -W $1 -P $2 -S $4 -- $3))
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
The whole point of creating __gitcomp_nl() back then was to fill
COMPREPLY without iterating through all words in the wordlist, making
completion faster for large number of words, e.g. a lot of refs, or
later a lot of symbols for 'git grep' in a larger project.
The loop here kills that optimization.
So your solution is to move the loop to awk? I fail to see how that
could bring more optimization, specially since it includes an extra
fork now.
This patch didn't aim for more optimization, but it was definitely a
goal not to waste what we gained by creating __gitcomp_nl() in
a31e6262 (completion: optimize refs completion, 2011-10-15). However,
as it turns out the new version with awk is actually faster than
current master with compgen:
Before:
$ refs=$(for i in {0..} ; do echo branch$i ; done)
$ time __gitcomp_nl $refs
real0m0.242s
user0m0.220s
sys 0m0.028s
After:
$ time __gitcomp_nl $refs
real0m0.109s
user0m0.096s
sys 0m0.012s
This one is even faster:
while read -r x; do
if [[ $x == $3* ]]; then
COMPREPLY+=($2$x$4)
fi
done $1
== 1 ==
one:
real0m0.148s
user0m0.134s
sys 0m0.025s
two:
real0m0.055s
user0m0.054s
sys 0m0.000s
Ah, nevermind, I didn't quote the $1.
However, this one is quite close and much simpler:
local IFS=$'\n'
COMPREPLY=($(printf -- $2%s$4\n $1 | grep ^$2$3))
== 1 ==
awk 1:
real0m0.064s
user0m0.062s
sys 0m0.003s
printf:
real0m0.069s
user0m0.064s
sys 0m0.020s
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
On Sat, Nov 17, 2012 at 8:28 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sat, Nov 17, 2012 at 8:08 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Sat, Nov 17, 2012 at 3:14 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sat, Nov 17, 2012 at 12:50:39PM +0100, Felipe Contreras wrote:
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor sze...@ira.uka.de wrote:
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 --
${3-$cur}))
+ COMPREPLY=($(awk -v pfx=${2-} -v sfx=${4- } -v
cur=${3-$cur} '
+ BEGIN {
+ FS=\n;
+ len=length(cur);
+ }
+ {
+ if (cur == substr($1, 1, len))
+ print pfx$1sfx;
+ }' $1 ))
}
Does this really perform better than my alternative?
+ for x in $1; do
+ if [[ $x = $3* ]]; then
+ COMPREPLY+=($2$x$4)
+ fi
+ done
It does:
My version:
$ refs=$(for i in {0..} ; do echo branch$i ; done)
$ time __gitcomp_nl $refs
real0m0.109s
user0m0.096s
sys 0m0.012s
Yours:
$ time __gitcomp_nl $refs
real0m0.321s
user0m0.312s
sys 0m0.008s
Yeah, for 1 refs, which is not the common case:
And this beats both in every case:
while read -r x; do
if [[ $x == $3* ]]; then
COMPREPLY+=($2$x$4)
fi
done $1
Nevermind that.
Here's another:
local IFS=$'\n'
COMPREPLY=($(printf -- $2%s$4\n $1 | grep ^$2$3))
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor sze...@ira.uka.de wrote:
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
+ COMPREPLY=($(awk -v pfx=${2-} -v sfx=${4- } -v cur=${3-$cur} '
+ BEGIN {
+ FS=\n;
+ len=length(cur);
+ }
+ {
+ if (cur == substr($1, 1, len))
+ print pfx$1sfx;
+ }' $1 ))
}
This version is simpler and faster:
local IFS=$'\n'
COMPREPLY=($(awk -v cur=${3-$cur} -v pre=${2-} -v suf=${4- }
'$0 ~ cur { print pre$0suf }' $1 ))
== 1 ==
awk 1:
real0m0.067s
user0m0.066s
sys 0m0.001s
awk 2:
real0m0.057s
user0m0.055s
sys 0m0.002s
--
Felipe Contreras
--
To unsubscribe from this list: send the line 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: t5801-remote-helpers.sh fails
On 10.11.12 23:05, Felipe Contreras wrote:
On Sat, Nov 10, 2012 at 8:20 PM, Torsten Bögershausen tbo...@web.de wrote:
On 11/10/2012 08:15 PM, Felipe Contreras wrote:
Hi,
On Sat, Nov 10, 2012 at 2:48 PM, Torsten Bögershausen tbo...@web.de
wrote:
on peff/pu t5801 fails, the error is in git-remote-testgit, please see
below.
That's on my Mac OS X box.
I haven't digged further into the test case, but it looks as if
[-+]A make NAMEs associative arrays
is not supported on this version of bash.
/Torsten
/Users/tb/projects/git/git.peff/git-remote-testgit: line 64: declare: -A:
invalid option
declare: usage: declare [-afFirtx] [-p] [name[=value] ...]
/Users/tb/projects/git/git.peff/git-remote-testgit: line 66:
refs/heads/master: division by 0 (error token is /master)
error: fast-export died of signal 13
fatal: Error while running fast-export
What is your bash --version?
bash --version
GNU bash, version 3.2.48(1)-release (x86_64-apple-darwin10.0)
Copyright (C) 2007 Free Software Foundation, Inc.
I see, that version indeed doesn't have associative arrays.
On the other hand, Documentation/CodingGuidelines says:
- No shell arrays.
Yeah, for shell code I guess, but this is bash code.
Could we use perl to have arrays?
I think the code in perl would be harder to follow, and this is meant
not only as a test, but also as a reference.
I'm not exactly sure what we should do here:
a) remove the code (would not be so good as a reference)
b) enable the code conditionally based on the version of bash (harder to read)
c) replace the code without associative arrays (will be much more
complicated and ugly)
d) add a check for the bash version to the top of the test in t/
I'm leaning towards d), followed by b).
If only there was a clean way to do this, so c) would not be so ugly.
After investigating different optins this seems to be the best:
join -e empty -o '0 1.2 2.2' -a 2 (echo $before) (echo $after)
| while read e a b; do
test $a == $b continue
echo changed $e
done
But to me seems a bit harder to grasp. Not sure.
Cheers.
Hi again,
I managed to have a working solution for
d) add a check for the bash version to the top of the test in t/
Please see diff below.
This unbreaks the test suite here.
Is this a good way forward?
Filipe, does the code line you mention above work for you?
If yes: I can test it here, if you send it as a patch.
/Torsten
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
old mode 100644
new mode 100755
index 6e4e078..ea3d0f3
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -13,6 +13,15 @@ compare_refs() {
test_cmp expect actual
}
+cat testbashArray -EOF
+ declare -A assa
+EOF
+
+/bin/bash testbashArray || {
+ skip_all='t5801. /bin/bash does not handle associative arrays'
+ test_done
+}
+
test_expect_success 'setup repository' '
git init server
(cd server
--
To unsubscribe from this list: send the line 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] Rename V15_MINGW_HEADERS into CYGWIN_OLD_WINSOCK_HEADERS
Mark Levedahl mleved...@gmail.com writes: ... -V15_MINGW_HEADERS = YesPlease +CYGWIN_OLD_WINSOCK_HEADERS = YesPlease WINSOCK is certainly a part of the win32 api implementation, but it is is the entire win32api that changed, not just the tiny bit dealing with sockets. Basically, WINDOWS.h, and everything it includes, and all of the dlls it touches, and the .o files, changed. ... So my suggestion in the bike shedding category is to s/V15_MINGW_HEADERS/CYGWIN_V15_WIN32API/ Sounds sensible. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
