* [PATCH v3 6/6] completion: simplify __gitcomp() test helper
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>
By using print_comp as suggested by SZEDER Gábor.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t9902-completion.sh | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index fba632f..42db3da 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -69,23 +69,18 @@ test_completion ()
test_cmp expected out
}
-newline=$'\n'
-
# Test __gitcomp
# Arguments are:
# 1: current word (cur)
# -: the rest are passed to __gitcomp
test_gitcomp ()
{
+ local -a COMPREPLY &&
sed -e 's/Z$//' > expected &&
- (
- local -a COMPREPLY &&
- cur="$1" &&
- shift &&
- __gitcomp "$@" &&
- IFS="$newline" &&
- echo "${COMPREPLY[*]}" > out
- ) &&
+ cur="$1" &&
+ shift &&
+ __gitcomp "$@" &&
+ print_comp &&
test_cmp expected out
}
--
1.8.0
^ permalink raw reply related
* [PATCH v3 5/6] completion: refactor __gitcomp related tests
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>
Lots of duplicated code removed!
No functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t9902-completion.sh | 76 ++++++++++++++++++---------------------------------
1 file changed, 27 insertions(+), 49 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 90a9a91..fba632f 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -71,87 +71,65 @@ test_completion ()
newline=$'\n'
-test_expect_success '__gitcomp - trailing space - options' '
- sed -e "s/Z$//" >expected <<-\EOF &&
- --reuse-message=Z
- --reedit-message=Z
- --reset-author Z
- EOF
+# Test __gitcomp
+# Arguments are:
+# 1: current word (cur)
+# -: the rest are passed to __gitcomp
+test_gitcomp ()
+{
+ sed -e 's/Z$//' > expected &&
(
local -a COMPREPLY &&
- cur="--re" &&
- __gitcomp "--dry-run --reuse-message= --reedit-message=
- --reset-author" &&
+ cur="$1" &&
+ shift &&
+ __gitcomp "$@" &&
IFS="$newline" &&
echo "${COMPREPLY[*]}" > out
) &&
test_cmp expected out
+}
+
+test_expect_success '__gitcomp - trailing space - options' '
+ test_gitcomp "--re" "--dry-run --reuse-message= --reedit-message=
+ --reset-author" <<-EOF
+ --reuse-message=Z
+ --reedit-message=Z
+ --reset-author Z
+ EOF
'
test_expect_success '__gitcomp - trailing space - config keys' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp "br" "branch. branch.autosetupmerge
+ branch.autosetuprebase browser." <<-\EOF
branch.Z
branch.autosetupmerge Z
branch.autosetuprebase Z
browser.Z
EOF
- (
- local -a COMPREPLY &&
- cur="br" &&
- __gitcomp "branch. branch.autosetupmerge
- branch.autosetuprebase browser." &&
- IFS="$newline" &&
- echo "${COMPREPLY[*]}" > out
- ) &&
- test_cmp expected out
'
test_expect_success '__gitcomp - option parameter' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp "--strategy=re" "octopus ours recursive resolve subtree" \
+ "" "re" <<-\EOF
recursive Z
resolve Z
EOF
- (
- local -a COMPREPLY &&
- cur="--strategy=re" &&
- __gitcomp "octopus ours recursive resolve subtree
- " "" "re" &&
- IFS="$newline" &&
- echo "${COMPREPLY[*]}" > out
- ) &&
- test_cmp expected out
'
test_expect_success '__gitcomp - prefix' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp "branch.me" "remote merge mergeoptions rebase" \
+ "branch.maint." "me" <<-\EOF
branch.maint.merge Z
branch.maint.mergeoptions Z
EOF
- (
- local -a COMPREPLY &&
- cur="branch.me" &&
- __gitcomp "remote merge mergeoptions rebase
- " "branch.maint." "me" &&
- IFS="$newline" &&
- echo "${COMPREPLY[*]}" > out
- ) &&
- test_cmp expected out
'
test_expect_success '__gitcomp - suffix' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp "branch.me" "master maint next pu" "branch." \
+ "ma" "." <<-\EOF
branch.master.Z
branch.maint.Z
EOF
- (
- local -a COMPREPLY &&
- cur="branch.me" &&
- __gitcomp "master maint next pu
- " "branch." "ma" "." &&
- IFS="$newline" &&
- echo "${COMPREPLY[*]}" > out
- ) &&
- test_cmp expected out
'
test_expect_success 'basic' '
--
1.8.0
^ permalink raw reply related
* [PATCH v3 4/6] completion: consolidate test_completion*() tests
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>
No need to have two versions; if a second argument is specified, use
that, otherwise use stdin.
Signed-off-by: Felipe Contreras <felipe.contreras@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 6a250ad..90a9a91 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 &&
run_completion "$1" &&
test_cmp expected out
}
-# Like test_completion, but reads expectation from stdin,
-# which is convenient when it is multiline.
-test_completion_long ()
-{
- sed -e 's/Z$//' > expected &&
- test_completion "$1"
-}
-
newline=$'\n'
test_expect_success '__gitcomp - trailing space - options' '
@@ -172,7 +168,7 @@ test_expect_success 'basic' '
'
test_expect_success 'double dash "git" itself' '
- test_completion_long "git --" <<-\EOF
+ test_completion "git --" <<-\EOF
--paginate Z
--no-pager Z
--git-dir=
@@ -190,7 +186,7 @@ test_expect_success 'double dash "git" itself' '
'
test_expect_success 'double dash "git checkout"' '
- test_completion_long "git checkout --" <<-\EOF
+ test_completion "git checkout --" <<-\EOF
--quiet Z
--ours Z
--theirs Z
@@ -206,7 +202,7 @@ test_expect_success 'double dash "git checkout"' '
test_expect_success 'general options' '
test_completion "git --ver" "--version " &&
test_completion "git --hel" "--help " &&
- test_completion_long "git --exe" <<-\EOF &&
+ test_completion "git --exe" <<-\EOF &&
--exec-path Z
--exec-path=
EOF
@@ -247,7 +243,7 @@ test_expect_success 'setup for ref completion' '
'
test_expect_success 'checkout completes ref names' '
- test_completion_long "git checkout m" <<-\EOF
+ test_completion "git checkout m" <<-\EOF
master Z
mybranch Z
mytag Z
@@ -255,7 +251,7 @@ test_expect_success 'checkout completes ref names' '
'
test_expect_success 'show completes all refs' '
- test_completion_long "git show m" <<-\EOF
+ test_completion "git show m" <<-\EOF
master Z
mybranch Z
mytag Z
@@ -263,7 +259,7 @@ test_expect_success 'show completes all refs' '
'
test_expect_success '<ref>: completes paths' '
- test_completion_long "git show mytag:f" <<-\EOF
+ test_completion "git show mytag:f" <<-\EOF
file1 Z
file2 Z
EOF
@@ -273,7 +269,7 @@ test_expect_success 'complete tree filename with spaces' '
echo content >"name with spaces" &&
git add . &&
git commit -m spaces &&
- test_completion_long "git show HEAD:nam" <<-\EOF
+ test_completion "git show HEAD:nam" <<-\EOF
name with spaces Z
EOF
'
@@ -282,7 +278,7 @@ test_expect_failure 'complete tree filename with metacharacters' '
echo content >"name with \${meta}" &&
git add . &&
git commit -m meta &&
- test_completion_long "git show HEAD:nam" <<-\EOF
+ test_completion "git show HEAD:nam" <<-\EOF
name with ${meta} Z
name with spaces Z
EOF
--
1.8.0
^ permalink raw reply related
* [PATCH v3 3/6] completion: simplify tests using test_completion_long()
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>
No need to duplicate that functionality.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t9902-completion.sh | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f4c7342..6a250ad 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -172,7 +172,7 @@ test_expect_success 'basic' '
'
test_expect_success 'double dash "git" itself' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_completion_long "git --" <<-\EOF
--paginate Z
--no-pager Z
--git-dir=
@@ -187,11 +187,10 @@ test_expect_success 'double dash "git" itself' '
--no-replace-objects Z
--help Z
EOF
- test_completion "git --"
'
test_expect_success 'double dash "git checkout"' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_completion_long "git checkout --" <<-\EOF
--quiet Z
--ours Z
--theirs Z
@@ -202,17 +201,15 @@ test_expect_success 'double dash "git checkout"' '
--orphan Z
--patch Z
EOF
- test_completion "git checkout --"
'
test_expect_success 'general options' '
test_completion "git --ver" "--version " &&
test_completion "git --hel" "--help " &&
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_completion_long "git --exe" <<-\EOF &&
--exec-path Z
--exec-path=
EOF
- test_completion "git --exe" &&
test_completion "git --htm" "--html-path " &&
test_completion "git --pag" "--paginate " &&
test_completion "git --no-p" "--no-pager " &&
--
1.8.0
^ permalink raw reply related
* [PATCH v3 2/6] completion: standardize final space marker in tests
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>
The rest of the code uses ' Z$'. Lets use that for
test_completion_long() as well.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t9902-completion.sh | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2276a37..f4c7342 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -66,11 +66,10 @@ test_completion ()
}
# Like test_completion, but reads expectation from stdin,
-# which is convenient when it is multiline. We also process "_" into
-# spaces to make test vectors more readable.
+# which is convenient when it is multiline.
test_completion_long ()
{
- tr _ " " >expected &&
+ sed -e 's/Z$//' > expected &&
test_completion "$1"
}
@@ -252,24 +251,24 @@ test_expect_success 'setup for ref completion' '
test_expect_success 'checkout completes ref names' '
test_completion_long "git checkout m" <<-\EOF
- master_
- mybranch_
- mytag_
+ master Z
+ mybranch Z
+ mytag Z
EOF
'
test_expect_success 'show completes all refs' '
test_completion_long "git show m" <<-\EOF
- master_
- mybranch_
- mytag_
+ master Z
+ mybranch Z
+ mytag Z
EOF
'
test_expect_success '<ref>: completes paths' '
test_completion_long "git show mytag:f" <<-\EOF
- file1_
- file2_
+ file1 Z
+ file2 Z
EOF
'
@@ -278,7 +277,7 @@ test_expect_success 'complete tree filename with spaces' '
git add . &&
git commit -m spaces &&
test_completion_long "git show HEAD:nam" <<-\EOF
- name with spaces_
+ name with spaces Z
EOF
'
@@ -287,8 +286,8 @@ test_expect_failure 'complete tree filename with metacharacters' '
git add . &&
git commit -m meta &&
test_completion_long "git show HEAD:nam" <<-\EOF
- name with ${meta}_
- name with spaces_
+ name with ${meta} Z
+ name with spaces Z
EOF
'
--
1.8.0
^ permalink raw reply related
* [PATCH v3 1/6] completion: add comment for test_completion()
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>
So that it's easier to understand what it does.
Also, make sure we pass only the first argument for completion.
Shouldn't cause any functional changes because run_completion only
checks $1.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t9902-completion.sh | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 8fa025f..2276a37 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -54,10 +54,14 @@ run_completion ()
__git_wrap__git_main && print_comp
}
+# Test high-level completion
+# Arguments are:
+# 1: current command line
+# 2: expected completion
test_completion ()
{
test $# -gt 1 && echo "$2" > expected
- run_completion "$@" &&
+ run_completion "$1" &&
test_cmp expected out
}
--
1.8.0
^ permalink raw reply related
* [PATCH v3 0/6] completion: test consolidations
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
These started from a discussion with SZEDER, but then I realized there were
many improvements possible.
Changes since v2:
* Updated comments and commit messages
Changes since v1:
* A lot more cleanups
Felipe Contreras (6):
completion: add comment for test_completion()
completion: standardize final space marker in tests
completion: simplify tests using test_completion_long()
completion: consolidate test_completion*() tests
completion: refactor __gitcomp related tests
completion: simplify __gitcomp() test helper
t/t9902-completion.sh | 133 +++++++++++++++++++-------------------------------
1 file changed, 51 insertions(+), 82 deletions(-)
--
1.8.0
^ permalink raw reply
* Re: Local clones aka forks disk size optimization
From: Sitaram Chamarty @ 2012-11-18 10:42 UTC (permalink / raw)
To: Enrico Weigelt; +Cc: Michael J Gruber, Andrew Ardill, Javier Domingo, git
In-Reply-To: <ccb8680a-97ad-4cf8-95d0-5e21f60494f4@zcs>
On Fri, Nov 16, 2012 at 11:34 PM, Enrico Weigelt <enrico.weigelt@vnc.biz> wrote:
>
>> Provide one "main" clone which is bare, pulls automatically, and is
>> there to stay (no pruning), so that all others can use that as a
>> reliable alternates source.
>
> The problem here, IMHO, is the assumption, that the main repo will
> never be cleaned up. But what to do if you dont wanna let it grow
> forever ?
That's not the only problem. I believe you only get the savings when
the main repo gets the commits first. Which is probably ok most of
the time but it's worth mentioning.
>
> hmm, distributed GC is a tricky problem.
Except for one little issue (see other thread, subject line "cloning a
namespace downloads all the objects"), namespaces appear to do
everything we want in terms of the typical use cases for alternates,
and/or 'git clone -l', at least on the server side.
^ permalink raw reply
* Re: [PATCH 1/7] completion: make the 'basic' test more tester-friendly
From: Felipe Contreras @ 2012-11-18 9:39 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-2-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> 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.
>
> 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' '
It's not very useful to see a single failure without context. Maybe this:
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -191,18 +191,20 @@ test_expect_success '__gitcomp_nl - doesnt fail
because of invalid variable name
test_expect_success 'basic' '
run_completion "git " &&
+ (
# built-in
- echo "add " >expected &&
- sed -n "/^add \$/p" out >out2 &&
- test_cmp expected out2 &&
+ sed -n "/^add $/p" out &&
# script
- echo "filter-branch " >expected &&
- sed -n "/^filter-branch \$/p" out >out2 &&
- test_cmp expected out2 &&
+ sed -n "/^filter-branch $/p" out &&
# plumbing
- >expected &&
- sed -n "/^ls-files \$/p" out >out2 &&
- test_cmp expected out2 &&
+ sed -n "/^ls-files $/p" out
+ ) > actual &&
+
+ sed -e 's/Z$//' > expected <<-EOF &&
+ add Z
+ filter-branch Z
+ EOF
+ test_cmp expected actual &&
run_completion "git f" &&
>expected &&
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 1/7] completion: make the 'basic' test more tester-friendly
From: Felipe Contreras @ 2012-11-18 9:38 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: SZEDER Gábor, git, Jeff King, Junio C Hamano
In-Reply-To: <20121117230022.GA3815@elie.Belkin>
On Sun, Nov 18, 2012 at 12:00 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 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.
It does.
--
Felipe Contreras
^ permalink raw reply
* [PATCH 4/4] tree_entry_interesting: do basedir compare on wildcard patterns when possible
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>
Currently we treat "*.c" and "path/to/*.c" the same way. Which means
we check all possible paths in repo against "path/to/*.c". One could
see that "path/elsewhere/foo.c" obviously cannot match "path/to/*.c"
and we only need to check all paths _inside_ "path/to/" against that
pattern.
This patch checks the leading fixed part of a pathspec against base
directory and exit early if possible. We could even optimize further
in "path/to/something*.c" case (i.e. check the fixed part against
name_entry as well) but that's more complicated and probably does not
gain us much.
-O2 build on linux-2.6, without and with this patch respectively:
$ time git rev-list --quiet HEAD -- 'drivers/*.c'
real 1m9.484s
user 1m9.128s
sys 0m0.181s
$ time ~/w/git/git rev-list --quiet HEAD -- 'drivers/*.c'
real 0m15.710s
user 0m15.564s
sys 0m0.107s
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
tree-walk.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/tree-walk.c b/tree-walk.c
index 42fe610..dcc1015 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -573,6 +573,52 @@ static int match_dir_prefix(const char *base,
}
/*
+ * Perform matching on the leading non-wildcard part of
+ * pathspec. item->nowildcard_len must be greater than zero. Return
+ * non-zero if base is matched.
+ */
+static int match_wildcard_base(const struct pathspec_item *item,
+ const char *base, int baselen,
+ int *matched)
+{
+ const char *match = item->match;
+ /* the wildcard part is not considered in this function */
+ int matchlen = item->nowildcard_len;
+
+ if (baselen) {
+ int dirlen;
+ /*
+ * Return early if base is longer than the
+ * non-wildcard part but it does not match.
+ */
+ if (baselen >= matchlen) {
+ *matched = matchlen;
+ return !strncmp(base, match, matchlen);
+ }
+
+ dirlen = matchlen;
+ while (dirlen && match[dirlen - 1] != '/')
+ dirlen--;
+
+ /* Return early if base is shorter than the
+ non-wildcard part but it does not match. Note that
+ base ends with '/' so we are sure it really matches
+ directory */
+ if (strncmp(base, match, baselen))
+ return 0;
+ *matched = baselen;
+ } else
+ *matched = 0;
+ /*
+ * we could have checked entry against the non-wildcard part
+ * that is not in base and does similar never_interesting
+ * optimization as in match_entry. For now just be happy with
+ * base comparison.
+ */
+ return entry_interesting;
+}
+
+/*
* Is a tree entry interesting given the pathspec we have?
*
* Pre-condition: either baselen == base_offset (i.e. empty path)
@@ -602,7 +648,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
const struct pathspec_item *item = ps->items+i;
const char *match = item->match;
const char *base_str = base->buf + base_offset;
- int matchlen = item->len;
+ int matchlen = item->len, matched = 0;
if (baselen >= matchlen) {
/* If it doesn't match, move along... */
@@ -647,9 +693,24 @@ match_wildcards:
if (item->nowildcard_len == item->len)
continue;
+ if (item->nowildcard_len &&
+ !match_wildcard_base(item, base_str, baselen, &matched))
+ return entry_not_interesting;
+
/*
* Concatenate base and entry->path into one and do
* fnmatch() on it.
+ *
+ * While we could avoid concatenation in certain cases
+ * [1], which saves a memcpy and potentially a
+ * realloc, it turns out not worth it. Measurement on
+ * linux-2.6 does not show any clear improvements,
+ * partly because of the nowildcard_len optimization
+ * in git_fnmatch(). Avoid micro-optimizations here.
+ *
+ * [1] if match_wildcard_base() says the base
+ * directory is already matched, we only need to match
+ * the rest, which is shorter so _in theory_ faster.
*/
strbuf_add(base, entry->path, pathlen);
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 2/4] pathspec: do exact comparison on the leading non-wildcard part
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 18 +++++++++++++++++-
dir.h | 8 ++++++++
tree-walk.c | 6 ++++--
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/dir.c b/dir.c
index c391d46..e4e6ca1 100644
--- a/dir.c
+++ b/dir.c
@@ -34,6 +34,21 @@ int fnmatch_icase(const char *pattern, const char *string, int flags)
return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));
}
+inline int git_fnmatch(const char *pattern, const char *string,
+ int flags, int prefix)
+{
+ int fnm_flags = 0;
+ if (flags & GF_PATHNAME)
+ fnm_flags |= FNM_PATHNAME;
+ if (prefix > 0) {
+ if (strncmp(pattern, string, prefix))
+ return FNM_NOMATCH;
+ pattern += prefix;
+ string += prefix;
+ }
+ return fnmatch(pattern, string, fnm_flags);
+}
+
static size_t common_prefix_len(const char **pathspec)
{
const char *n, *first;
@@ -230,7 +245,8 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
return MATCHED_RECURSIVELY;
}
- if (item->nowildcard_len < item->len && !fnmatch(match, name, 0))
+ if (item->nowildcard_len < item->len &&
+ !git_fnmatch(match, name, 0, item->nowildcard_len - prefix))
return MATCHED_FNMATCH;
return 0;
diff --git a/dir.h b/dir.h
index f5c89e3..4cd5074 100644
--- a/dir.h
+++ b/dir.h
@@ -139,4 +139,12 @@ extern int strcmp_icase(const char *a, const char *b);
extern int strncmp_icase(const char *a, const char *b, size_t count);
extern int fnmatch_icase(const char *pattern, const char *string, int flags);
+/*
+ * The prefix part of pattern must not contains wildcards.
+ */
+#define GF_PATHNAME 1
+
+extern int git_fnmatch(const char *pattern, const char *string,
+ int flags, int prefix);
+
#endif
diff --git a/tree-walk.c b/tree-walk.c
index af871c5..2fcf3c0 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -627,7 +627,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
return entry_interesting;
if (item->nowildcard_len < item->len) {
- if (!fnmatch(match + baselen, entry->path, 0))
+ if (!git_fnmatch(match + baselen, entry->path,
+ 0, item->nowildcard_len - baselen))
return entry_interesting;
/*
@@ -652,7 +653,8 @@ match_wildcards:
strbuf_add(base, entry->path, pathlen);
- if (!fnmatch(match, base->buf + base_offset, 0)) {
+ if (!git_fnmatch(match, base->buf + base_offset,
+ 0, item->nowildcard_len)) {
strbuf_setlen(base, base_offset + baselen);
return entry_interesting;
}
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 3/4] pathspec: apply "*.c" optimization from exclude
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>
-O2 build on linux-2.6, without the patch:
$ time git rev-list --quiet HEAD -- '*.c'
real 0m40.770s
user 0m40.290s
sys 0m0.256s
With the patch
$ time ~/w/git/git rev-list --quiet HEAD -- '*.c'
real 0m34.288s
user 0m33.997s
sys 0m0.205s
The above command is not supposed to be widely popular. It's chosen
because it exercises pathspec matching a lot. The point is it cuts
down matching time for popular patterns like *.c, which could be used
as pathspec in other places.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
cache.h | 3 +++
dir.c | 17 +++++++++++++++--
dir.h | 1 +
tree-walk.c | 6 ++++--
4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index bf031f1..d18f584 100644
--- a/cache.h
+++ b/cache.h
@@ -473,6 +473,8 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
+#define PSF_ONESTAR 1
+
struct pathspec {
const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
int nr;
@@ -483,6 +485,7 @@ struct pathspec {
const char *match;
int len;
int nowildcard_len;
+ int flags;
} *items;
};
diff --git a/dir.c b/dir.c
index e4e6ca1..d00f240 100644
--- a/dir.c
+++ b/dir.c
@@ -46,6 +46,12 @@ inline int git_fnmatch(const char *pattern, const char *string,
pattern += prefix;
string += prefix;
}
+ if (flags & GF_ONESTAR) {
+ int pattern_len = strlen(++pattern);
+ int string_len = strlen(string);
+ return strcmp(pattern,
+ string + string_len - pattern_len);
+ }
return fnmatch(pattern, string, fnm_flags);
}
@@ -246,7 +252,9 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
}
if (item->nowildcard_len < item->len &&
- !git_fnmatch(match, name, 0, item->nowildcard_len - prefix))
+ !git_fnmatch(match, name,
+ item->flags & PSF_ONESTAR ? GF_ONESTAR : 0,
+ item->nowildcard_len - prefix))
return MATCHED_FNMATCH;
return 0;
@@ -1446,8 +1454,13 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
item->match = path;
item->len = strlen(path);
item->nowildcard_len = simple_length(path);
- if (item->nowildcard_len < item->len)
+ item->flags = 0;
+ if (item->nowildcard_len < item->len) {
pathspec->has_wildcard = 1;
+ if (path[item->nowildcard_len] == '*' &&
+ no_wildcard(path + item->nowildcard_len + 1))
+ item->flags |= PSF_ONESTAR;
+ }
}
qsort(pathspec->items, pathspec->nr,
diff --git a/dir.h b/dir.h
index 4cd5074..590532b 100644
--- a/dir.h
+++ b/dir.h
@@ -143,6 +143,7 @@ extern int fnmatch_icase(const char *pattern, const char *string, int flags);
* The prefix part of pattern must not contains wildcards.
*/
#define GF_PATHNAME 1
+#define GF_ONESTAR 2
extern int git_fnmatch(const char *pattern, const char *string,
int flags, int prefix);
diff --git a/tree-walk.c b/tree-walk.c
index 2fcf3c0..42fe610 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -628,7 +628,8 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
if (item->nowildcard_len < item->len) {
if (!git_fnmatch(match + baselen, entry->path,
- 0, item->nowildcard_len - baselen))
+ item->flags & PSF_ONESTAR ? GF_ONESTAR : 0,
+ item->nowildcard_len - baselen))
return entry_interesting;
/*
@@ -654,7 +655,8 @@ match_wildcards:
strbuf_add(base, entry->path, pathlen);
if (!git_fnmatch(match, base->buf + base_offset,
- 0, item->nowildcard_len)) {
+ item->flags & PSF_ONESTAR ? GF_ONESTAR : 0,
+ item->nowildcard_len)) {
strbuf_setlen(base, base_offset + baselen);
return entry_interesting;
}
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 1/4] pathspec: save the non-wildcard length part
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>
We marks pathspec with wildcards with the field use_wildcard. We could
do better by saving the length of the non-wildcard part, which can be
for optimizations such as f9f6e2c (exclude: do strcmp as much as
possible before fnmatch - 2012-06-07)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/ls-files.c | 2 +-
builtin/ls-tree.c | 2 +-
cache.h | 2 +-
dir.c | 6 +++---
tree-walk.c | 4 ++--
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b5434af..4a9ee69 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -337,7 +337,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
matchbuf[0] = prefix;
matchbuf[1] = NULL;
init_pathspec(&pathspec, matchbuf);
- pathspec.items[0].use_wildcard = 0;
+ pathspec.items[0].nowildcard_len = pathspec.items[0].len;
} else
init_pathspec(&pathspec, NULL);
if (read_tree(tree, 1, &pathspec))
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 235c17c..fb76e38 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -168,7 +168,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
init_pathspec(&pathspec, get_pathspec(prefix, argv + 1));
for (i = 0; i < pathspec.nr; i++)
- pathspec.items[i].use_wildcard = 0;
+ pathspec.items[i].nowildcard_len = pathspec.items[i].len;
pathspec.has_wildcard = 0;
tree = parse_tree_indirect(sha1);
if (!tree)
diff --git a/cache.h b/cache.h
index dbd8018..bf031f1 100644
--- a/cache.h
+++ b/cache.h
@@ -482,7 +482,7 @@ struct pathspec {
struct pathspec_item {
const char *match;
int len;
- unsigned int use_wildcard:1;
+ int nowildcard_len;
} *items;
};
diff --git a/dir.c b/dir.c
index 5a83aa7..c391d46 100644
--- a/dir.c
+++ b/dir.c
@@ -230,7 +230,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
return MATCHED_RECURSIVELY;
}
- if (item->use_wildcard && !fnmatch(match, name, 0))
+ if (item->nowildcard_len < item->len && !fnmatch(match, name, 0))
return MATCHED_FNMATCH;
return 0;
@@ -1429,8 +1429,8 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
item->match = path;
item->len = strlen(path);
- item->use_wildcard = !no_wildcard(path);
- if (item->use_wildcard)
+ item->nowildcard_len = simple_length(path);
+ if (item->nowildcard_len < item->len)
pathspec->has_wildcard = 1;
}
diff --git a/tree-walk.c b/tree-walk.c
index 3f54c02..af871c5 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -626,7 +626,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
&never_interesting))
return entry_interesting;
- if (item->use_wildcard) {
+ if (item->nowildcard_len < item->len) {
if (!fnmatch(match + baselen, entry->path, 0))
return entry_interesting;
@@ -642,7 +642,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
}
match_wildcards:
- if (!item->use_wildcard)
+ if (item->nowildcard_len == item->len)
continue;
/*
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* [PATCH 0/4] Some pathspec wildcard optimization
From: Nguyễn Thái Ngọc Duy @ 2012-11-18 9:13 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This ports the "*.c" optimization from .gitignore to pathspec code.
Nguyễn Thái Ngọc Duy (4):
pathspec: save the non-wildcard length part
pathspec: do exact comparison on the leading non-wildcard part
pathspec: apply "*.c" optimization from exclude
tree_entry_interesting: do basedir compare on wildcard patterns when
possible
builtin/ls-files.c | 2 +-
builtin/ls-tree.c | 2 +-
cache.h | 5 +++-
dir.c | 35 ++++++++++++++++++++++---
dir.h | 9 +++++++
tree-walk.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++----
6 files changed, 117 insertions(+), 11 deletions(-)
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply
* Re: [PATCH 3/7] completion: add tests for the __gitcomp_nl() completion helper function
From: Felipe Contreras @ 2012-11-18 8:53 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-4-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> 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(+)
Too much code that can be simplified:
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -85,6 +85,22 @@ test_gitcomp ()
test_cmp expected out
}
+# Test __gitcomp_nl
+# Arguments are:
+# 1: typed text so far (cur)
+# -: the rest are passed to __gitcomp_nl
+test_gitcomp_nl ()
+{
+ local -a COMPREPLY &&
+ sed -e 's/Z$//' > expected &&
+ cur="$1" &&
+ shift &&
+ __gitcomp_nl "$@" &&
+ print_comp &&
+ cp expected out /tmp
+ test_cmp expected out
+}
+
invalid_variable_name="${foo.bar}"
test_expect_success '__gitcomp - trailing space - options' '
@@ -134,88 +150,39 @@ test_expect_success '__gitcomp - doesnt fail
because of invalid variable name' '
__gitcomp "$invalid_variable_name"
'
+read -r -d "" refs <<-\EOF
+maint
+master
+next
+pu
+EOF
+
test_expect_success '__gitcomp_nl - trailing space' '
- sed -e "s/Z$//" >expected <<-\EOF &&
+ test_gitcomp_nl "m" "$refs" <<-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 &&
+ test_gitcomp_nl "--fixup=m" "$refs" "--fixup=" "m" <<-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 &&
+ test_gitcomp_nl "branch.ma" "$refs" "branch." "ma" "." <<-\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 &&
+ test_gitcomp_nl "ma" "$refs" "" "ma" "" <<-\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 '__gitcomp_nl - doesnt fail because of invalid
variable name' '
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/7] completion: fix args of run_completion() test helper
From: Felipe Contreras @ 2012-11-18 8:48 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-3-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> test_expect_success 'basic' '
> - run_completion "git \"\"" &&
> + run_completion git "" &&
I don't like this approach. I prefer
run_completion "git "
So that it's similar to how test_completion is called:
run_completion "git f"
test_completion "git f"
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 4/7] completion: add tests for invalid variable name among completion words
From: Felipe Contreras @ 2012-11-18 8:34 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: SZEDER Gábor, git, Jeff King, Junio C Hamano
In-Reply-To: <20121117234059.GD3815@elie.Belkin>
On Sun, Nov 18, 2012 at 12:40 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 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.
Shouldn't this go to the commit message?
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 4/7] completion: add tests for invalid variable name among completion words
From: Felipe Contreras @ 2012-11-18 8:34 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-5-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> @@ -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"
> + )
> +'
Why in a subshell?
--
Felipe Contreras
^ permalink raw reply
* Re: t5801-remote-helpers.sh fails
From: Felipe Contreras @ 2012-11-18 8:23 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git Mailing List
In-Reply-To: <50A87718.4030806@web.de>
Hi,
On Sun, Nov 18, 2012 at 6:50 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> 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.
It's already sent:
http://article.gmane.org/gmane.comp.version-control.git/209364
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] Rename V15_MINGW_HEADERS into CYGWIN_OLD_WINSOCK_HEADERS
From: Junio C Hamano @ 2012-11-18 7:46 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Torsten Bögershausen, git
In-Reply-To: <50A7A161.9020805@gmail.com>
Mark Levedahl <mlevedahl@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.
^ permalink raw reply
* Re: t5801-remote-helpers.sh fails
From: Torsten Bögershausen @ 2012-11-18 5:50 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Torsten Bögershausen, Git Mailing List
In-Reply-To: <CAMP44s2yenQKSgdUXfZP+yDJJ+bdveyms=SQ+3ptPvpT6D0hsg@mail.gmail.com>
On 10.11.12 23:05, Felipe Contreras wrote:
> On Sat, Nov 10, 2012 at 8:20 PM, Torsten Bögershausen <tboegi@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 <tboegi@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 &&
^ permalink raw reply related
* Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: Felipe Contreras @ 2012-11-18 0:58 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" ))
> }
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" ))
== 10000 ==
awk 1:
real 0m0.067s
user 0m0.066s
sys 0m0.001s
awk 2:
real 0m0.057s
user 0m0.055s
sys 0m0.002s
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: Felipe Contreras @ 2012-11-18 0:55 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <CAMP44s1zaxey7TQxGaLtaMUwPTVTmRBn1w6=Zqefy7FJzEepBw@mail.gmail.com>
On Sat, Nov 17, 2012 at 8:28 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Nov 17, 2012 at 8:08 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Sat, Nov 17, 2012 at 3:14 PM, SZEDER Gábor <szeder@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 <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
>>>
>>> It does:
>>>
>>> My version:
>>>
>>> $ refs="$(for i in {0..9999} ; do echo branch$i ; done)"
>>> $ time __gitcomp_nl "$refs"
>>>
>>> real 0m0.109s
>>> user 0m0.096s
>>> sys 0m0.012s
>>>
>>> Yours:
>>>
>>> $ time __gitcomp_nl "$refs"
>>>
>>> real 0m0.321s
>>> user 0m0.312s
>>> sys 0m0.008s
>>
>> Yeah, for 10000 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
^ permalink raw reply
* Re: [RFC/PATCH 4/5] completion: get rid of compgen
From: Felipe Contreras @ 2012-11-18 0:53 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: <CAMP44s0YnoGwJEgUbXZ831_OrgO=dDf5_QHxT5JYnGUHYPuiTw@mail.gmail.com>
On Sat, Nov 17, 2012 at 8:33 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Nov 17, 2012 at 3:12 PM, SZEDER Gábor <szeder@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 <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.
>>
>> 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..9999} ; do echo branch$i ; done)"
>> $ time __gitcomp_nl "$refs"
>>
>> real 0m0.242s
>> user 0m0.220s
>> sys 0m0.028s
>>
>> After:
>>
>> $ time __gitcomp_nl "$refs"
>>
>> real 0m0.109s
>> user 0m0.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
>
> == 10000 ==
> one:
> real 0m0.148s
> user 0m0.134s
> sys 0m0.025s
> two:
> real 0m0.055s
> user 0m0.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"))
== 10000 ==
awk 1:
real 0m0.064s
user 0m0.062s
sys 0m0.003s
printf:
real 0m0.069s
user 0m0.064s
sys 0m0.020s
--
Felipe Contreras
^ permalink raw reply
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