* Re: rebase: strange failures to apply patc 3-way
From: Andrew Wong @ 2013-03-12 1:03 UTC (permalink / raw)
To: Jeff King; +Cc: Max Horn, git@vger.kernel.org
In-Reply-To: <20130312010114.GA21026@sigill.intra.peff.net>
On 03/11/13 21:01, Jeff King wrote:
> From "git help config":
>
> core.trustctime
> If false, the ctime differences between the index and the working
> tree are ignored; useful when the inode change time is regularly
> modified by something outside Git (file system crawlers and some
> backup systems). See git-update- index(1). True by default.
In an earlier email, Max did mention setting the config does workaround
the issue. But it's still strange that it only happens to a few specific
files in this particular repo. The issue never popped up in other repos
that he has, which I assume has all been excluded from Time Machine...
^ permalink raw reply
* Re: rebase: strange failures to apply patc 3-way
From: Jeff King @ 2013-03-12 1:10 UTC (permalink / raw)
To: Andrew Wong; +Cc: Max Horn, git@vger.kernel.org
In-Reply-To: <513E7EEE.3060107@gmail.com>
On Mon, Mar 11, 2013 at 09:03:42PM -0400, Andrew Wong wrote:
> On 03/11/13 21:01, Jeff King wrote:
> > From "git help config":
> >
> > core.trustctime
> > If false, the ctime differences between the index and the working
> > tree are ignored; useful when the inode change time is regularly
> > modified by something outside Git (file system crawlers and some
> > backup systems). See git-update- index(1). True by default.
>
> In an earlier email, Max did mention setting the config does workaround
> the issue. But it's still strange that it only happens to a few specific
> files in this particular repo. The issue never popped up in other repos
> that he has, which I assume has all been excluded from Time Machine...
Ah, sorry, I missed the earlier reference to it. trustctime is the only
sensible thing to do from the git side. I think figuring out the rest is
deep Apple magic that I'm not qualified to comment on.
-Peff
^ permalink raw reply
* Re: ZSH segmentation fault while completing "git mv dir/"
From: Jeff Epler @ 2013-03-12 1:37 UTC (permalink / raw)
To: Manlio Perillo; +Cc: Matthieu Moy, git, felipe.contreras
In-Reply-To: <513E3190.20100@gmail.com>
If it's dependent on eval "`dircolors`", it suggests it might be
dependent on the size of the environment. Maybe try with FOO=`perl -e
'print "x"x1000'` for various values of 1000...
Jeff
^ permalink raw reply
* git files from another branch in a separate repository
From: Scott Haynes @ 2013-03-12 2:22 UTC (permalink / raw)
To: git
I'm currently working on a git project( vwf - virtualworldframework ), and we need to get a couple of files from another repository( threes ). We've forked threejs into our repository, but I need to be able to grab only those few files that I need. I thought I wanted to do a sub-tree merge, but after reading about the -tree options, I don't think that is what I need to do. Do I want to right a hook script to pull in those files?
TIA,
Scott
^ permalink raw reply
* Re: ZSH segmentation fault while completing "git mv dir/"
From: Matthieu Moy @ 2013-03-12 7:53 UTC (permalink / raw)
To: Jeff Epler; +Cc: Manlio Perillo, git, felipe.contreras
In-Reply-To: <20130312013736.GB7105@unpythonic.net>
Jeff Epler <jepler@unpythonic.net> writes:
> If it's dependent on eval "`dircolors`", it suggests it might be
> dependent on the size of the environment. Maybe try with FOO=`perl -e
> 'print "x"x1000'` for various values of 1000...
It's not this, but I could reduce the problem to a slightly simpler
.zshrc:
-----------------------------------------------------------------
fpath=(${HOME}/usr/etc/zsh ${fpath})
autoload -Uz compinit
compinit
zstyle ':completion:*' list-colors 'rs=0' 'di=01;34' 'ln=01;36' \
'mh=00' 'pi=40;33' 'so=01;35' 'do=01;35' 'bd=40;33;01' 'cd=40;33;01' \
'or=40;31;01' 'su=37;41' 'sg=30;43' 'ca=30;41' 'tw=30;42' 'ow=34;42' \
'st=37;44' 'ex=01;32' '*.tar=01;31' '*.tgz=01;31' '*.arj=01;31' \
'*.taz=01;31' '*.lzh=01;31' '*.lzma=01;31' '*.tlz=01;31' '*.txz=01;31' \
'*.zip=01;31' '*.z=01;31' '*.Z=01;31' '*.dz=01;31' '*.gz=01;31' \
'*.lz=01;31' '*.xz=01;31' '*.bz2=01;31' '*.bz=01;31' '*.tbz=01;31' \
'*.tbz2=01;31' '*.tz=01;31' '*.deb=01;31' '*.rpm=01;31' '*.jar=01;31' \
'*.rar=01;31' '*.ace=01;31' '*.pgm=01;35' '*.ppm=01;35' \
'*.tga=01;35' '*.xbm=01;35' '*.zoo=01;31' '*.cpio=01;31' '*.7z=01;31' \
'*.rz=01;31' '*.jpg=01;35' '*.jpeg=01;35' '*.gif=01;35' \
'*.bmp=01;35' '*.pbm=01;35'
-----------------------------------------------------------------
The problem disapears if I remove one of the arguments after list-colors
(I didn't try with each of them, but picking 5 of them randomly and
removing it worked), so it seems to be a matter of number of arguments.
On git-completion.bash's side, it's really strange. If I comment out the
"case" statement in _git_mv like this, the problem disapears:
_git_mv ()
{
# case "$cur" in
# --*)
# __gitcomp "--dry-run"
# return
# ;;
# esac
if [ $(__git_count_arguments "mv") -gt 0 ]; then
# We need to show both cached and untracked files (including
# empty directories) since this may not be the last argument.
__git_complete_index_file "--cached --others --directory"
else
__git_complete_index_file "--cached"
fi
}
If I add a simple 'echo "$cur" >&2' instead of the case, the problem
reappears. Somehow, the fact that I'm accessing $cur seems to create the
segfault. Actually, the minimalistic _git_mv reproducing the problem is:
_git_mv ()
{
echo "$cur"
[ $(__git_count_arguments "mv") = 1 ]
__git_complete_index_file "--cached --others --directory"
}
gdb doesn't say much interesting since my ZSH isn't compile with debug:
(gdb) r
Starting program: /usr/bin/zsh
anie$ cd /tmp/git
anie$ git mv subdir/
Program received signal SIGSEGV, Segmentation fault.
__strlen_sse2 () at ../sysdeps/i386/i686/multiarch/strlen.S:87
87 ../sysdeps/i386/i686/multiarch/strlen.S: No such file or directory.
in ../sysdeps/i386/i686/multiarch/strlen.S
Current language: auto
The current source language is "auto; currently asm".
(gdb) where
#0 __strlen_sse2 () at ../sysdeps/i386/i686/multiarch/strlen.S:87
#1 0x080ae31d in ztrdup ()
#2 0xb7b9d287 in permmatches () from /usr/lib/zsh/4.3.10/zsh/complete.so
#3 0xb7b9a22e in ?? () from /usr/lib/zsh/4.3.10/zsh/complete.so
#4 0x08098f33 in getstrvalue ()
#5 0x08099e9d in ?? ()
#6 0x0809acd7 in getindex ()
#7 0x0809b3d9 in fetchvalue ()
#8 0x080b0f84 in ?? ()
#9 0x080b49a7 in prefork ()
#10 0x08066cb9 in ?? ()
#11 0x08067151 in ?? ()
#12 0x0806cfc0 in execlist ()
#13 0x0808984c in execif ()
#14 0x080670c6 in ?? ()
#15 0x0806cfc0 in execlist ()
#16 0x0806d69a in execode ()
#17 0x0806d785 in runshfunc ()
#18 0xb7b99a7a in ?? () from /usr/lib/zsh/4.3.10/zsh/complete.so
#19 0x0806d724 in runshfunc ()
#20 0x0806db92 in doshfunc ()
#21 0xb7ba3363 in ?? () from /usr/lib/zsh/4.3.10/zsh/complete.so
#22 0xb7ba44dc in do_completion () from /usr/lib/zsh/4.3.10/zsh/complete.so
#23 0xb7bdaf47 in ?? () from /usr/lib/zsh/4.3.10/zsh/zle.so
#24 0xb7bd5b40 in completecall () from /usr/lib/zsh/4.3.10/zsh/zle.so
#25 0xb7bc6f6e in execzlefunc () from /usr/lib/zsh/4.3.10/zsh/zle.so
#26 0xb7bc72a2 in zlecore () from /usr/lib/zsh/4.3.10/zsh/zle.so
#27 0xb7bc78d5 in zleread () from /usr/lib/zsh/4.3.10/zsh/zle.so
#28 0xb7bc96ff in ?? () from /usr/lib/zsh/4.3.10/zsh/zle.so
#29 0x0807d462 in zleentry ()
#30 0x08080a31 in ingetc ()
#31 0x0807b03c in ?? ()
#32 0x08087fb6 in zshlex ()
#33 0x080a3b4a in parse_event ()
#34 0x0807f2f1 in loop ()
#35 0x080800c6 in zsh_main ()
#36 0x08054cbb in main ()
I can't reproduce the problem with the exact same config and a freshly
compiled ZSH (so it's no use reporting the issue to the ZSH developers).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: rebase: strange failures to apply patc 3-way
From: Heiko Voigt @ 2013-03-12 8:26 UTC (permalink / raw)
To: Jeff King; +Cc: Andrew Wong, Max Horn, git@vger.kernel.org
In-Reply-To: <20130312011004.GA21242@sigill.intra.peff.net>
On Mon, Mar 11, 2013 at 09:10:04PM -0400, Jeff King wrote:
> On Mon, Mar 11, 2013 at 09:03:42PM -0400, Andrew Wong wrote:
>
> > On 03/11/13 21:01, Jeff King wrote:
> > > From "git help config":
> > >
> > > core.trustctime
> > > If false, the ctime differences between the index and the working
> > > tree are ignored; useful when the inode change time is regularly
> > > modified by something outside Git (file system crawlers and some
> > > backup systems). See git-update- index(1). True by default.
> >
> > In an earlier email, Max did mention setting the config does workaround
> > the issue. But it's still strange that it only happens to a few specific
> > files in this particular repo. The issue never popped up in other repos
> > that he has, which I assume has all been excluded from Time Machine...
>
> Ah, sorry, I missed the earlier reference to it. trustctime is the only
> sensible thing to do from the git side. I think figuring out the rest is
> deep Apple magic that I'm not qualified to comment on.
>From what I read isn't the ctime the only thing that changes? I read
that you tried this option and the issues disappeared? Maybe the
resolution (or part of it) is to change the default of core.trustctime
to false on osx?
Cheers Heiko
^ permalink raw reply
* Re: Proposal: sharing .git/config
From: Jeff King @ 2013-03-12 8:53 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Duy Nguyen, Git List
In-Reply-To: <CALkWK0kZiYmbh-dSr-c2mzK2Mzhf3iZ_T3hFNuR57F0b5tptGw@mail.gmail.com>
On Tue, Mar 12, 2013 at 01:01:08AM +0530, Ramkumar Ramachandra wrote:
> > But it was pointed out that you could also just do:
> >
> > $ git config include.ref upstream-config
> > $ git show origin/config ;# make sure it looks reasonable
> > $ git show origin/config >.git/upstream-config
> >
> > and so forth. There are some ways that a pure ref can be more
> > convenient (e.g., if you are carrying local changes on top of the
> > upstream config and want to merge), but ultimately, you can replicate
> > any include.ref workflow with include.path by adding a "deploy" step
> > where you copy the file into $GIT_DIR.
>
> This seems to be unnecessarily complex and inelegant. Maybe this
> functionality is best managed as a separate git repository: `repo`
> from depot_tools uses a manifest repository containing all the project
> metadata. Maybe we can extend it/ write an more general version?
I don't think you can avoid the 3-step problem and retain the safety in
the general case. Forgetting implementation details for a minute, you
have either a 1-step system:
1. Fetch and start using config from the remote.
which is subject to fetching and executing malicious config, or:
1. Fetch config from remote.
2. Inspect it.
3. Integrate it into the current config.
We can automate the sequence to remove as much friction as possible, but
fundamentally step 2 requires some effort from the user. Moving the
config to a separate repo does not get rid of those steps. The user
either does not look at the config before using it, in which case we are
no better than the 1-step scenario, or they do, in which case they are
replicating the 3-step scenario.
The other alternative is to automate step 2. The simplest way would be
to have a whitelist of "ok to share" config, that would not include
things like diff.external that can run arbitrary code. I don't know
whether that would make the system too limited for what people want to
do. Do we have a concrete example of what config people would like to
share in this manner?
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
From: Ramkumar Ramachandra @ 2013-03-12 9:43 UTC (permalink / raw)
To: Heiko Voigt
Cc: Git List, Duy Nguyen, Junio C Hamano, Torsten Bögershausen,
Robert Zeh, Jeff King, Erik Faye-Lund, Karsten Blees,
Drew Northup
In-Reply-To: <20130311170542.GB4167@sandbox-ub.fritz.box>
Heiko Voigt wrote:
> While talking about platform independence. How about Windows? AFAIK
> there are no file based sockets. How about using shared memory, thats
> available, instead? It would greatly reduce the needed porting effort.
What about the git credential helper: it uses UNIX sockets, no? How
does git-credential-winstore [1] work?
[1]: https://github.com/anurse/git-credential-winstore
^ permalink raw reply
* Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
From: Erik Faye-Lund @ 2013-03-12 9:50 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Heiko Voigt, Git List, Duy Nguyen, Junio C Hamano,
Torsten Bögershausen, Robert Zeh, Jeff King, Karsten Blees,
Drew Northup
In-Reply-To: <CALkWK0m+CS25=Z91y90z49g4GK+b7oFugiXoH0h0J6P-yQXRCQ@mail.gmail.com>
On Tue, Mar 12, 2013 at 10:43 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Heiko Voigt wrote:
>> While talking about platform independence. How about Windows? AFAIK
>> there are no file based sockets. How about using shared memory, thats
>> available, instead? It would greatly reduce the needed porting effort.
>
> What about the git credential helper: it uses UNIX sockets, no? How
> does git-credential-winstore [1] work?
>
> [1]: https://github.com/anurse/git-credential-winstore
First, we have a proper credential helper for Windows in
contrib/credential/wincred these days. As the one who wrote that, we
communicate using stdin/stdout. The credential-helper doesn't maintain
state in itself, the Windows Credential Manager does. I suspect
git-credential-winstore works the same way.
As for Windows support, AFAIK there is no support for Unix domain
sockets in Windows. But there is support for named pipes, which is
almost the same thing. What we have support for in compat/mingw.[ch]
is a different matter, but we can extend that if needed.
^ permalink raw reply
* Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
From: Jeff King @ 2013-03-12 9:55 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Heiko Voigt, Git List, Duy Nguyen, Junio C Hamano,
Torsten Bögershausen, Robert Zeh, Erik Faye-Lund,
Karsten Blees, Drew Northup
In-Reply-To: <CALkWK0m+CS25=Z91y90z49g4GK+b7oFugiXoH0h0J6P-yQXRCQ@mail.gmail.com>
On Tue, Mar 12, 2013 at 03:13:39PM +0530, Ramkumar Ramachandra wrote:
> Heiko Voigt wrote:
> > While talking about platform independence. How about Windows? AFAIK
> > there are no file based sockets. How about using shared memory, thats
> > available, instead? It would greatly reduce the needed porting effort.
>
> What about the git credential helper: it uses UNIX sockets, no? How
> does git-credential-winstore [1] work?
No, the main credential protocol happens over pipes to a child process's
stdin/stdout. The credential-cache helper does use unix sockets (since
it needs to contact a long-running daemon that caches the credentials),
and AFAIK is not available under Windows (but that's OK, because
Windows-specific helpers that use secure storage are better anyway).
When I introduced credential-cache, I recall somebody mentioned that
there is some Windows-equivalent IPC that can be used to emulate unix
domain sockets. The calls aren't the same, but as long as your
requirements are basically "get messages to/from the daemon", you can
probably abstract away the details on a per-platform basis.
Unfortunately I can't seem to find the original message or any details
in the archive (and I know next to nothing about Windows IPC).
-Peff
^ permalink raw reply
* Re: Questions/investigations on git-subtree and tags
From: Jeremy Rosen @ 2013-03-12 10:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Campbell, git
In-Reply-To: <886271566.251606.1362763771332.JavaMail.root@openwide.fr>
[-- Attachment #1: Type: text/plain, Size: 416 bytes --]
Paul, I'm not quite sure where I should go from here...
should I send you a patch so you make it a V3 of your patch ? should I
send a patch superseeding yours ?
I have also found a similar problem in git-subtree pull, which needs
the same fix.
in the mean time, attached is the current version of my changes
Cordialement
Jérémy Rosen
fight key loggers : write some perl using vim
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-git-subtree-make-sure-the-SHA-saved-as-ancestor-is-a.patch --]
[-- Type: text/x-patch; name=0001-git-subtree-make-sure-the-SHA-saved-as-ancestor-is-a.patch, Size: 1343 bytes --]
From 12490724ae955719694d825dff4fa9e0d2709c1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Rosen?= <jeremy.rosen@openwide.fr>
Date: Tue, 12 Mar 2013 10:56:54 +0100
Subject: [PATCH 1/2] git-subtree: make sure the SHA saved as ancestor is a
commit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When adding or merging the first parameter might not be a commit, it can also be a tag SHA.
This needs to be fixed by using the underlying commit or the ancestor finding code will croak at split time
Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
---
contrib/subtree/git-subtree.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 8a23f58..8b9d114 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -531,7 +531,7 @@ cmd_add_repository()
cmd_add_commit()
{
- revs=$(git rev-parse $default --revs-only "$@") || exit $?
+ revs=$(git rev-parse $default --revs-only "$1^{commit}") || exit $?
set -- $revs
rev="$1"
@@ -655,7 +655,7 @@ cmd_split()
cmd_merge()
{
- revs=$(git rev-parse $default --revs-only "$@") || exit $?
+ revs=$(git rev-parse $default --revs-only "$1^{commit}") || exit $?
ensure_clean
set -- $revs
--
1.7.10.4
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-git-subtree-use-ls-remote-to-check-the-refspec-passe.patch --]
[-- Type: text/x-patch; name=0002-git-subtree-use-ls-remote-to-check-the-refspec-passe.patch, Size: 1628 bytes --]
From 05d1dd56217be59d69952a41d97e204cc7821948 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Rosen?= <jeremy.rosen@openwide.fr>
Date: Tue, 12 Mar 2013 10:57:56 +0100
Subject: [PATCH 2/2] git-subtree: use ls-remote to check the refspec passed
to pull and add
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
ls-remote is the correct way to check that a parameter is a valid fetchable target
Signed-off-by: Jérémy Rosen <jeremy.rosen@openwide.fr>
---
contrib/subtree/git-subtree.sh | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 8b9d114..61d4eab 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -503,13 +503,8 @@ cmd_add()
"cmd_add_commit" "$@"
elif [ $# -eq 2 ]; then
- # Technically we could accept a refspec here but we're
- # just going to turn around and add FETCH_HEAD under the
- # specified directory. Allowing a refspec might be
- # misleading because we won't do anything with any other
- # branches fetched via the refspec.
- git rev-parse -q --verify "$2^{commit}" >/dev/null ||
- die "'$2' does not refer to a commit"
+ git ls-remote --exit-code "$1" "$2" ||
+ die "'$2' is not a correct reference on '$1'"
"cmd_add_repository" "$@"
else
@@ -700,6 +695,8 @@ cmd_merge()
cmd_pull()
{
ensure_clean
+ git ls-remote --exit-code "$1" "$2" ||
+ die "'$2' is not a correct reference on '$1'"
git fetch "$@" || exit $?
revs=FETCH_HEAD
set -- $revs
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v3 0/2] shell: allow 'no-interactive-login' command to disable interactive shell
From: Jeff King @ 2013-03-12 10:47 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ethan Reesor, git, Sitaram Chamarty, Junio C Hamano,
Ramkumar Ramachandra, Greg Brockman
In-Reply-To: <20130309215237.GA24777@elie.Belkin>
On Sat, Mar 09, 2013 at 01:52:37PM -0800, Jonathan Nieder wrote:
> Here's a reroll along the lines described at
> http://thread.gmane.org/gmane.comp.version-control.git/216229
>
> As before, this series is meant to give users of basic 'git shell'
> setups a chance to imitate some nice behaviors that GitHub and
> gitolite offer in more complicated ways. Thanks for your help on it
> so far.
Thanks, this version looks good to me.
-Peff
^ permalink raw reply
* Re: [PATCH v2 1/4] config: factor out config file stack management
From: Jeff King @ 2013-03-12 10:52 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Junio C Hamano, git, Jens Lehmann, Ramsay Jones
In-Reply-To: <20130310165752.GB1136@sandbox-ub.fritz.box>
On Sun, Mar 10, 2013 at 05:57:53PM +0100, Heiko Voigt wrote:
> Because a config callback may start parsing a new file, the
> global context regarding the current config file is stored
> as a stack. Currently we only need to manage that stack from
> git_config_from_file. Let's factor it out to allow new
> sources of config data.
>
> [...]
>
> +static int do_config_from(struct config_file *top, config_fn_t fn, void *data)
> +{
> + int ret;
> +
> + /* push config-file parsing state stack */
> + top->prev = cf;
> + top->linenr = 1;
> + top->eof = 0;
> + strbuf_init(&top->value, 1024);
> + strbuf_init(&top->var, 1024);
> + cf = top;
> +
> + ret = git_parse_file(fn, data);
> +
> + /* pop config-file parsing state stack */
> + strbuf_release(&top->value);
> + strbuf_release(&top->var);
> + cf = top->prev;
> +
> + return ret;
> +}
Can we throw in a comment at the top here with the expected usage? In
particular, do_config_from is expecting the caller to have filled in
certain fields (at this point, top->f and top->name), but there is
nothing to make that clear.
-Peff
^ permalink raw reply
* Re: [PATCH v2 2/4] config: drop file pointer validity check in get_next_char()
From: Jeff King @ 2013-03-12 11:00 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Junio C Hamano, git, Jens Lehmann, Ramsay Jones
In-Reply-To: <20130310165857.GC1136@sandbox-ub.fritz.box>
On Sun, Mar 10, 2013 at 05:58:57PM +0100, Heiko Voigt wrote:
> The only location where cf is set in this file is in do_config_from().
> This function has only one callsite which is config_from_file(). In
> config_from_file() its ensured that the f member is set to non-zero.
> [...]
> - if (cf && ((f = cf->f) != NULL)) {
> + if (cf) {
I still think we can drop this conditional entirely. The complete call
graph looks like:
git_config_from_file
-> git_parse_file
-> get_next_char
-> get_value
-> get_next_char
-> parse_value
-> get_next_char
-> get_base_var
-> get_next_char
-> get_extended_base_var
-> get_next_char
That is, every path to get_next_char happens while we are in
git_config_from_file, and that function guarantees that cf = &top, and
that top.f != NULL. We do not have to even do any analysis of the
conditions for each call, because we never change "cf" nor "top.f"
except when we set them in git_config_from_file.
-Peff
^ permalink raw reply
* Re: [PATCH v2 3/4] config: make parsing stack struct independent from actual data source
From: Jeff King @ 2013-03-12 11:03 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Junio C Hamano, git, Jens Lehmann, Ramsay Jones
In-Reply-To: <20130310165940.GD1136@sandbox-ub.fritz.box>
On Sun, Mar 10, 2013 at 05:59:40PM +0100, Heiko Voigt wrote:
> diff --git a/config.c b/config.c
> index f55c43d..fe1c0e5 100644
> --- a/config.c
> +++ b/config.c
> @@ -10,20 +10,42 @@
> #include "strbuf.h"
> #include "quote.h"
>
> -typedef struct config_file {
> - struct config_file *prev;
> - FILE *f;
> +struct config_source {
> + struct config_source *prev;
> + void *data;
Would a union be more appropriate here? We do not ever have to pass it
directly as a parameter, since we pass the "struct config_source" to the
method functions.
It's still possible to screw up using a union, but it's slightly harder
than screwing up using a void pointer. And I do not think we need the
run-time flexibility offered by the void pointer in this case.
> +static int config_file_fgetc(struct config_source *conf)
> +{
> + FILE *f = conf->data;
> + return fgetc(f);
> +}
This could become just:
return fgetc(conf->u.f);
and so forth (might it make sense to give "f" a more descriptive name,
as we are adding other sources?).
-Peff
^ permalink raw reply
* Re: [PATCH v2 4/4] teach config parsing to read from strbuf
From: Jeff King @ 2013-03-12 11:18 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Junio C Hamano, git, Jens Lehmann, Ramsay Jones
In-Reply-To: <20130310170052.GE1136@sandbox-ub.fritz.box>
On Sun, Mar 10, 2013 at 06:00:52PM +0100, Heiko Voigt wrote:
> This can be used to read configuration values directly from gits
> database.
>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
This is lacking motivation. IIRC, the rest of the story is something
like "...so we can read .gitmodules directly from the repo" or something
like that?
> +struct config_strbuf {
> + struct strbuf *strbuf;
> + int pos;
> +};
>
> +static int config_strbuf_fgetc(struct config_source *conf)
> +{
> + struct config_strbuf *str = conf->data;
Yuck. If you used a union in the previous patch, then this could just go
inline into the "struct config_source".
> +int git_config_from_strbuf(config_fn_t fn, const char *name, struct strbuf *strbuf, void *data)
Should this be a "const struct strbuf *strbuf"? For that matter, is
there any reason not to take a bare pointer/len combination? It seems
likely that callers would get the data from read_sha1_file, which means
they have to stuff it into a strbuf for no good reason.
> diff --git a/test-config.c b/test-config.c
> new file mode 100644
> index 0000000..c650837
> --- /dev/null
> +++ b/test-config.c
> @@ -0,0 +1,40 @@
I'm slightly "meh" on this test-config program. Having to add a C test
harness like this is a good indication that we are short-changing users
of the shell API in favor of builtin C code.
Your series does not actually add any callers of the new function. The
obvious "patch 5/4" would be to plumb it into "git config --blob", and
then we can just directly test it there (there could be other callers
besides reading from a blob, of course, but I think the point of the
series is to head in that direction).
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] require pathspec for "git add -u/-A"
From: Jeff King @ 2013-03-12 11:28 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, git
In-Reply-To: <vpqmwubgsqy.fsf@grenoble-inp.fr>
On Sun, Mar 10, 2013 at 04:49:09PM +0100, Matthieu Moy wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without
> > pathspec, 2013-01-28), "git add -u/-A" that is run without pathspec
> > in a subdirectory will stop working sometime before Git 2.0, to wean
> > users off of the old default, in preparation for adopting the new
> > default in Git 2.0.
>
> I originally thought this step was necessary, but I changed my mind. The
> warning is big enough and doesn't need to be turned into an error.
>
> If this step is applied, it should be applied at 2.0, not before, as
> this is the big incompatible change. Re-introducing a new behavior won't
> harm users OTOH.
FWIW, I agree with this. The warning is enough without an error period
(unless the intent was to switch to the error and stay there forever,
but I do not think that is the proposal).
-Peff
PS I wonder how others are finding the warning? I'm finding it slightly
annoying. Perhaps I just haven't retrained my fingers yet. But one
problem with that retraining is that I type "git add -u" from the
root _most_ of the time, and it just works. But occasionally, I get
the "hey, do not do that" warning, because I'm in a subdir, even
though I would be happy to have the full-tree behavior. I guess we
already rejected the idea of being able to shut off the warning and
just get the new behavior, in favor of having people specify it
manually each time?
^ permalink raw reply
* [PATCH v3 01/13] dir.c: add MEASURE_EXCLUDE code for tracking exclude performance
From: Nguyễn Thái Ngọc Duy @ 2013-03-12 13:04 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1363093500-16796-1-git-send-email-pclouds@gmail.com>
Hot read_directory() codepaths are tracked. This could be helpful to
see how changes affect read_directory() performance.
Results are printed when environment variable GIT_MEASURE_EXCLUDE is set.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 100 insertions(+), 9 deletions(-)
diff --git a/dir.c b/dir.c
index 57394e4..69c045b 100644
--- a/dir.c
+++ b/dir.c
@@ -12,6 +12,32 @@
#include "refs.h"
#include "wildmatch.h"
+#ifdef MEASURE_EXCLUDE
+static uint32_t tv_treat_leading_path;
+static uint32_t tv_read_directory;
+static uint32_t tv_treat_one_path;
+static uint32_t tv_is_excluded;
+static uint32_t tv_prep_exclude;
+static uint32_t tv_last_exclude_matching;
+static uint32_t tv_dir_add_name;
+static uint32_t tv_directory_exists_in_index;
+static uint32_t tv_simplify_away;
+static uint32_t tv_index_name_exists;
+static uint32_t tv_lazy_init_name_hash;
+#define START_CLOCK() \
+ { \
+ struct timeval tv1, tv2; \
+ gettimeofday(&tv1, NULL);
+#define STOP_CLOCK(v) \
+ gettimeofday(&tv2, NULL); \
+ v += (uint64_t)tv2.tv_sec * 1000000 + tv2.tv_usec - \
+ (uint64_t)tv1.tv_sec * 1000000 - tv1.tv_usec; \
+ }
+#else
+#define START_CLOCK()
+#define STOP_CLOCK(v)
+#endif
+
struct path_simplify {
int len;
const char *path;
@@ -768,8 +794,11 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
const char *basename = strrchr(pathname, '/');
basename = (basename) ? basename+1 : pathname;
+ START_CLOCK();
prep_exclude(dir, pathname, basename-pathname);
+ STOP_CLOCK(tv_prep_exclude);
+ START_CLOCK();
for (i = EXC_CMDL; i <= EXC_FILE; i++) {
group = &dir->exclude_list_group[i];
for (j = group->nr - 1; j >= 0; j--) {
@@ -780,6 +809,7 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
return exclude;
}
}
+ STOP_CLOCK(tv_last_exclude_matching);
return NULL;
}
@@ -897,9 +927,14 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)
static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)
{
- if (!(dir->flags & DIR_SHOW_IGNORED) &&
- cache_name_exists(pathname, len, ignore_case))
- return NULL;
+ if (!(dir->flags & DIR_SHOW_IGNORED)) {
+ struct cache_entry *ce;
+ START_CLOCK();
+ ce = cache_name_exists(pathname, len, ignore_case);
+ STOP_CLOCK(tv_index_name_exists);
+ if (ce)
+ return NULL;
+ }
ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
return dir->entries[dir->nr++] = dir_entry_new(pathname, len);
@@ -1034,8 +1069,12 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
const char *dirname, int len, int exclude,
const struct path_simplify *simplify)
{
+ int ret;
+ START_CLOCK();
/* The "len-1" is to strip the final '/' */
- switch (directory_exists_in_index(dirname, len-1)) {
+ ret = directory_exists_in_index(dirname, len-1);
+ STOP_CLOCK(tv_directory_exists_in_index);
+ switch (ret) {
case index_directory:
if ((dir->flags & DIR_SHOW_OTHER_DIRECTORIES) && exclude)
break;
@@ -1179,7 +1218,9 @@ static int get_index_dtype(const char *path, int len)
int pos;
struct cache_entry *ce;
+ START_CLOCK();
ce = cache_name_exists(path, len, 0);
+ STOP_CLOCK(tv_index_name_exists);
if (ce) {
if (!ce_uptodate(ce))
return DT_UNKNOWN;
@@ -1244,7 +1285,12 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
const struct path_simplify *simplify,
int dtype, struct dirent *de)
{
- int exclude = is_excluded(dir, path->buf, &dtype);
+ int exclude;
+
+ START_CLOCK();
+ exclude = is_excluded(dir, path->buf, &dtype);
+ STOP_CLOCK(tv_is_excluded);
+
if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
&& exclude_matches_pathspec(path->buf, path->len, simplify))
dir_add_ignored(dir, path->buf, path->len);
@@ -1292,17 +1338,23 @@ static enum path_treatment treat_path(struct dir_struct *dir,
int baselen,
const struct path_simplify *simplify)
{
- int dtype;
+ int dtype, ret;
if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git"))
return path_ignored;
strbuf_setlen(path, baselen);
strbuf_addstr(path, de->d_name);
- if (simplify_away(path->buf, path->len, simplify))
+ START_CLOCK();
+ ret = simplify_away(path->buf, path->len, simplify);
+ STOP_CLOCK(tv_simplify_away);
+ if (ret)
return path_ignored;
dtype = DTYPE(de);
- return treat_one_path(dir, path, simplify, dtype, de);
+ START_CLOCK();
+ ret = treat_one_path(dir, path, simplify, dtype, de);
+ STOP_CLOCK(tv_treat_one_path);
+ return ret;
}
/*
@@ -1345,7 +1397,9 @@ static int read_directory_recursive(struct dir_struct *dir,
contents++;
if (check_only)
break;
+ START_CLOCK();
dir_add_name(dir, path.buf, path.len);
+ STOP_CLOCK(tv_dir_add_name);
}
closedir(fdir);
out:
@@ -1405,6 +1459,7 @@ static int treat_leading_path(struct dir_struct *dir,
len--;
if (!len)
return 1;
+ START_CLOCK();
baselen = 0;
while (1) {
cp = path + baselen + !!baselen;
@@ -1428,6 +1483,7 @@ static int treat_leading_path(struct dir_struct *dir,
}
}
strbuf_release(&sb);
+ STOP_CLOCK(tv_treat_leading_path);
return rc;
}
@@ -1439,8 +1495,43 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const char
return dir->nr;
simplify = create_simplify(pathspec);
- if (!len || treat_leading_path(dir, path, len, simplify))
+ if (!len || treat_leading_path(dir, path, len, simplify)) {
+#ifdef MEASURE_EXCLUDE
+ /* The first call triggers lazy_init_name_hash() */
+ START_CLOCK();
+ index_name_exists(&the_index, "", 0, ignore_case);
+ STOP_CLOCK(tv_lazy_init_name_hash);
+#endif
+ START_CLOCK();
read_directory_recursive(dir, path, len, 0, simplify);
+ STOP_CLOCK(tv_read_directory);
+ }
+#ifdef MEASURE_EXCLUDE
+ if (getenv("GIT_MEASURE_EXCLUDE")) {
+ fprintf(stderr, "treat_leading_path: %10.3f\n",
+ (float)tv_treat_leading_path / 1000000);
+ fprintf(stderr, "read_directory: %10.3f\n",
+ (float)tv_read_directory / 1000000);
+ fprintf(stderr, "+treat_one_path: %10.3f\n",
+ (float)tv_treat_one_path / 1000000);
+ fprintf(stderr, "++is_excluded: %10.3f\n",
+ (float)tv_is_excluded / 1000000);
+ fprintf(stderr, "+++prep_exclude: %10.3f\n",
+ (float)tv_prep_exclude / 1000000);
+ fprintf(stderr, "+++matching: %10.3f\n",
+ (float)tv_last_exclude_matching / 1000000);
+ fprintf(stderr, "++dir_exist: %10.3f\n",
+ (float)tv_directory_exists_in_index / 1000000);
+ fprintf(stderr, "++index_name_exists: %10.3f\n",
+ (float)tv_index_name_exists / 1000000);
+ fprintf(stderr, "lazy_init_name_hash: %10.3f\n",
+ (float)tv_lazy_init_name_hash / 1000000);
+ fprintf(stderr, "+simplify_away: %10.3f\n",
+ (float)tv_simplify_away / 1000000);
+ fprintf(stderr, "+dir_add_name: %10.3f\n",
+ (float)tv_dir_add_name / 1000000);
+ }
+#endif
free_simplify(simplify);
qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* [PATCH v3 00/13] Exclude optimizations
From: Nguyễn Thái Ngọc Duy @ 2013-03-12 13:04 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1362896070-17456-1-git-send-email-pclouds@gmail.com>
Result of today. I cherry-picked nd/read-directory-recursive-optim to
see how far I can get after pulling all the tricks. This is a slower
machine so time is longer. Anyway, read_directory time is reduced
about 70% in the end.
function before after
----------------------------------
treat_leading_path: 0.000 0.000
read_directory: 4.102 1.235
+treat_one_path: 2.843 0.531
++is_excluded: 2.632 0.102
+++prep_exclude: 0.225 0.040
+++matching: 2.054 0.035
++dir_exist: 0.035 0.035
++index_name_exists: 0.292 0.225
lazy_init_name_hash: 0.258 0.155
+simplify_away: 0.085 0.083
+dir_add_name: 0.446 0.000
I don't expect all these patches to go in. The meat is
nd/read-directory-recursive-optim (or 10/13) and 09/13. Some other
patches are safe even if they do not contribute much to the gain. The
last two are probably not worth the trouble.
Nguyễn Thái Ngọc Duy (13):
dir.c: add MEASURE_EXCLUDE code for tracking exclude performance
match_pathname: avoid calling strncmp if baselen is 0
dir.c: inline convenient *_icase helpers
match_basename: use strncmp instead of strcmp
match_{base,path}name: replace strncmp_icase with memequal_icase
dir: pass pathname length to last_exclude_matching
exclude: avoid calling prep_exclude on entries of the same directory
exclude: record baselen in the pattern
exclude: filter out patterns not applicable to the current directory
read_directory: avoid invoking exclude machinery on tracked files
Preallocate hash tables when the number of inserts are known in advance
name-hash: allow to lookup a name with precalculated base hash
read_directory: calculate name hashes incrementally
Makefile | 1 +
attr.c | 6 +-
cache.h | 2 -
diffcore-rename.c | 1 +
dir.c | 392 ++++++++++++++++++++++++++++++++++++++++++++----------
dir.h | 26 +++-
hash.h | 7 +
name-hash.c | 49 ++++---
name-hash.h (new) | 45 +++++++
read-cache.c | 1 +
unpack-trees.c | 1 +
11 files changed, 431 insertions(+), 100 deletions(-)
create mode 100644 name-hash.h
--
1.8.1.2.536.gf441e6d
^ permalink raw reply
* [PATCH v3 02/13] match_pathname: avoid calling strncmp if baselen is 0
From: Nguyễn Thái Ngọc Duy @ 2013-03-12 13:04 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1363093500-16796-1-git-send-email-pclouds@gmail.com>
This reduces "git status" user time by a little bit. This is the
sorted results of 10 consecutive runs of "git ls-files
--exclude-standard -o" on webkit.git, compiled with gcc -O2:
treat_leading_path: 0.000 0.000
read_directory: 4.102 3.674
+treat_one_path: 2.843 2.427
++is_excluded: 2.632 2.221
+++prep_exclude: 0.225 0.224
+++matching: 2.054 1.650
++dir_exist: 0.035 0.035
++index_name_exists: 0.292 0.288
lazy_init_name_hash: 0.258 0.257
+simplify_away: 0.085 0.085
+dir_add_name: 0.446 0.441
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dir.c b/dir.c
index 69c045b..32a3adb 100644
--- a/dir.c
+++ b/dir.c
@@ -688,8 +688,8 @@ int match_pathname(const char *pathname, int pathlen,
* may not end with a trailing slash though.
*/
if (pathlen < baselen + 1 ||
- (baselen && pathname[baselen] != '/') ||
- strncmp_icase(pathname, base, baselen))
+ (baselen && (pathname[baselen] != '/' ||
+ strncmp_icase(pathname, base, baselen))))
return 0;
namelen = baselen ? pathlen - baselen - 1 : pathlen;
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* [PATCH v3 03/13] dir.c: inline convenient *_icase helpers
From: Nguyễn Thái Ngọc Duy @ 2013-03-12 13:04 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1363093500-16796-1-git-send-email-pclouds@gmail.com>
Like the previous patch, this cuts down the number of str*cmp calls in
read_directory (which does _a lot_). Again sorted results on webkit.git:
treat_leading_path: 0.000 0.000
read_directory: 3.674 3.558
+treat_one_path: 2.427 2.305
++is_excluded: 2.221 2.098
+++prep_exclude: 0.224 0.223
+++matching: 1.650 1.529
++dir_exist: 0.035 0.035
++index_name_exists: 0.288 0.291
lazy_init_name_hash: 0.257 0.257
+simplify_away: 0.085 0.086
+dir_add_name: 0.441 0.445
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 16 ----------------
dir.h | 18 +++++++++++++++---
2 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/dir.c b/dir.c
index 32a3adb..a69c8ac 100644
--- a/dir.c
+++ b/dir.c
@@ -47,22 +47,6 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, in
int check_only, const struct path_simplify *simplify);
static int get_dtype(struct dirent *de, const char *path, int len);
-/* helper string functions with support for the ignore_case flag */
-int strcmp_icase(const char *a, const char *b)
-{
- return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
-}
-
-int strncmp_icase(const char *a, const char *b, size_t count)
-{
- return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
-}
-
-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)
{
diff --git a/dir.h b/dir.h
index c3eb4b5..560ade4 100644
--- a/dir.h
+++ b/dir.h
@@ -200,9 +200,21 @@ extern int remove_dir_recursively(struct strbuf *path, int flag);
/* tries to remove the path with empty directories along it, ignores ENOENT */
extern int remove_path(const char *path);
-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);
+/* helper string functions with support for the ignore_case flag */
+static inline int strcmp_icase(const char *a, const char *b)
+{
+ return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
+}
+
+static inline int strncmp_icase(const char *a, const char *b, size_t count)
+{
+ return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
+}
+
+static inline int fnmatch_icase(const char *pattern, const char *string, int flags)
+{
+ return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));
+}
/*
* The prefix part of pattern must not contains wildcards.
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* [PATCH v3 04/13] match_basename: use strncmp instead of strcmp
From: Nguyễn Thái Ngọc Duy @ 2013-03-12 13:04 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1363093500-16796-1-git-send-email-pclouds@gmail.com>
strncmp provides length information, compared to strcmp, which could
be taken advantage by the implementation. Even better, we could check
if the lengths are equal before calling strncmp, eliminating a bit of
strncmp calls.
treat_leading_path: 0.000 0.000
read_directory: 3.558 3.578
+treat_one_path: 2.305 2.323
++is_excluded: 2.098 2.117
+++prep_exclude: 0.223 0.224
+++matching: 1.529 1.544
++dir_exist: 0.035 0.035
++index_name_exists: 0.291 0.290
lazy_init_name_hash: 0.257 0.258
+simplify_away: 0.086 0.087
+dir_add_name: 0.445 0.445
While at there, fix an inconsistency about pattern/patternlen in how
attr handles EXC_FLAG_MUSTBEDIR. When parse_exclude_pattern detects
this flag, it sets patternlen _not_ to include the trailing slash and
expects the caller to trim it. add_exclude does, parse_attr_line does
not.
In attr.c, the pattern could be "foo/" while patternlen tells us it
only has 3 chars. Some functions do not care about patternlen and will
see the pattern as "foo/" while others may see it as "foo". This patch
makes patternlen 4 in this case. (Although for a piece of mind,
perhaps we should trim it to "foo" like exclude, and never pass a
pathname like "abc/" to match_{base,path}name)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
attr.c | 2 ++
dir.c | 8 +++++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/attr.c b/attr.c
index e2f9377..1818ba5 100644
--- a/attr.c
+++ b/attr.c
@@ -255,6 +255,8 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
&res->u.pat.patternlen,
&res->u.pat.flags,
&res->u.pat.nowildcardlen);
+ if (res->u.pat.flags & EXC_FLAG_MUSTBEDIR)
+ res->u.pat.patternlen++;
if (res->u.pat.flags & EXC_FLAG_NEGATIVE) {
warning(_("Negative patterns are ignored in git attributes\n"
"Use '\\!' for literal leading exclamation."));
diff --git a/dir.c b/dir.c
index a69c8ac..a2ab200 100644
--- a/dir.c
+++ b/dir.c
@@ -636,12 +636,14 @@ int match_basename(const char *basename, int basenamelen,
int flags)
{
if (prefix == patternlen) {
- if (!strcmp_icase(pattern, basename))
+ if (patternlen == basenamelen &&
+ !strncmp_icase(pattern, basename, patternlen))
return 1;
} else if (flags & EXC_FLAG_ENDSWITH) {
if (patternlen - 1 <= basenamelen &&
- !strcmp_icase(pattern + 1,
- basename + basenamelen - patternlen + 1))
+ !strncmp_icase(pattern + 1,
+ basename + basenamelen - patternlen + 1,
+ patternlen - 1))
return 1;
} else {
if (fnmatch_icase(pattern, basename, 0) == 0)
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* [PATCH v3 06/13] dir: pass pathname length to last_exclude_matching
From: Nguyễn Thái Ngọc Duy @ 2013-03-12 13:04 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1363093500-16796-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 26c3b3a..58739f3 100644
--- a/dir.c
+++ b/dir.c
@@ -794,9 +794,9 @@ int is_excluded_from_list(const char *pathname,
*/
static struct exclude *last_exclude_matching(struct dir_struct *dir,
const char *pathname,
+ int pathlen,
int *dtype_p)
{
- int pathlen = strlen(pathname);
int i, j;
struct exclude_list_group *group;
struct exclude *exclude;
@@ -827,10 +827,12 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
* scans all exclude lists to determine whether pathname is excluded.
* Returns 1 if true, otherwise 0.
*/
-static int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)
+static int is_excluded(struct dir_struct *dir,
+ const char *pathname, int pathlen,
+ int *dtype_p)
{
struct exclude *exclude =
- last_exclude_matching(dir, pathname, dtype_p);
+ last_exclude_matching(dir, pathname, pathlen, dtype_p);
if (exclude)
return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return 0;
@@ -893,7 +895,8 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
if (ch == '/') {
int dt = DT_DIR;
exclude = last_exclude_matching(check->dir,
- path->buf, &dt);
+ path->buf, path->len,
+ &dt);
if (exclude) {
check->exclude = exclude;
return exclude;
@@ -905,7 +908,7 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
/* An entry in the index; cannot be a directory with subentries */
strbuf_setlen(path, 0);
- return last_exclude_matching(check->dir, name, dtype);
+ return last_exclude_matching(check->dir, name, namelen, dtype);
}
/*
@@ -1297,7 +1300,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
int exclude;
START_CLOCK();
- exclude = is_excluded(dir, path->buf, &dtype);
+ exclude = is_excluded(dir, path->buf, path->len, &dtype);
STOP_CLOCK(tv_is_excluded);
if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* [PATCH v3 05/13] match_{base,path}name: replace strncmp_icase with memequal_icase
From: Nguyễn Thái Ngọc Duy @ 2013-03-12 13:04 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1363093500-16796-1-git-send-email-pclouds@gmail.com>
treat_leading_path: 0.000 0.000
read_directory: 3.578 3.501
+treat_one_path: 2.323 2.257
++is_excluded: 2.117 2.056
+++prep_exclude: 0.224 0.216
+++matching: 1.544 1.493
++dir_exist: 0.035 0.035
++index_name_exists: 0.290 0.292
lazy_init_name_hash: 0.258 0.256
+simplify_away: 0.087 0.084
+dir_add_name: 0.445 0.447
Suggested-by: Fredrik Gustafsson <iveqy@iveqy.com>
Suggested-by: Antoine Pelisse <apelisse@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/dir.c b/dir.c
index a2ab200..26c3b3a 100644
--- a/dir.c
+++ b/dir.c
@@ -47,6 +47,29 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, in
int check_only, const struct path_simplify *simplify);
static int get_dtype(struct dirent *de, const char *path, int len);
+static inline int memequal_icase(const char *a, const char *b, int n)
+{
+ if (!memcmp(a, b, n))
+ return 1;
+
+ if (!ignore_case)
+ return 0;
+
+ /*
+ * This assumes that ASCII is used in most repositories. We
+ * try the ascii-only version first (i.e. Git's
+ * toupper). Failing that, fall back to normal strncasecmp.
+ */
+ while (n && toupper(*a) == toupper(*b)) {
+ a++;
+ b++;
+ n--;
+ }
+ if (!n)
+ return 1;
+ return !strncasecmp(a, b, n);
+}
+
inline int git_fnmatch(const char *pattern, const char *string,
int flags, int prefix)
{
@@ -637,11 +660,11 @@ int match_basename(const char *basename, int basenamelen,
{
if (prefix == patternlen) {
if (patternlen == basenamelen &&
- !strncmp_icase(pattern, basename, patternlen))
+ memequal_icase(pattern, basename, patternlen))
return 1;
} else if (flags & EXC_FLAG_ENDSWITH) {
if (patternlen - 1 <= basenamelen &&
- !strncmp_icase(pattern + 1,
+ memequal_icase(pattern + 1,
basename + basenamelen - patternlen + 1,
patternlen - 1))
return 1;
@@ -675,7 +698,7 @@ int match_pathname(const char *pathname, int pathlen,
*/
if (pathlen < baselen + 1 ||
(baselen && (pathname[baselen] != '/' ||
- strncmp_icase(pathname, base, baselen))))
+ !memequal_icase(pathname, base, baselen))))
return 0;
namelen = baselen ? pathlen - baselen - 1 : pathlen;
@@ -689,7 +712,7 @@ int match_pathname(const char *pathname, int pathlen,
if (prefix > namelen)
return 0;
- if (strncmp_icase(pattern, name, prefix))
+ if (!memequal_icase(pattern, name, prefix))
return 0;
pattern += prefix;
name += prefix;
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* [PATCH v3 07/13] exclude: avoid calling prep_exclude on entries of the same directory
From: Nguyễn Thái Ngọc Duy @ 2013-03-12 13:04 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1363093500-16796-1-git-send-email-pclouds@gmail.com>
prep_exclude is only necessary when we enter or leave a directory. Now
it's called for every entry in a directory. With this patch, the
number of prep_exclude calls in webkit.git goes from 188k down to
11k. This patch does not make exclude any faster, but it prepares for
making prep_exclude heavier in terms of computation, where a large
number of calls may have bigger impacts.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
dir.c | 10 +++++++++-
dir.h | 1 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/dir.c b/dir.c
index 58739f3..f8f7a7e 100644
--- a/dir.c
+++ b/dir.c
@@ -804,7 +804,10 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
basename = (basename) ? basename+1 : pathname;
START_CLOCK();
- prep_exclude(dir, pathname, basename-pathname);
+ if (!dir->exclude_prepared) {
+ prep_exclude(dir, pathname, basename-pathname);
+ dir->exclude_prepared = 1;
+ }
STOP_CLOCK(tv_prep_exclude);
START_CLOCK();
@@ -894,6 +897,7 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
if (ch == '/') {
int dt = DT_DIR;
+ check->dir->exclude_prepared = 0;
exclude = last_exclude_matching(check->dir,
path->buf, path->len,
&dt);
@@ -908,6 +912,7 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
/* An entry in the index; cannot be a directory with subentries */
strbuf_setlen(path, 0);
+ check->dir->exclude_prepared = 0;
return last_exclude_matching(check->dir, name, namelen, dtype);
}
@@ -1394,6 +1399,7 @@ static int read_directory_recursive(struct dir_struct *dir,
if (!fdir)
goto out;
+ dir->exclude_prepared = 0;
while ((de = readdir(fdir)) != NULL) {
switch (treat_path(dir, de, &path, baselen, simplify)) {
case path_recurse:
@@ -1415,6 +1421,7 @@ static int read_directory_recursive(struct dir_struct *dir,
}
closedir(fdir);
out:
+ dir->exclude_prepared = 0;
strbuf_release(&path);
return contents;
@@ -1486,6 +1493,7 @@ static int treat_leading_path(struct dir_struct *dir,
break;
if (simplify_away(sb.buf, sb.len, simplify))
break;
+ dir->exclude_prepared = 0;
if (treat_one_path(dir, &sb, simplify,
DT_DIR, NULL) == path_ignored)
break; /* do not recurse into it */
diff --git a/dir.h b/dir.h
index 560ade4..0748407 100644
--- a/dir.h
+++ b/dir.h
@@ -86,6 +86,7 @@ struct dir_struct {
/* Exclude info */
const char *exclude_per_dir;
+ int exclude_prepared;
/*
* We maintain three groups of exclude pattern lists:
--
1.8.1.2.536.gf441e6d
^ 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