* git-credential-gnome-keyring fails at multilib
From: Peter Alfredsen @ 2012-11-17 14:05 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]
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
[-- Attachment #2: git-1.8.0-gnome-keyring-multilib.patch --]
[-- Type: application/octet-stream, Size: 597 bytes --]
diff -NrU5 git-1.8.0.orig/contrib/credential/gnome-keyring/Makefile git-1.8.0/contrib/credential/gnome-keyring/Makefile
--- git-1.8.0.orig/contrib/credential/gnome-keyring/Makefile 2012-11-17 14:45:06.536641381 +0100
+++ git-1.8.0/contrib/credential/gnome-keyring/Makefile 2012-11-17 14:45:40.883442939 +0100
@@ -1,11 +1,11 @@
MAIN:=git-credential-gnome-keyring
all:: $(MAIN)
-CC = gcc
-RM = rm -f
-CFLAGS = -g -O2 -Wall
+CC ?= gcc
+RM ?= rm -f
+CFLAGS ?= -g -O2 -Wall
-include ../../../config.mak.autogen
-include ../../../config.mak
INCS:=$(shell pkg-config --cflags gnome-keyring-1)
^ permalink raw reply
* Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: Felipe Contreras @ 2012-11-17 11:50 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-6-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@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
^ permalink raw reply
* Re: [PATCH] tcsh-completion re-using git-completion.bash
From: Felipe Contreras @ 2012-11-17 11:46 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Marc Khouzam, git
In-Reply-To: <20121117105605.GB12052@goldbirke>
On Sat, Nov 17, 2012 at 11:56 AM, SZEDER Gábor <szeder@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
^ permalink raw reply
* Re: [RFC/PATCH 4/5] completion: get rid of compgen
From: Felipe Contreras @ 2012-11-17 11:42 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <20121117110031.GE12052@goldbirke>
On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor <szeder@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.contreras@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
^ permalink raw reply
* Re: [PATCH 6/7] completion: fix expansion issue in __gitcomp()
From: Felipe Contreras @ 2012-11-17 11:39 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-7-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@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.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1
From: Felipe Contreras @ 2012-11-17 11:27 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <20121117105837.GC12052@goldbirke>
On Sat, Nov 17, 2012 at 11:58 AM, SZEDER Gábor <szeder@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
^ permalink raw reply
* [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
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 <jjgmeijer@hotmail.com>
Signed-off-by: SZEDER Gábor <szeder@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
^ permalink raw reply related
* [PATCH 7/7] completion: remove the now unused __gitcomp_1() internal helper function
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
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 <szeder@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
^ permalink raw reply related
* [PATCH 6/7] completion: fix expansion issue in __gitcomp()
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
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 <szeder@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
^ permalink raw reply related
* [PATCH 4/7] completion: add tests for invalid variable name among completion words
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
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 <jjgmeijer@hotmail.com>
Signed-off-by: SZEDER Gábor <szeder@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
^ permalink raw reply related
* [PATCH 1/7] completion: make the 'basic' test more tester-friendly
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
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 <szeder@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
^ permalink raw reply related
* [PATCH 3/7] completion: add tests for the __gitcomp_nl() completion helper function
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
Test __gitcomp_nl()'s basic functionality, i.e. that trailing space,
prefix, and suffix are added correctly.
Signed-off-by: SZEDER Gábor <szeder@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
^ permalink raw reply related
* [PATCH 2/7] completion: fix args of run_completion() test helper
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
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 <szeder@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
^ permalink raw reply related
* [PATCH 0/7] completion: fix expansion issues with compgen
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
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
^ permalink raw reply
* Re: [RFC/PATCH 4/5] completion: get rid of compgen
From: SZEDER Gábor @ 2012-11-17 11:00 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353116298-11798-5-git-send-email-felipe.contreras@gmail.com>
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.contreras@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
^ permalink raw reply
* Re: [RFC/PATCH 3/5] completion: trivial test improvement
From: SZEDER Gábor @ 2012-11-17 10:59 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353116298-11798-4-git-send-email-felipe.contreras@gmail.com>
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.contreras@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
>
^ permalink raw reply
* Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1
From: SZEDER Gábor @ 2012-11-17 10:58 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353116298-11798-6-git-send-email-felipe.contreras@gmail.com>
[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.contreras@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
>
^ permalink raw reply
* Re: [PATCH] tcsh-completion re-using git-completion.bash
From: SZEDER Gábor @ 2012-11-17 10:56 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Marc Khouzam, git
In-Reply-To: <CAMP44s3pi0iDOho_qYZEutebDNDveWWv6wEAs-C1bs1A_yL+Sg@mail.gmail.com>
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 <szeder@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.
^ permalink raw reply
* Re: Auto-repo-repair
From: Enrico Weigelt @ 2012-11-17 9:21 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121116190004.GA2310@sigill.intra.peff.net>
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.weigelt@vnc.biz; www.vnc.de
^ permalink raw reply
* Re: [PATCH] Add support for a 'pre-push' hook
From: Aske Olsson @ 2012-11-17 9:00 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpq625547ne.fsf@grenoble-inp.fr>
On Fri, Nov 16, 2012 at 9:25 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Aske Olsson <askeolsson@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
^ permalink raw reply
* Re: [PATCH] Add support for a 'pre-push' hook
From: Aske Olsson @ 2012-11-17 8:47 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Junio C Hamano, git
In-Reply-To: <50A73120.9040301@alum.mit.edu>
On Sat, Nov 17, 2012 at 7:39 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>
> On 11/16/2012 09:30 PM, Junio C Hamano wrote:
> > Aske Olsson <askeolsson@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 <askeolsson@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
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Torsten Bögershausen @ 2012-11-17 7:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, Jeff King, Mark Levedahl, git
In-Reply-To: <7vlie1mlcb.fsf@alter.siamese.dyndns.org>
On 16.11.12 19:52, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> My understanding:
>> Either use people cygwin 1.5 or they use cygwin 1.7, and in this case
>> the installation is updated frequently.
>>
>> Peff or Junio, please go ahead with the patch.
>>
>> If it turns out that we want to support cygwin installations like 1.7.7
>> which could be upgraded, but are not upgraded since they are
>> "production machines we do not dare to touch" we can still improve
>> the autodetection.
>
> OK. I moved the topic forward but we may still want to rename the
> name of the macro to have CYGWIN somewhere in the name.
I send a patch some seconds ago.
I forgot to mention that this should be applied to next
^ permalink raw reply
* [PATCH] Rename V15_MINGW_HEADERS into CYGWIN_OLD_WINSOCK_HEADERS
From: Torsten Bögershausen @ 2012-11-17 7:09 UTC (permalink / raw)
To: git; +Cc: mlevedahl, tboegi
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
Addtional note:
Cygwin versions 1.7.1 up to 1.7.16 are expected to upgrade to
Cygwin 1.7.17 or higher
As a temporary workaround make can be run as
CYGWIN_OLD_WINSOCK_HEADERS=Yes make
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Makefile | 6 +++---
compat/cygwin.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
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
endif
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
@@ -1901,8 +1901,8 @@ ifdef NO_REGEX
COMPAT_CFLAGS += -Icompat/regex
COMPAT_OBJS += compat/regex/regex.o
endif
-ifdef V15_MINGW_HEADERS
- COMPAT_CFLAGS += -DV15_MINGW_HEADERS
+ifdef CYGWIN_OLD_WINSOCK_HEADERS
+ COMPAT_CFLAGS += -DCYGWIN_OLD_WINSOCK_HEADERS
endif
ifdef USE_NED_ALLOCATOR
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 59d86e4..b9f2862 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,5 +1,5 @@
#define WIN32_LEAN_AND_MEAN
-#ifdef V15_MINGW_HEADERS
+#ifdef CYGWIN_OLD_WINSOCK_HEADERS
#include "../git-compat-util.h"
#include "win32.h"
#else
--
1.7.12
^ permalink raw reply related
* Re: [PATCH] Add support for a 'pre-push' hook
From: Michael Haggerty @ 2012-11-17 6:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Aske Olsson, git
In-Reply-To: <7vvcd5l290.fsf@alter.siamese.dyndns.org>
On 11/16/2012 09:30 PM, Junio C Hamano wrote:
> Aske Olsson <askeolsson@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 <askeolsson@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.
> 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.
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).
* 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.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* [RFC/PATCH 5/5] completion: refactor __gitcomp_1
From: Felipe Contreras @ 2012-11-17 1:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Björn Gustavsson,
Shawn O. Pearce, Robert Zeh, Peter van der Does, Jonathan Nieder,
Felipe Contreras, Jeff King
In-Reply-To: <1353116298-11798-1-git-send-email-felipe.contreras@gmail.com>
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.contreras@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-}"`
+ if [[ "$c" = "$cur_"* ]]; then
+ COMPREPLY+=("${2-}$c")
+ fi
+ done
;;
esac
+
}
# Generates completion reply with compgen from newline-separated possible
--
1.8.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox