Re: ZSH segmentation fault while completing git mv dir/
Jeff Epler jep...@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 xx1000'` 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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: rebase: strange failures to apply patc 3-way
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposal: sharing .git/config
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
On Tue, Mar 12, 2013 at 10:43 AM, Ramkumar Ramachandra artag...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions/investigations on git-subtree and tags
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
From 12490724ae955719694d825dff4fa9e0d2709c1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Rosen?= jeremy.ro...@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.ro...@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
From 05d1dd56217be59d69952a41d97e204cc7821948 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Rosen?= jeremy.ro...@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.ro...@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
Re: [PATCH v3 0/2] shell: allow 'no-interactive-login' command to disable interactive shell
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] config: factor out config file stack management
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] config: drop file pointer validity check in get_next_char()
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] config: make parsing stack struct independent from actual data source
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] require pathspec for git add -u/-A
On Sun, Mar 10, 2013 at 04:49:09PM +0100, Matthieu Moy wrote: Junio C Hamano gits...@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? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 01/13] dir.c: add MEASURE_EXCLUDE code for tracking exclude performance
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 pclo...@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 * 100 + tv2.tv_usec - \
+ (uint64_t)tv1.tv_sec * 100 - 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,
[PATCH v3 02/13] match_pathname: avoid calling strncmp if baselen is 0
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 pclo...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 00/13] Exclude optimizations
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 03/13] dir.c: inline convenient *_icase helpers
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 pclo...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 04/13] match_basename: use strncmp instead of strcmp
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 pclo...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 05/13] match_{base,path}name: replace strncmp_icase with memequal_icase
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 iv...@iveqy.com
Suggested-by: Antoine Pelisse apeli...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 07/13] exclude: avoid calling prep_exclude on entries of the same directory
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 pclo...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 08/13] exclude: record baselen in the pattern
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
attr.c | 4 +++-
dir.c | 19 ++-
dir.h | 6 +-
3 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/attr.c b/attr.c
index 1818ba5..b89da33 100644
--- a/attr.c
+++ b/attr.c
@@ -249,12 +249,14 @@ static struct match_attr *parse_attr_line(const char
*line, const char *src,
res-u.attr = git_attr_internal(name, namelen);
else {
char *p = (char *)(res-state[num_attr]);
+ int pattern_baselen;
memcpy(p, name, namelen);
res-u.pat.pattern = p;
parse_exclude_pattern(res-u.pat.pattern,
res-u.pat.patternlen,
res-u.pat.flags,
- res-u.pat.nowildcardlen);
+ res-u.pat.nowildcardlen,
+ pattern_baselen);
if (res-u.pat.flags EXC_FLAG_MUSTBEDIR)
res-u.pat.patternlen++;
if (res-u.pat.flags EXC_FLAG_NEGATIVE) {
diff --git a/dir.c b/dir.c
index f8f7a7e..932fd2f 100644
--- a/dir.c
+++ b/dir.c
@@ -390,10 +390,11 @@ static int no_wildcard(const char *string)
void parse_exclude_pattern(const char **pattern,
int *patternlen,
int *flags,
- int *nowildcardlen)
+ int *nowildcardlen,
+ int *pattern_baselen)
{
const char *p = *pattern;
- size_t i, len;
+ int i, len;
*flags = 0;
if (*p == '!') {
@@ -405,12 +406,15 @@ void parse_exclude_pattern(const char **pattern,
len--;
*flags |= EXC_FLAG_MUSTBEDIR;
}
- for (i = 0; i len; i++) {
+ for (i = len - 1; i = 0; i--) {
if (p[i] == '/')
break;
}
- if (i == len)
+ if (i 0) {
*flags |= EXC_FLAG_NODIR;
+ *pattern_baselen = -1;
+ } else
+ *pattern_baselen = i;
*nowildcardlen = simple_length(p);
/*
* we should have excluded the trailing slash from 'p' too,
@@ -421,6 +425,8 @@ void parse_exclude_pattern(const char **pattern,
*nowildcardlen = len;
if (*p == '*' no_wildcard(p + 1))
*flags |= EXC_FLAG_ENDSWITH;
+ else if (*nowildcardlen != len)
+ *pattern_baselen = -1;
*pattern = p;
*patternlen = len;
}
@@ -432,8 +438,10 @@ void add_exclude(const char *string, const char *base,
int patternlen;
int flags;
int nowildcardlen;
+ int pattern_baselen;
- parse_exclude_pattern(string, patternlen, flags, nowildcardlen);
+ parse_exclude_pattern(string, patternlen, flags,
+ nowildcardlen, pattern_baselen);
if (flags EXC_FLAG_MUSTBEDIR) {
char *s;
x = xmalloc(sizeof(*x) + patternlen + 1);
@@ -449,6 +457,7 @@ void add_exclude(const char *string, const char *base,
x-nowildcardlen = nowildcardlen;
x-base = base;
x-baselen = baselen;
+ x-pattern_baselen = pattern_baselen;
x-flags = flags;
x-srcpos = srcpos;
ALLOC_GROW(el-excludes, el-nr + 1, el-alloc);
diff --git a/dir.h b/dir.h
index 0748407..cb50a85 100644
--- a/dir.h
+++ b/dir.h
@@ -44,6 +44,7 @@ struct exclude_list {
int nowildcardlen;
const char *base;
int baselen;
+ int pattern_baselen;
int flags;
/*
@@ -172,7 +173,10 @@ extern struct exclude_list *add_exclude_list(struct
dir_struct *dir,
extern int add_excludes_from_file_to_list(const char *fname, const char *base,
int baselen,
struct exclude_list *el, int
check_index);
extern void add_excludes_from_file(struct dir_struct *, const char *fname);
-extern void parse_exclude_pattern(const char **string, int *patternlen, int
*flags, int *nowildcardlen);
+extern void parse_exclude_pattern(const char **string,
+ int *patternlen, int *flags,
+ int *nowildcardlen,
+ int *pattern_baselen);
extern void add_exclude(const char *string, const char *base,
int baselen, struct exclude_list *el, int srcpos);
extern void clear_exclude_list(struct exclude_list *el);
--
1.8.1.2.536.gf441e6d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 09/13] exclude: filter out patterns not applicable to the current directory
.gitignore files are spread over directories (*) so that when we check
for ignored files at foo, we are not bothered by foo/bar/.gitignore,
which contains ignore rules for foo/bar only.
This is not enough. foo/.gitignore can contain the pattern
foo/bar/*.c. When we stay at foo, we know that the pattern cannot
match anything. Similarly, the pattern /autom4te.cache at root
directory cannot match anything in foo. This patch attempts to filter
out such patterns to drive down matching cost.
The algorithm implemented here is a naive one. Patterns can be either
active or passive:
- When we enter a new directory (e.g. from root to foo), currently
active patterns may no longer be applicable and can be turned to
passive.
- On the opposite, when we leave a directory (foo back to roo),
passive patterns may come alive again.
We could do smarter things. But this implementation cuts a big portion
of cost already (and solves the root .gitignore is evil problem).
There's probably no need to be smart.
(*) this design forces us to try to find .gitignore at every
directory. On webkit.git that equals to 6k open syscalls. It feels
like .svn on every directory again. I suggest we add
~/.gitignore.master, containing the list .gitignore files in
worktree. If this file exists, we don't poke at every directory for
.gitignore.
treat_leading_path: 0.000 0.000
read_directory: 3.455 2.879
+treat_one_path: 2.203 1.620
++is_excluded:2.000 1.416
+++prep_exclude: 0.171 0.198
+++matching: 1.509 0.904
++dir_exist: 0.036 0.035
++index_name_exists: 0.292 0.289
lazy_init_name_hash: 0.257 0.257
+simplify_away: 0.084 0.085
+dir_add_name:0.446 0.446
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
dir.c | 93 +--
dir.h | 1 +
2 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/dir.c b/dir.c
index 932fd2f..c57bf06 100644
--- a/dir.c
+++ b/dir.c
@@ -458,7 +458,7 @@ void add_exclude(const char *string, const char *base,
x-base = base;
x-baselen = baselen;
x-pattern_baselen = pattern_baselen;
- x-flags = flags;
+ x-flags = flags | EXC_FLAG_ACTIVE;
x-srcpos = srcpos;
ALLOC_GROW(el-excludes, el-nr + 1, el-alloc);
el-excludes[el-nr++] = x;
@@ -591,6 +591,87 @@ void add_excludes_from_file(struct dir_struct *dir, const
char *fname)
die(cannot use %s as an exclude file, fname);
}
+static int pattern_match_base(struct dir_struct *dir,
+ const char *base, int baselen,
+ const struct exclude *exc)
+{
+ const char *pattern;
+
+ /*
+* TODO: if a patterns come from a .gitignore, exc-base would
+* be the same for all of them. We could compare once and
+* reuse the result, instead of perform the comparison per
+* pattern like this.
+*/
+ if (exc-baselen) {
+ if (baselen exc-baselen + 1)
+ return 0;
+
+ if (base[exc-baselen] != '/' ||
+ memcmp(base, exc-base, exc-baselen))
+ return 0;
+
+ base += exc-baselen + 1;
+ baselen -= exc-baselen + 1;
+ }
+
+ if (baselen != exc-pattern_baselen)
+ return 0;
+
+ if (exc-pattern_baselen) {
+ pattern = exc-pattern;
+ if (*pattern == '/')
+ pattern++;
+ if (memcmp(base, pattern, exc-pattern_baselen))
+ return 0;
+ }
+
+ return 1;
+}
+
+/*
+ * If pushed is non-zero, we have entered a new directory. Some
+ * pathname patterns may no longer applicable. Go over all active
+ * patterns and disable them if so.
+ *
+ * If popped is non-zero, we have left a directory. Inactive patterns
+ * may be applicable again. Go over them and re-enable if so.
+ */
+static void scan_patterns(struct dir_struct *dir,
+ const char *base, int baselen,
+ int pushed, int popped)
+{
+ int i, j, k;
+
+ for (i = EXC_CMDL; i = EXC_FILE; i++) {
+ struct exclude_list_group *group = dir-exclude_list_group[i];
+ for (j = group-nr - 1; j = 0; j--) {
+ struct exclude_list *list = group-el[j];
+ for (k = 0; k list-nr; k++) {
+ struct exclude *exc = list-excludes[k];
+
+ /*
+* No base (i.e. EXC_FLAG_NODIR) or
+* applicable to many bases (**
+* patterns)
+*/
+ if (exc-pattern_baselen == -1)
+ continue;
+
+ if (exc-flags EXC_FLAG_ACTIVE) {
+
[PATCH v3 10/13] read_directory: avoid invoking exclude machinery on tracked files
read_directory() (and its friendly wrapper fill_directory) collects
untracked/ignored files by traversing through the whole worktree,
feeding every entry to treat_one_path(), where each entry is checked
against .gitignore patterns.
One may see that tracked files can't be excluded and we do not need to
run them through exclude machinery. On repos where there are many
.gitignore patterns and/or a lot of tracked files, this unnecessary
processing can become expensive.
This patch avoids it mostly for normal cases. Directories are still
processed as before. DIR_SHOW_IGNORED and DIR_COLLECT_IGNORED are not
normally used unless some options are given (e.g. checkout
--overwrite-ignore, add -f...)
treat_one_path's behavior changes when taking this shortcut. With
current code, when a non-directory path is not excluded,
treat_one_path calls treat_file, which returns the initial value of
exclude_file and causes treat_one_path to return path_handled. With
this patch, on the same conditions, treat_one_path returns
path_ignored.
read_directory_recursive() cares about this difference. Check out the
snippet:
while (...) {
switch (treat_path(...)) {
case path_ignored:
continue;
case path_handled:
break;
}
contents++;
if (check_only)
break;
dir_add_name(dir, path.buf, path.len);
}
If path_handled is returned, contents goes up. And if check_only is
true, the loop could be broken early. These will not happen when
treat_one_path (and its wrapper treat_path) returns
path_ignored. dir_add_name internally does a cache_name_exists() check
so it makes no difference.
To avoid this behavior change, treat_one_path is instructed to skip
the optimization when check_only or contents is used.
Finally some numbers (best of 20 runs) that shows why it's worth all
the hassle:
git status | webkit linux-2.6 libreoffice-core gentoo-x86
-+--
before | 1.097s0.208s 0.399s 0.539s
after| 0.736s0.159s 0.248s 0.501s
nr. patterns |89 376 19 0
nr. tracked | 182k 40k 63k 101k
treat_leading_path: 0.000 0.000
read_directory: 2.879 1.299
+treat_one_path: 1.620 0.599
++is_excluded:1.416 0.103
+++prep_exclude: 0.198 0.040
+++matching: 0.904 0.036
++dir_exist: 0.035 0.036
++index_name_exists: 0.289 0.291
lazy_init_name_hash: 0.257 0.257
+simplify_away: 0.085 0.082
+dir_add_name:0.446 0.000
Tracked-down-by: Karsten Blees karsten.bl...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
dir.c | 80 ---
1 file changed, 53 insertions(+), 27 deletions(-)
diff --git a/dir.c b/dir.c
index c57bf06..6809dd2 100644
--- a/dir.c
+++ b/dir.c
@@ -43,8 +43,11 @@ struct path_simplify {
const char *path;
};
-static int read_directory_recursive(struct dir_struct *dir, const char *path,
int len,
- int check_only, const struct path_simplify *simplify);
+static void read_directory_recursive(struct dir_struct *dir,
+const char *path, int len,
+int check_only,
+const struct path_simplify *simplify,
+int *contents);
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)
@@ -1184,7 +1187,7 @@ static enum directory_treatment treat_directory(struct
dir_struct *dir,
const char *dirname, int len, int exclude,
const struct path_simplify *simplify)
{
- int ret;
+ int contents = 0, ret;
START_CLOCK();
/* The len-1 is to strip the final '/' */
ret = directory_exists_in_index(dirname, len-1);
@@ -1219,19 +1222,19 @@ static enum directory_treatment treat_directory(struct
dir_struct *dir,
* check if it contains only ignored files
*/
if ((dir-flags DIR_SHOW_IGNORED) !exclude) {
- int ignored;
dir-flags = ~DIR_SHOW_IGNORED;
dir-flags |= DIR_HIDE_EMPTY_DIRECTORIES;
- ignored = read_directory_recursive(dir, dirname, len, 1,
simplify);
+ read_directory_recursive(dir, dirname, len, 1, simplify,
contents);
dir-flags = ~DIR_HIDE_EMPTY_DIRECTORIES;
dir-flags |= DIR_SHOW_IGNORED;
- return ignored ? ignore_directory : show_directory;
+ return contents ? ignore_directory : show_directory;
}
if (!(dir-flags DIR_SHOW_IGNORED)
!(dir-flags
[PATCH v3 11/13] Preallocate hash tables when the number of inserts are known in advance
This avoids unnecessary allocations and reinsertions. On webkit.git
(i.e. about 100k inserts to the name hash table), this reduces about
100ms out of 3s user time.
treat_leading_path: 0.000 0.000
read_directory: 1.299 1.305
+treat_one_path: 0.599 0.599
++is_excluded:0.103 0.103
+++prep_exclude: 0.040 0.040
+++matching: 0.036 0.035
++dir_exist: 0.036 0.035
++index_name_exists: 0.291 0.292
lazy_init_name_hash: 0.257 0.155
+simplify_away: 0.082 0.083
+dir_add_name:0.000 0.000
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
diffcore-rename.c | 1 +
hash.h| 7 +++
name-hash.c | 1 +
3 files changed, 9 insertions(+)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 512d0ac..8d3d9bb 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -389,6 +389,7 @@ static int find_exact_renames(struct diff_options *options)
struct hash_table file_table;
init_hash(file_table);
+ preallocate_hash(file_table, (rename_src_nr + rename_dst_nr) * 2);
for (i = 0; i rename_src_nr; i++)
insert_file_table(file_table, -1, i, rename_src[i].p-one);
diff --git a/hash.h b/hash.h
index b875ce6..244d1fe 100644
--- a/hash.h
+++ b/hash.h
@@ -40,4 +40,11 @@ static inline void init_hash(struct hash_table *table)
table-array = NULL;
}
+static inline void preallocate_hash(struct hash_table *table, unsigned int
size)
+{
+ assert(table-size == 0 table-nr == 0 table-array == NULL);
+ table-size = size;
+ table-array = xcalloc(sizeof(struct hash_table_entry), size);
+}
+
#endif
diff --git a/name-hash.c b/name-hash.c
index 942c459..1287a19 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -92,6 +92,7 @@ static void lazy_init_name_hash(struct index_state *istate)
if (istate-name_hash_initialized)
return;
+ preallocate_hash(istate-name_hash, istate-cache_nr * 2);
for (nr = 0; nr istate-cache_nr; nr++)
hash_index_entry(istate, istate-cache[nr]);
istate-name_hash_initialized = 1;
--
1.8.1.2.536.gf441e6d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 12/13] name-hash: allow to lookup a name with precalculated base hash
index_name_exists_base() differs from index_name_exists() in how the
hash is calculated. It uses the base hash as the seed and skips the
baselen part.
The intention is to reduce hashing cost during directory traversal,
where the hash of the directory is calculated once, and used as base
hash for all entries inside.
This poses a constraint in hash function. The function has not changed
since 2008. With luck it'll never change again. If resuming hashing at
any characters are not feasible with a new (better) hash function,
maybe we could stop at directory boundary.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Makefile | 1 +
cache.h | 2 --
dir.c | 1 +
name-hash.c | 48 ++--
name-hash.h (new) | 45 +
read-cache.c | 1 +
unpack-trees.c| 1 +
7 files changed, 71 insertions(+), 28 deletions(-)
create mode 100644 name-hash.h
diff --git a/Makefile b/Makefile
index 26d3332..8d753af 100644
--- a/Makefile
+++ b/Makefile
@@ -690,6 +690,7 @@ LIB_H += mailmap.h
LIB_H += merge-blobs.h
LIB_H += merge-recursive.h
LIB_H += mergesort.h
+LIB_H += name-hash.h
LIB_H += notes-cache.h
LIB_H += notes-merge.h
LIB_H += notes.h
diff --git a/cache.h b/cache.h
index e493563..cf33c67 100644
--- a/cache.h
+++ b/cache.h
@@ -313,7 +313,6 @@ static inline void remove_name_hash(struct cache_entry *ce)
#define refresh_cache(flags) refresh_index(the_index, (flags), NULL, NULL,
NULL)
#define ce_match_stat(ce, st, options) ie_match_stat(the_index, (ce), (st),
(options))
#define ce_modified(ce, st, options) ie_modified(the_index, (ce), (st),
(options))
-#define cache_name_exists(name, namelen, igncase)
index_name_exists(the_index, (name), (namelen), (igncase))
#define cache_name_is_other(name, namelen) index_name_is_other(the_index,
(name), (namelen))
#define resolve_undo_clear() resolve_undo_clear_index(the_index)
#define unmerge_cache_entry_at(at) unmerge_index_entry_at(the_index, at)
@@ -442,7 +441,6 @@ extern int write_index(struct index_state *, int newfd);
extern int discard_index(struct index_state *);
extern int unmerged_index(const struct index_state *);
extern int verify_path(const char *path);
-extern struct cache_entry *index_name_exists(struct index_state *istate, const
char *name, int namelen, int igncase);
extern int index_name_pos(const struct index_state *, const char *name, int
namelen);
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
#define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */
diff --git a/dir.c b/dir.c
index 6809dd2..5fda5af 100644
--- a/dir.c
+++ b/dir.c
@@ -11,6 +11,7 @@
#include dir.h
#include refs.h
#include wildmatch.h
+#include name-hash.h
#ifdef MEASURE_EXCLUDE
static uint32_t tv_treat_leading_path;
diff --git a/name-hash.c b/name-hash.c
index 1287a19..572d232 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -7,30 +7,7 @@
*/
#define NO_THE_INDEX_COMPATIBILITY_MACROS
#include cache.h
-
-/*
- * This removes bit 5 if bit 6 is set.
- *
- * That will make US-ASCII characters hash to their upper-case
- * equivalent. We could easily do this one whole word at a time,
- * but that's for future worries.
- */
-static inline unsigned char icase_hash(unsigned char c)
-{
- return c ~((c 0x40) 1);
-}
-
-static unsigned int hash_name(const char *name, int namelen)
-{
- unsigned int hash = 0x123;
-
- while (namelen--) {
- unsigned char c = *name++;
- c = icase_hash(c);
- hash = hash*101 + c;
- }
- return hash;
-}
+#include name-hash.h
static void hash_index_entry_directories(struct index_state *istate, struct
cache_entry *ce)
{
@@ -152,9 +129,11 @@ static int same_name(const struct cache_entry *ce, const
char *name, int namelen
return slow_same_name(name, namelen, ce-name, namelen len ? namelen
: len);
}
-struct cache_entry *index_name_exists(struct index_state *istate, const char
*name, int namelen, int icase)
+static inline struct cache_entry *find_entry_by_hash(struct index_state
*istate,
+const char *name, int
namelen,
+unsigned int hash,
+int icase)
{
- unsigned int hash = hash_name(name, namelen);
struct cache_entry *ce;
lazy_init_name_hash(istate);
@@ -189,3 +168,20 @@ struct cache_entry *index_name_exists(struct index_state
*istate, const char *na
}
return NULL;
}
+
+struct cache_entry *index_name_exists(struct index_state *istate,
+ const char *name, int namelen,
+ int icase)
+{
+ unsigned int hash = hash_name(name, namelen);
+ return find_entry_by_hash(istate, name, namelen, hash, icase);
+}
+
+struct cache_entry
[PATCH v3 13/13] read_directory: calculate name hashes incrementally
Instead of index_name_exists() calculating a hash for full pathname
for every entry, we calculate partial hash per directory, use it as a
seed. The number of characters that icase_hash has to chew will
reduce.
treat_leading_path: 0.000 0.000
read_directory: 1.296 1.235
+treat_one_path: 0.599 0.531
++is_excluded:0.102 0.102
+++prep_exclude: 0.040 0.040
+++matching: 0.035 0.035
++dir_exist: 0.035 0.035
++index_name_exists: 0.292 0.225
lazy_init_name_hash: 0.155 0.155
+simplify_away: 0.082 0.083
+dir_add_name:0.000 0.000
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
dir.c | 44 +---
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/dir.c b/dir.c
index 5fda5af..8638dcd 100644
--- a/dir.c
+++ b/dir.c
@@ -46,6 +46,7 @@ struct path_simplify {
static void read_directory_recursive(struct dir_struct *dir,
const char *path, int len,
+unsigned int hash,
int check_only,
const struct path_simplify *simplify,
int *contents);
@@ -1044,12 +1045,17 @@ static struct dir_entry *dir_entry_new(const char
*pathname, int len)
return ent;
}
-static struct dir_entry *dir_add_name(struct dir_struct *dir, const char
*pathname, int len)
+static struct dir_entry *dir_add_name(struct dir_struct *dir,
+ const char *pathname, int len,
+ unsigned int hash, int baselen)
{
if (!(dir-flags DIR_SHOW_IGNORED)) {
struct cache_entry *ce;
START_CLOCK();
- ce = cache_name_exists(pathname, len, ignore_case);
+ ce = index_name_exists_base(the_index,
+ hash, baselen,
+ pathname, len,
+ ignore_case);
STOP_CLOCK(tv_index_name_exists);
if (ce)
return NULL;
@@ -1225,7 +1231,9 @@ static enum directory_treatment treat_directory(struct
dir_struct *dir,
if ((dir-flags DIR_SHOW_IGNORED) !exclude) {
dir-flags = ~DIR_SHOW_IGNORED;
dir-flags |= DIR_HIDE_EMPTY_DIRECTORIES;
- read_directory_recursive(dir, dirname, len, 1, simplify,
contents);
+ read_directory_recursive(dir, dirname, len,
+hash_name(dirname, len),
+1, simplify, contents);
dir-flags = ~DIR_HIDE_EMPTY_DIRECTORIES;
dir-flags |= DIR_SHOW_IGNORED;
@@ -1234,7 +1242,9 @@ static enum directory_treatment treat_directory(struct
dir_struct *dir,
if (!(dir-flags DIR_SHOW_IGNORED)
!(dir-flags DIR_HIDE_EMPTY_DIRECTORIES))
return show_directory;
- read_directory_recursive(dir, dirname, len, 1, simplify, contents);
+ read_directory_recursive(dir, dirname, len,
+hash_name(dirname, len),
+1, simplify, contents);
if (!contents)
return ignore_directory;
return show_directory;
@@ -1401,6 +1411,8 @@ enum path_treatment {
static enum path_treatment treat_one_path(struct dir_struct *dir,
struct strbuf *path,
+ unsigned int hash,
+ int baselen,
const struct path_simplify *simplify,
int dtype, struct dirent *de,
int exclude_shortcut_ok)
@@ -1416,7 +1428,8 @@ static enum path_treatment treat_one_path(struct
dir_struct *dir,
dtype != DT_DIR) {
struct cache_entry *ce;
START_CLOCK();
- ce = cache_name_exists(path-buf, path-len, ignore_case);
+ ce = index_name_exists_base(the_index, hash, baselen,
+ path-buf, path-len, ignore_case);
STOP_CLOCK(tv_index_name_exists);
if (ce)
return path_ignored;
@@ -1467,6 +1480,7 @@ static enum path_treatment treat_one_path(struct
dir_struct *dir,
static enum path_treatment treat_path(struct dir_struct *dir,
struct dirent *de,
struct strbuf *path,
+ unsigned int hash,
int baselen,
const struct path_simplify *simplify,
int exclude_shortcut_ok)
@@ -1485,7 +1499,8 @@ static enum
Updating not actual branch
Hi All, I have a question if there is a posibility tu update a branch which is not actual working copy. I have following situation: A - B - C - I - J master \ - D - E - F feature 1 \ G - H feature 2 (working copy) I would like tu update whole tree with latest changes in master A - B - C - I - J master \ - D* - E* - F* feature 1 \ G* - H* feature 2 (working copy) Is there some way how to do it without swithing to each branch and update them manually? Thanks, Jan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] require pathspec for git add -u/-A
Jeff King p...@peff.net writes: 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. Same here. Not terribly disturbing, but I did get the warning occasionally, even though I'm the author of the patch introducing it ;-). 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? Somehow, but we may find a way to do so, as long as it temporary (i.e. something that will have no effect after the transition period), and that is is crystal clear that it's temporary. All proposals up to now were rejected because of the risk of confusing users (either shutting the warning blindly, or letting people think they could keep the current behavior after Git 2.0). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Updating not actual branch
On Tue, 12 Mar 2013 14:22:00 +0100
Jan Pešta jan.pe...@certicon.cz wrote:
I have a question if there is a posibility tu update a branch which
is not actual working copy.
I have following situation:
A - B - C - I - J master
\ - D - E - F feature 1
\ G - H feature 2 (working copy)
I would like tu update whole tree with latest changes in master
A - B - C - I - J master
\ - D* - E* - F* feature 1
\ G* - H*
feature 2 (working copy)
Is there some way how to do it without swithing to each branch and
update them manually?
It's partially possible to do: you are able to forcibly fetch a remote
object into any local branch provided it's not checked out. Hence, in
your case you'll be able to update any branch excapt for feature 2.
To do this, you could use the explicit refspec when fetching, like this:
$ git fetch origin +src1:dst1 '+blooper 1:feature 1'
(Consult the `git fetch` manual for more info on using refspecs.)
One possible downside is that I'm not sure this approach would play
nicely with remote branches, if you have any. I mean, direct fetching
into local branches will unlikely to update their matching upstream
remote branches.
So supposedly a better way to go would be to write a script which would
go like this:
1) Do a simple one-argument `git fetch remotename` to fetch from
the specified remote and update its remote branches.
2) Run `git for-each-ref`, and for each local branch found first check
whether it's currently checked out (that is, HEAD points at it)
and ignore the branch if it is; otherwise do `git update-ref`
updating the branch pointer from its upstream branch -- referring to
that using the ref@{upstream} syntax.
(Consult the gitrevisions manual for more info on this syntax.)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/19] Document pull-all and push-all
Am 09.03.2013 20:28, schrieb Paul Campbell: From 7dcd40ab8687a588b7b0c6ff914a7cfb601b6774 Mon Sep 17 00:00:00 2001 From: Herman van Rink r...@initfour.nl Date: Tue, 27 Mar 2012 13:59:16 +0200 Subject: [PATCH 14/19] Document pull-all and push-all --- contrib/subtree/git-subtree.txt | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt index e0957ee..c8fc103 100644 --- a/contrib/subtree/git-subtree.txt +++ b/contrib/subtree/git-subtree.txt @@ -92,13 +92,19 @@ pull:: Exactly like 'merge', but parallels 'git pull' in that it fetches the given commit from the specified remote repository. - + +pull-all:: + Perform a pull operation on all in .gittrees registered subtrees. + push:: Does a 'split' (see below) using the prefix supplied and then does a 'git push' to push the result to the repository and refspec. This can be used to push your subtree to different branches of the remote repository. +push-all:: + Perform a pull operation on all in .gittrees registered subtrees. - pull-push + split:: Extract a new, synthetic project history from the history of the prefix subtree. The new history -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] submodule: add 'deinit' command
On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann jens.lehm...@web.de wrote: With git submodule init the user is able to tell git he cares about one or more submodules and wants to have it populated on the next call to git submodule update. But currently there is no easy way he could tell git he does not care about a submodule anymore and wants to get rid of his local work tree (except he knows a lot about submodule internals and removes the submodule.$name.url setting from .git/config together with the work tree himself). Help those users by providing a 'deinit' command. This removes the whole submodule.name section from .git/config either for the given submodule(s) or for all those which have been initialized if '.' is given. Fail if the current work tree contains modifications unless forced. Complain when for a submodule given on the command line the url setting can't be found in .git/config, but nonetheless don't fail. Add tests and link the man pages of git submodule deinit and git rm to assist the user in deciding whether removing or unregistering the submodule is the right thing to do for him. Signed-off-by: Jens Lehmann jens.lehm...@web.de Probably because I was new to this command, I was confused by this output. $ git submodule deinit submodule rm 'submodule' Submodule 'submodule' (gerrit:foo/submodule) unregistered for path 'submodule' $ git rm submodule rm 'submodule' This line is confusing to me in the deinit command: rm 'submodule' It doesn't mean what git usually means when it says this to me. See how the 'git rm' command says the same thing but means something different in the next command. In the deinit case, git removes the workdir contents of 'submodule' and it reports rm 'submodule'. In the rm case, git removes the submodule link from the tree and rmdirs the empty 'submodule' directory. I think this would be clearer if 'git deinit' said rm 'submodule/*' or maybe Removed workdir for 'submodule' Is it just me? Phil -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] config: factor out config file stack management
On Tue, Mar 12, 2013 at 06:52:00AM -0400, Jeff King wrote:
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.
Of course. Will do that in the next iteration. How about I squash this in:
diff --git a/config.c b/config.c
index b8c8640..b7632c9 100644
--- a/config.c
+++ b/config.c
@@ -948,6 +954,9 @@ int git_default_config(const char *var, const char *value, v
return 0;
}
+/* The fields data, name and the source specific callbacks of top need
+ * to be initialized before calling this function.
+ */
static int do_config_from_source(struct config_source *top, config_fn_t fn, voi
{
int ret;
I would add that to the third patch:
config: make parsing stack struct independent from actual data source
because that contains the final modification to config_file/config_source.
Cheers Heiko
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] config: drop file pointer validity check in get_next_char()
On Tue, Mar 12, 2013 at 07:00:03AM -0400, Jeff King wrote:
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.
Ok if you say so I will do that :-). I was thinking about adding a patch
that would remove cf as a global variable and explicitely pass it down
to get_next_char. That makes it more obvious that it actually is != NULL.
Looking at your callgraph I do not think its that much work. What do you
think?
BTW, how did you generate this callgraph? Do you have a nice tool for that?
Cheers Heiko
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well
On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann jens.lehm...@web.de wrote:
Am 05.03.2013 22:17, schrieb Phil Hord:
On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann jens.lehm...@web.de wrote:
Am 05.03.2013 19:34, schrieb Junio C Hamano:
Eric Cousineau eacousin...@gmail.com writes:
...
I am not entirely convinced we would want --include-super in the
first place, though. It does not belong to submodule foreach;
it is doing something _outside_ the submoudules.
I totally agree with that. First, adding --include-super does not
belong into the --post-order patch at all, as that is a different
topic (even though it belongs to the same use case Eric has). Also
the reason why we are thinking about adding the --post-order option
IMO cuts the other way for --include-super: It is so easy to do
that yourself I'm not convinced we should add an extra option to
foreach for that, especially as it has nothing to do with submodules.
So I think we should just drop --include-super.
I agree it should not be part of this commit, but I've often found
myself in need of an --include-super switch. To me,
git-submodule-foreach means visit all my .git repos in this project
and execute $cmd. It's a pity that the super-project is considered a
second-class citizen in this regard.
Hmm, for me the super-project is a very natural second-class citizen
to git *submodule* foreach. But also I understand that sometimes the
user wants to apply a command to superproject and submodules alike (I
just recently did exactly that with git gc on our build server).
I have to do this sometimes:
${cmd} git submodule foreach --recursive '${cmd}'
I often forget the first part in scripts, though, and I've seen others
do it too. I usually create a function for it in git-heavy scripts.
In a shell, it usually goes like this:
git submodule foreach --recursive '${cmd}'
uphomedel{30-ish}endbackspaceenter
It'd be easier if I could just include a switch for this, and maybe
even create an alias for it. But maybe this is different command
altogether.
Are you sure you wouldn't forget to provide such a switch too? ;-)
No. However, when I remember to add the switch, my shell history will
remember it for me. This does not happen naturally for me in the
uphomedel{30-ish}... workflow.
I also hope this switch grows up into a configuration option someday.
Or maybe a completely different command, like I said before; because I
actually think it could be dangerous as a configuration option since
it would have drastic consequences for users executing scripts or
commands in other users' environments.
I'm still not convinced we should add a new switch, as it can easily
be achieved by adding ${cmd} to your scripts. And on the command
line you could use an alias like this one to achieve that:
[alias]
recurse = !sh -c \$@ git submodule foreach --recursive $@\
Yes, making the feature itself a 2nd-class citizen. :-)
But this alias also denies me the benefit of the --post-order option.
For 'git recurse git push', for example, I wouldn't want the
superproject push to occur first; I would want it to occur last after
the submodules have been successfully pushed.
I agree this should go in some other commit, but I do not think it is
so trivial it should never be considered as a feature for git. That's
all I'm trying to say.
Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Updating not actual branch
Jan Pešta jan.pe...@certicon.cz writes: I have following situation: A - B - C - I - J master \ - D - E - F feature 1 \ G - H feature 2 (working copy) I would like tu update whole tree with latest changes in master A - B - C - I - J master \ - D* - E* - F* feature 1 \ G* - H* feature 2 (working copy) Is there some way how to do it without swithing to each branch and update them manually? With these asterisks I would assume that you are rebasing feature #n on top of updated master. As rebasing requires you to have a working tree, so that you can resolve potential conflicts between your work and work done on the updated upstream, you fundamentally would need to check out the branch you work on. In the case you depicted where feature-1 is a complete subset of feature-2, you are rebasing both of them, and you do not end up in a nasty conflict, you could start from this state: A---B---C master \ D---E---F feature-1 \ G---H feature-2 update the master from the upstream: $ git checkout master ; git pull A---B---C---I---J master \ D---E---F feature-1 \ G---H feature-2 rebase feature-2 on top of the updated master: $ git rebase master feature-2 G'--H' feature-2 / D'--E'--F' / A---B---C---I---J master \ D---E---F feature-1 \ G---H and finally repoint feature-1 to its updated version: $ git branch -f feature-1 F' G'--H' feature-2 / D'--E'--F' feature-1 / A---B---C---I---J master \ D---E---F \ G---H Depending on the interaction between commits C..J and C..F, your rebasing of feature-2 may end up not losing any of D', E' or F'. Imagine the case where J was committed on the upstream by applying the same patch as the original E; E' will become redundant and the result of your git rebase master feature-2 may look like this instead: G'--H' feature-2 / D'--F' / A---B---C---I---J master \ D---E---F feature-1 \ G---H Or J could remove something E depends on, in which case you may have to add it back with a new commit X when you rebase feature-2, like so: G'--H' feature-2 / D'--X---E'--F' / A---B---C---I---J master \ D---E---F feature-1 \ G---H Because you cannot mechanically decide where the tip of updated feature-1 has to be, you would need to use your brain to decide where to repoint the tip of feature-1 in the last step. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] config: drop file pointer validity check in get_next_char()
On Tue, Mar 12, 2013 at 05:00:56PM +0100, Heiko Voigt wrote:
On Tue, Mar 12, 2013 at 07:00:03AM -0400, Jeff King wrote:
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.
Ok if you say so I will do that :-). I was thinking about adding a patch
that would remove cf as a global variable and explicitely pass it down
to get_next_char. That makes it more obvious that it actually is != NULL.
Looking at your callgraph I do not think its that much work. What do you
think?
I just had a look and unfortunately there are other functions that use
this variable (namely handle_path_include) for which its not that easy
to pass this in. So I will just drop the check here.
Cheers Heiko
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] submodule: add 'deinit' command
Phil Hord phil.h...@gmail.com writes: I think this would be clearer if 'git deinit' said rm 'submodule/*' or maybe Removed workdir for 'submodule' Is it just me? The latter may probably be better. After cloning the superproject, you show interest in individual submodules by saying git submodule init that submodule and until then your .git/config in the superproject does not indicate that you are interested in that submodule, and you won't have a submodule checkout. deinit is a way to revert back to that original state; recording that you are no longer interested in the submodule is the primary effect, and removal of its checkout is a mere side effect of it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] config: make parsing stack struct independent from actual data source
On Tue, Mar 12, 2013 at 07:03:55AM -0400, Jeff King wrote:
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.
No we do not need the void pointer flexibility. But that means every new
source would need to add to this union. Junio complained about that fact
when I first added the extra members directly to the struct. A union
does not waste that much space and we get some seperation using the
union members. Since this struct is local only I think that should be
ok.
+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?).
Will change that.
Cheers Heiko
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] teach config parsing to read from strbuf
On Tue, Mar 12, 2013 at 07:18:06AM -0400, Jeff King wrote:
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 hvo...@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?
Will add some more here.
+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.
pointer/len should be fine too. I just used strbuf since when you find
out later that you need to modify the string its easier to handle. A
config parser should not need to do that so I will change that.
diff --git a/test-config.c b/test-config.c
new file mode 100644
index 000..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.
I mainly did this because I needed some test for the config part while
developing the fetch renamed submodules series.
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).
Since this is a split of the series mentioned above there are no real
callers yet. The main reason for the split was that I wanted to reduce
the review burden of one big series into multiple reviews of smaller
chunks. If you think it is useful to add the --blob option I can also
test from there. It could actually be useful to look at certain
.gitmodules options from the submodule script.
Cheers Heiko
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
[ Added Junio and git to the recipients, and leaving a lot of stuff quoted due to that... ] On Mon, Mar 11, 2013 at 9:16 PM, Theodore Ts'o ty...@mit.edu wrote: On Tue, Mar 12, 2013 at 03:10:53PM +1100, James Morris wrote: On Tue, 12 Mar 2013, Stephen Rothwell wrote: The top commit in the security tree today is a merge of v3.9-rc2. This is a completely unnecessary merge as the tree before the merge was a subset of v3.9-rc1 and so if the merge had been done using anything but the tag, it would have just been a fast forward. I know that this is now deliberate behaviour on git's behalf, but isn't there some way we can make this easier on maintainers who are just really just trying to pick a new starting point for their trees after a release? (at least I assume that is what James was trying to do) Yes, and I was merging to a tag as required by Linus. Now, quite frankly, I'd prefer people not merge -rc tags either, just real releases. -rc tags are certainly *much* better than merging random daily stuff, but the basic rule should be don't back-merge AT ALL rather than back-merge tags. That said, you didn't really want a merge at all, you just wanted to sync up and start development. Which is different (but should still prefer real releases, and only use rc tags if it's fixing stuff that happened in the merge window - which may be the case here). Why not just force the head of the security tree to be v3.9-rc2? Then you don't end up creating a completely unnecessary merge commit, and users who were at the previous head of the security tree will experience a fast forward when they pull your new head. So I think that may *technically* be the right solution, but it's a rather annoying UI issue, partly because you can't just do it in a single operation (you can't do a pull of the tag to both fetch and fast-forward it), but partly because git reset --hard is also an operation that can lose history, so it's something that people should be nervous about, and shouldn't use as some kind of standard let's just fast-forward to Linus' tree thing. At the same time, it's absolutely true that when *I* pull a signed tag from a downstream developer, I don't want a fast-forward, because then I'd lose the signature. So when a maintainer pulls a submaintainer tree, you want the signature to come upstream, but when a submaintainer wants to just sync up with upstream, you don't want to generate the pointless signed merge commit, because the signature is already upstream because it's a public tag. So gthe behavior of git pull is fundamentally ambiguous. But git doesn't know the difference between official public upstream tag and signed tag used to verify the pull request. I'm adding the git list just to get this issue out there and see if people have any ideas. I've got a couple of workarounds, but they aren't wonderful.. One is simple: git config alias.sync=pull --ff-only which works fine, but forces submaintainers to be careful when doing things like this, and using a special command to do back-merges. And maybe that's the right thing to do? Back-merges *are* special, after all. But the above alias is particularly fragile, in that there's both pull and merge that people want to use this for, and it doesn't really handle both. And --ff-only will obviously fail if you actually have some work in your tree, and want to do a real merge, so then you have to do that differently. So I'm mentioning this as a better model than git reset, but not really a *solution*. That said, the fact that --ff-only errors out if you have local development may actually be a big bonus - because you really shouldn't do merges at all if you have local development, but syncing up to my tree if you don't have it (and are going to start it) may be something reasonable. Now, the other approach - and perhaps preferable, but requiring actual changes to git itself - is to do the non-fast-forward merge *only* for FETCH_HEAD, which already has magic semantics in other ways. So if somebody does git fetch linus git merge v3.8 to sync with me, they would *not* get a merge commit with a signature, just a fast-forward. But if you do git pull linus v3.8 or a git fetch linus v3.8 git merge FETCH_HEAD it would look like a maintainer merge and stash the signature in the merge commit rather than fast-forward. It would probably work in practice. The final approach might be to make it like the merge summary and simply make it configurable _and_ have a command line flag for it, defaulting to our current behavior or to the above suggested default on for FETCH_HEAD, off for anything else. Hmm? Linus -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/13] match_basename: use strncmp instead of strcmp
--- 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;
I can see that you kept strncmp(), was it worse with memcmp() ?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
On Tue, Mar 12, 2013 at 6:13 PM, Linus Torvalds
torva...@linux-foundation.org wrote:
Why not just force the head of the security tree to be v3.9-rc2? Then
you don't end up creating a completely unnecessary merge commit, and
users who were at the previous head of the security tree will
experience a fast forward when they pull your new head.
So I think that may *technically* be the right solution, but it's a
rather annoying UI issue, partly because you can't just do it in a
single operation (you can't do a pull of the tag to both fetch and
fast-forward it), but partly because git reset --hard is also an
operation that can lose history, so it's something that people should
be nervous about, and shouldn't use as some kind of standard let's
just fast-forward to Linus' tree thing.
In many cases, git rebase x does the exact same thing as
git reset --hard x, with an added safeguard: if you forgot to upstream
something, it'll boil up on top of x.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
On Tue, Nov 27, 2012 at 7:41 AM, Matt McClure matthewlmccl...@gmail.com wrote: On Tuesday, November 27, 2012, David Aguilar wrote: It seems that there is an edge case here that we are not accounting for: unmodified worktree paths, when checked out into the temporary directory, can be edited by the tool when comparing against older commits. These edits will be lost. Yes. That is exactly my desired use case. I want to make edits while I'm reviewing the diff. I took a crack at implementing the change to make difftool -d use symlinks more aggressively. I've tested it lightly, and it works for the limited cases I've tried. This is my first foray into the Git source code, so it's entirely possible that there are unintended side effects and regressions if other features depend on the same code path and make different assumptions. https://github.com/matthewlmcclure/git/compare/difftool-directory-symlink-work-tree Your thoughts on the change? -- Matt McClure http://www.matthewlmcclure.com http://www.mapmyfitness.com/profile/matthewlmcclure -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] config: factor out config file stack management
On Tue, Mar 12, 2013 at 04:44:35PM +0100, Heiko Voigt wrote: 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. Of course. Will do that in the next iteration. How about I squash this in: [...] +/* The fields data, name and the source specific callbacks of top need + * to be initialized before calling this function. + */ static int do_config_from_source(struct config_source *top, config_fn_t fn, voi I think that is OK, but it may be even better to list the fields by name. Also, our multi-line comment style is: /* * Multi-line comment. */ I would add that to the third patch: config: make parsing stack struct independent from actual data source because that contains the final modification to config_file/config_source. It does not matter to the end result, but I find it helps with reviewing when the comment is added along with the function, and then expanded as the function is changed. It helps to understand the effects of later patches if they need to tweak comments. I do not care that much in this instance, since we have already discussed it, and I know what is going on, though. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] config: drop file pointer validity check in get_next_char()
On Tue, Mar 12, 2013 at 05:00:56PM +0100, Heiko Voigt wrote: 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. Ok if you say so I will do that :-). I was thinking about adding a patch that would remove cf as a global variable and explicitely pass it down to get_next_char. That makes it more obvious that it actually is != NULL. Looking at your callgraph I do not think its that much work. What do you think? Yeah, I think that makes it more obvious what is going on, but you will run into trouble if you ever want that information to cross the void * boundary of a config callback, as adding a new parameter there is hard. The only place we do that now is for git_config_include, and I think you could get by with stuffing it into the config_include_data struct (which is already there to deal with this problem). It would also make something like this patch hard or impossible: http://article.gmane.org/gmane.comp.version-control.git/190267 as it wants to access cf from arbitrary callbacks. That is not a patch that is actively under consideration, but I had considered polishing it up at some point. BTW, how did you generate this callgraph? Do you have a nice tool for that? I just did it manually in vi, working backwards from every instance of get_next_char, as it is not that big a graph. I suspect something like cscope could make it easier, but I don't know of any tool that will build the graph automatically (it would not be that hard from the data cscope has, though, so I wouldn't be surprised if such a tool exists). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
On Tue, Mar 12, 2013 at 12:09 PM, John Keeping j...@keeping.me.uk wrote:
On Tue, Mar 12, 2013 at 02:12:29PM -0400, Matt McClure wrote:
On Tue, Nov 27, 2012 at 7:41 AM, Matt McClure matthewlmccl...@gmail.com
wrote:
Your thoughts on the change?
Please include the patch in your message so that interested parties can
comment on it here, especially since the compare view on GitHub seems to
mangle the tabs.
For others' reference the patch is:
-- 8 --
From: Matt McClure matt.mccl...@mapmyfitness.com
Subject: [PATCH] difftool: Make directory diff symlink work tree
difftool -d formerly knew how to symlink to the work tree when the work
tree contains uncommitted changes. In practice, prior to this change, it
would not symlink to the work tree in case there were no uncommitted
changes, even when the user invoked difftool with the form:
git difftool -d [--options] commit [--] [path...]
This form is to view the changes you have in your working tree
relative to the named commit. You can use HEAD to compare it
with the latest commit, or a branch name to compare with the tip
of a different branch.
Instead, prior to this change, difftool would use the file's HEAD blob
sha1 to find its content rather than the work tree content. This change
teaches `git diff --raw` to emit the null SHA1 for consumption by
difftool -d, so that difftool -d will use a symlink rather than a copy
of the file.
Before:
$ git diff --raw HEAD^ -- diff-lib.c
:100644 100644 f35de0f... ead9399... M diff-lib.c
After:
$ ./git diff --raw HEAD^ -- diff-lib.c
:100644 100644 f35de0f... 000... M diff-lib.c
Interesting approach. While this does get the intended behavior
for difftool, I'm afraid this would be a grave regression for
existing git diff --raw users who cannot have such behavior.
I don't think we could do this without adding an additional flag
to trigger this change in behavior (e.g. --null-sha1-for-?)
so that existing users are unaffected by the change.
It feels like forcing the null SHA-1 is heavy-handed, but I
haven't thought it through enough.
While this may be a quick way to get this behavior,
I wonder if there is a better way.
Does anybody else have any comments/suggestions on how to
better accomplish this?
---
diff-lib.c | 4
1 file changed, 4 insertions(+)
diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..ead9399 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -319,6 +319,10 @@ static int show_modified(struct rev_info *revs,
return -1;
}
+ if (!cached hashcmp(old-sha1, new-sha1)) {
+ sha1 = null_sha1;
+ }
+
if (revs-combine_merges !cached
(hashcmp(sha1, old-sha1) || hashcmp(old-sha1, new-sha1))) {
struct combine_diff_path *p;
--
1.8.2.rc2.4.g7799588
--
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: Make directory diff symlink work tree
difftool -d formerly knew how to symlink to the work tree when the work
tree contains uncommitted changes. In practice, prior to this change, it
would not symlink to the work tree in case there were no uncommitted
changes, even when the user invoked difftool with the form:
git difftool -d [--options] commit [--] [path...]
This form is to view the changes you have in your working tree
relative to the named commit. You can use HEAD to compare it
with the latest commit, or a branch name to compare with the tip
of a different branch.
Instead, prior to this change, difftool would use the file's HEAD blob
sha1 to find its content rather than the work tree content. This change
teaches `git diff --raw` to emit the null SHA1 for consumption by
difftool -d, so that difftool -d will use a symlink rather than a copy
of the file.
Before:
$ git diff --raw HEAD^ -- diff-lib.c
:100644 100644 f35de0f... ead9399... M diff-lib.c
After:
$ ./git diff --raw HEAD^ -- diff-lib.c
:100644 100644 f35de0f... 000... M diff-lib.c
When I tried this I got the expected behaviour even without this patch.
It turns out that an uncommitted, but *staged* change emits the SHA1 of
the blob rather than the null SHA1. Do you get the desired behaviour if
you git reset before using difftool?
If so I think you want some new mode of operation for difftool instead
of this patch which will also affect unrelated commands.
---
diff-lib.c | 4
1 file changed, 4 insertions(+)
diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..ead9399 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -319,6 +319,10 @@ static int show_modified(struct rev_info *revs,
return -1;
}
+ if (!cached hashcmp(old-sha1, new-sha1)) {
+ sha1 = null_sha1;
+ }
+
if (revs-combine_merges !cached
(hashcmp(sha1, old-sha1) || hashcmp(old-sha1, new-sha1))) {
struct combine_diff_path *p;
--
1.8.2.rc2.4.g7799588
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] config: make parsing stack struct independent from actual data source
On Tue, Mar 12, 2013 at 05:27:35PM +0100, Heiko Voigt wrote: 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. No we do not need the void pointer flexibility. But that means every new source would need to add to this union. Junio complained about that fact when I first added the extra members directly to the struct. A union does not waste that much space and we get some seperation using the union members. Since this struct is local only I think that should be ok. Right. I think that is not a big deal, as we are not exposing this struct outside of the config.c; any additions would need to add a new git_config_from_foo function, anyway. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] teach config parsing to read from strbuf
On Tue, Mar 12, 2013 at 05:42:54PM +0100, Heiko Voigt wrote: 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). Since this is a split of the series mentioned above there are no real callers yet. The main reason for the split was that I wanted to reduce the review burden of one big series into multiple reviews of smaller chunks. If you think it is useful to add the --blob option I can also test from there. It could actually be useful to look at certain .gitmodules options from the submodule script. I am on the fence. I do not want to create more work for you, but I do think it may come in handy, if only for doing submodule things from shell. And it is hopefully not a very large patch. I'd say try it, and if starts looking like it will be very ugly, the right thing may be to leave it until somebody really wants it. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
Linus Torvalds torva...@linux-foundation.org writes: One is simple: git config alias.sync=pull --ff-only Heh, I just wrote that myself after reading the early part of this message ;-) which works fine, but forces submaintainers to be careful when doing things like this, and using a special command to do back-merges. And maybe that's the right thing to do? Back-merges *are* special, Yes. after all. But the above alias is particularly fragile, in that there's both pull and merge that people want to use this for, and it doesn't really handle both. And --ff-only will obviously fail if you actually have some work in your tree, and want to do a real merge, so then you have to do that differently. So I'm mentioning this as a better model than git reset, but not really a *solution*. That said, the fact that --ff-only errors out if you have local development may actually be a big bonus - because you really shouldn't do merges at all if you have local development, but syncing up to my tree if you don't have it (and are going to start it) may be something reasonable. Yes, that's the reasoning behind all the behaviours you described above. Now, the other approach - and perhaps preferable, but requiring actual changes to git itself - is to do the non-fast-forward merge *only* for FETCH_HEAD, which already has magic semantics in other ways. So if somebody does git fetch linus git merge v3.8 to sync with me, they would *not* get a merge commit with a signature, just a fast-forward. But if you do git pull linus v3.8 or a git fetch linus v3.8 git merge FETCH_HEAD it would look like a maintainer merge I am not sure I follow. Are you solving the real problem, the pointeless merge in the security tree that started this thread? I would imagine it was made by somebody thinking that pulling a tagged stable point from you is a good idea, like this: git pull linus v3.9-rc2 which under your FETCH_HEAD rule would look like a maintainer merge, no? An alternative may be to activate the magic mergetag thing only when you give --no-ff explicitly; otherwise merge would unwrap the tag, whether it comes from FETCH_HEAD. The following examples all assume that your HEAD is somewhere behind v3.9-rc2, perhaps done by git checkout -b test v3.8^0 Then under the --no-ff activates the magic rule: git merge v3.9-rc2 will fast-forward, but this git merge --no-ff v3.9-rc2 creates a real merge with the mergetag signature block. The one that caused trouble in the security tree, i.e. git pull linus v3.9-rc2 or its equivalent git fetch linus v3.9-rc2 git merge FETCH_HEAD would still fast-forward under this rule. The maintainer needs to do git pull --no-ff git://git.kernel.org/... for-linus if the pull could fast-forward under this rule, though. Having thought this up to this point, I am not sure it generally is a good change. It feels that pull --ff-only that prevents people from creating pointless back-merges may still be a better mechanism. I dunno. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
Junio C Hamano gits...@pobox.com writes: Then under the --no-ff activates the magic rule: git merge v3.9-rc2 will fast-forward, but this git merge --no-ff v3.9-rc2 creates a real merge with the mergetag signature block. The one that caused trouble in the security tree, i.e. git pull linus v3.9-rc2 or its equivalent git fetch linus v3.9-rc2 git merge FETCH_HEAD would still fast-forward under this rule. The maintainer needs to do git pull --no-ff git://git.kernel.org/... for-linus if the pull could fast-forward under this rule, though. Scratch the last sentence. It should have been whether the pull fast-forwards or not. You'd always need to. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
David Aguilar dav...@gmail.com writes:
Interesting approach. While this does get the intended behavior
for difftool, I'm afraid this would be a grave regression for
existing git diff --raw users who cannot have such behavior.
The 0{40} in RHS of --raw output is to say that we do not know what
object name the contents at the path hashes to.
If you run git diff HEAD^ for a path that is different between
HEAD and the index for which you do not have a local change in the
working tree, we have to show the path (because it is different
between the working tree and HEAD^), but we know the object name for
copy in the working tree, simply because we know it matches what is
in the index. Showing 0{40} on the RHS in such a case loses
information, making us say We don't know when we perfectly well
know. That is a regression.
If the user is allowed to touch any random file in the working tree,
I do not see a workable solution other than John Keeping's follow-up
patch to make symlinks of all paths involved.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
John Keeping j...@keeping.me.uk writes: How about something like --symlink-all where the everything in the right-hand tree is symlink'd? Does it even have to be conditional? What is the situation when you do not want symbolic links? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
Duy Nguyen pclo...@gmail.com writes: glibc's C strncmp version does 4-byte comparison at a time when n =4, then fall back to 1-byte for the rest. I don't know if it's faster than a plain always 1-byte comparison though. There's also the hand written assembly version that compares n from 1..16, not exactly sure how this version works yet. It sounds to me more like a very popular implementation of strcmp/strncmp pair found to have more optimized strncmp than strcmp. While that may be a good reason to justify this patch, I do not think it justifies this: strncmp is provided length information which could be taken advantage by the underlying implementation. After all, strcmp() could also be optimized to fetch word-at-a-time while being careful about not overstepping the page boundary. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
On Tue, Mar 12, 2013 at 01:39:17PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: How about something like --symlink-all where the everything in the right-hand tree is symlink'd? Does it even have to be conditional? What is the situation when you do not want symbolic links? When you're not comparing the working tree. If we can reliably say the RHS is the working tree then it could be unconditional, but I haven't thought about how to do that - I can't see a particularly easy way to check for that; is it sufficient to say there is no more than one non-option to the left of '--' and '--cached' is not among the options? John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
What if we added the ability to do something like this: [remote origin] url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git fetch = +refs/heads/master:refs/heads/master mergeoptions = --ff-only This would be an analog to branch.name.mergeoptions, but it would apply to the source of the pull request, instead of the destination. That way, people who do a git pull from Linus's tree would get the protection of --ff-only, while pulls from submaintainer trees would automatically get a merge commit, which is what we want. It doesn't handle the case of a submaintainer pulling from a maintainer in a back-merge scenario, but that should be a pretty rare case, so maybe that's OK. - Ted -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
John Keeping j...@keeping.me.uk writes:
Does it even have to be conditional? What is the situation when you
do not want symbolic links?
When you're not comparing the working tree.
OK, so what you want is essentially:
* If you see 0{40} in diff --raw, you *know* you are showing the working tree
file on the RHS, and you want to symlink, so that the edit made
by the user will be reflected back to theh working tree copy.
* If your working tree file match what is in the index, you won't
see 0{40} but you still want to symlink, for the same reason.
* If you are comparing two trees, and especially if your RHS is not
HEAD, you will send everything to a temporary without
symlinks. Any edit made by the user will be lost.
If that is the case, perhaps the safest way to go may be to write
the object out when you see non 0{40}, compare it with the working
tree version and then turn that into symlink? That way, you not
only cover the second bullet point, but also cover half of the third
one where the user may find a bug in the RHS, update it in difftool.
I am assuming that you are write-protecting the non-symlink files in
the temporary tree (i.e. those that do not match the working tree)
to prevent users from accidentally modifying something there is no
place to save back to.
Hrm?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
On Tue, Mar 12, 2013 at 2:20 PM, Theodore Ts'o ty...@mit.edu wrote: What if we added the ability to do something like this: [remote origin] url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git fetch = +refs/heads/master:refs/heads/master mergeoptions = --ff-only Hmm. Something like this could be interesting for other things: - use --rebase when pulling (this is common for people who maintain a set of patches and do *not* export their git tree - I use it for projects like git and subsurface where there is an upstream maintainer and I usually send patches by email rather than git) - --no-summary. As a maintainer, you people probably do want to enable summaries for people they pull from, but *not* from upstream. So this might even make sense to do by default when you clone a new repository. - I do think that we might want a --no-signatures for the specific case of merging signed tags without actually taking the signature (because it's a upstream repo). The --ff-only thing is *too* strict. Sometimes you really do want to merge in new code, disallowing it entirely is tough. Of course, I'm not really sure if we want to list the flags. Maybe it's better to just introduce the notion of upstream directly, and make that a flag, and make origin default to that when you clone. And then have git use different heurstics for pulling upstream (like warning by default when doing a back-merge, perhaps?) Linus -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
Theodore Ts'o ty...@mit.edu writes: What if we added the ability to do something like this: [remote origin] url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git fetch = +refs/heads/master:refs/heads/master mergeoptions = --ff-only This would be an analog to branch.name.mergeoptions, but it would apply to the source of the pull request, instead of the destination. That way, people who do a git pull from Linus's tree would get the protection of --ff-only, while pulls from submaintainer trees would automatically get a merge commit, which is what we want. It doesn't handle the case of a submaintainer pulling from a maintainer in a back-merge scenario, but that should be a pretty rare case, so maybe that's OK. Is there an escape hatch for that rare case? IOW, how does a submaintainer who configured the above to override --ff-only? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
On Tue, Mar 12, 2013 at 5:06 PM, John Keeping j...@keeping.me.uk wrote: On Tue, Mar 12, 2013 at 01:39:17PM -0700, Junio C Hamano wrote: What is the situation when you do not want symbolic links? When you're not comparing the working tree. If we can reliably say the RHS is the working tree then it could be unconditional, but I haven't thought about how to do that - I can't see a particularly easy way to check for that; Agreed. From what I can see, the only form of the diff options that compares against the working tree is git difftool -d [--options] commit [--] [path...] At first, I thought that the following cases were also working tree cases, but actually they use the HEAD commit. git difftool -d commit1.. git difftool -d commit1... is it sufficient to say there is no more than one non-option to the left of '--' and '--cached' is not among the options? An alternative approach would be to reuse git-diff's option parsing and make it tell git-difftool when git-diff sees the working tree case. At this point, I haven't seen an obvious place in the source where git-diff makes that choice, but if someone could point me in the right direction, I think I'd actually prefer that approach. What do you think? -- Matt McClure http://www.matthewlmcclure.com http://www.mapmyfitness.com/profile/matthewlmcclure -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
Linus Torvalds torva...@linux-foundation.org writes: - I do think that we might want a --no-signatures for the specific case of merging signed tags without actually taking the signature (because it's a upstream repo). The --ff-only thing is *too* strict. Sometimes you really do want to merge in new code, disallowing it entirely is tough. I agree that --ff-only thing is too strict and sometimes you would want to allow back-merges, but when you do allow such a back-merge, is there a reason you want it to be --no-signatures merge? When a subtree maintainer decides to merge a stable release point from you with a good reason, I do not see anything wrong in recording that the resulting commit _did_ merge what you released with a signature. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
On Tue, Mar 12, 2013 at 2:47 PM, Junio C Hamano gits...@pobox.com wrote: I agree that --ff-only thing is too strict and sometimes you would want to allow back-merges, but when you do allow such a back-merge, is there a reason you want it to be --no-signatures merge? When a subtree maintainer decides to merge a stable release point from you with a good reason, I do not see anything wrong in recording that the resulting commit _did_ merge what you released with a signature. No, there's nothing really bad with adding the signature to the merge commit if you do make a merge. It's the fact that it currently makes a non-ff merge when that is pointless that hurts. That said, adding the signature from an upstream tag doesn't really seem to be hugely useful. I'm not seeing much of an upside, in other words. I'd *expect* that people would pick up upstream tags regardless, no? Linus -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
Linus Torvalds torva...@linux-foundation.org writes: That said, adding the signature from an upstream tag doesn't really seem to be hugely useful. I'm not seeing much of an upside, in other words. I'd *expect* that people would pick up upstream tags regardless, no? Yes, their git fetch will auto-follow, but mergetag embedded in the commit objects will give the history auditable trail the same way as the merges you make your downstream. You of course could match out-of-line tags against back-merges and verify your merges with mergetags, but you do not have to. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
On Tue, Mar 12, 2013 at 5:43 PM, Matt McClure matthewlmccl...@gmail.com wrote:
On Tue, Mar 12, 2013 at 5:06 PM, John Keeping j...@keeping.me.uk wrote:
is it sufficient to say
there is no more than one non-option to the left of '--' and '--cached'
is not among the options?
An alternative approach would be to reuse git-diff's option parsing
and make it tell git-difftool when git-diff sees the working tree
case. At this point, I haven't seen an obvious place in the source
where git-diff makes that choice, but if someone could point me in the
right direction, I think I'd actually prefer that approach. What do
you think?
There's an interesting comment in cmd_diff:
/*
* We could get N tree-ish in the rev.pending_objects list.
* Also there could be M blobs there, and P pathspecs.
*
* N=0, M=0:
* cache vs files (diff-files)
* N=0, M=2:
* compare two random blobs. P must be zero.
* N=0, M=1, P=1:
* compare a blob with a working tree file.
*
* N=1, M=0:
* tree vs cache (diff-index --cached)
*
* N=2, M=0:
* tree vs tree (diff-tree)
*
* N=0, M=0, P=2:
* compare two filesystem entities (aka --no-index).
*
* Other cases are errors.
*/
whereas inspecting rev.pending in the compare against working tree
case, I see:
(gdb) p rev.pending
$3 = {
nr = 1,
alloc = 64,
objects = 0x100807a00
}
(gdb) p *rev.pending.objects
$4 = {
item = 0x100831a48,
name = 0x7fff5fbff8f8 HEAD^,
mode = 12288
}
Given the cases listed in the comment, I assume cmd_diff must
interpret this case as:
* N=1, M=0:
* tree vs cache (diff-index --cached)
The description of that case is confusing or wrong given that
git-diff-index(1) says:
--cached
do not consider the on-disk file at all
***
cmd_diff executes this case:
else if (ents == 1)
result = builtin_diff_index(rev, argc, argv);
So it looks like I could short-circuit in builtin_diff_index or
something it calls -- e.g., run_diff_index -- to get git-diff to tell
git-difftool that it's the working tree case. I see that
run_diff_index does:
diff_set_mnemonic_prefix(revs-diffopt, c/, cached ? i/ : w/);
So that looks like a good place where the code is already deciding
that it's the working tree case -- w/, though surprisingly to me:
(gdb) p revs-diffopt
$12 = {
...
a_prefix = 0x1001c25aa a/,
b_prefix = 0x1001c25ad b/,
...
So diff_set_mnemonic_prefix doesn't actually use the w/ value passed
to it because:
if (!options-b_prefix)
options-b_prefix = b;
Maybe if I could prevent b_prefix from getting set earlier, I could
get some variant of git-diff to emit the w/ for git-difftool.
--
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
Matt McClure matthewlmccl...@gmail.com writes: An alternative approach would be to reuse git-diff's option parsing and make it tell git-difftool when git-diff sees the working tree case. At this point, I haven't seen an obvious place in the source where git-diff makes that choice, but if someone could point me in the right direction, I think I'd actually prefer that approach. What do you think? I do not think you want to go there. That wouldn't solve the third case in my previous message, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/5] blame: introduce $ as end of file in -L syntax
Junio C Hamano gits...@pobox.com writes: Thomas Rast tr...@student.ethz.ch writes: To save the user a lookup of the last line number, introduce $ as a shorthand for the last line. This is mostly useful to spell until the end of the file as '-Lbegin,$'. Doesn't -L begin or -L begin, do that already? If it were to introduce -L $-4, or -L$-4,+2, I would understand why the addition may be useful, but otherwise I do not think it adds much value. It is a quiet-period so there is no need to rush, but did anything happened further on this series? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
On Mar 12, 2013, at 4:16 PM, Junio C Hamano gits...@pobox.com wrote: Matt McClure matthewlmccl...@gmail.com writes: An alternative approach would be to reuse git-diff's option parsing I do not think you want to go there. That wouldn't solve the third case in my previous message, no? I think I don't fully understand your third bullet. * If you are comparing two trees, and especially if your RHS is not HEAD, you will send everything to a temporary without symlinks. Any edit made by the user will be lost. I think you're suggesting to use a symlink any time the content of any given RHS revision is the same as the working tree. I imagine that might confuse me as a user. It would create circumstances where some files are symlinked and others aren't for reasons that won't be straightforward. I imagine solving that case, I might instead implement a copy back to the working tree with conflict detection/resolution. Some earlier iterations of the directory diff feature used copy back without conflict detection and created situations where I clobbered my own changes by finishing a directory diff after making edits concurrently. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: Make directory diff symlink work tree
On Mar 12, 2013, at 1:25 PM, John Keeping j...@keeping.me.uk wrote: When I tried this I got the expected behaviour even without this patch. git diff --raw commit emits the null SHA1 if the working tree file's stat differs from the blob corresponding to commit. Is that the case you observed? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 09/13] exclude: filter out patterns not applicable to the current directory
On Tue, Mar 12, 2013 at 9:04 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: The algorithm implemented here is a naive one. Patterns can be either active or passive: - When we enter a new directory (e.g. from root to foo), currently active patterns may no longer be applicable and can be turned to passive. - On the opposite, when we leave a directory (foo back to roo), s/roo/root/ Also, perhaps you meant s/On/Or/ ? +/* + * If pushed is non-zero, we have entered a new directory. Some + * pathname patterns may no longer applicable. Go over all active s/applicable/be applicable/ -- ES -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
Am 10.03.2013 21:17, schrieb Ramkumar Ramachandra: git operations are slow on repositories with lots of files, and lots of tiny filesystem calls like lstat(), getdents(), open() are reposible for this. On the linux-2.6 repository, for instance, the numbers for git status look like this: top syscalls sorted top syscalls sorted by acc. timeby number -- 0.401906 40950 lstat0.401906 40950 lstat 0.190484 5343 getdents 0.150055 5374 open 0.150055 5374 open 0.190484 5343 getdents 0.074843 2806 close 0.074843 2806 close 0.003216 157 read 0.003216 157 read To solve this problem, we propose to build a daemon which will watch the filesystem using inotify and report batched up events over a UNIX socket. [...] + +The credential C API is meant to be called by Git code which needs +information aboutx filesystem changes. It is centered around an +object representing the changes the filesystem since the last +invocation. + Hmmm...I don't see how filesystem changes since last invocation can solve the problem, or am I missing something? I think what you mean to say is that the daemon should keep track of the filesystem *state* of the working copy, or alternatively the deltas/changes to some known state (such as .git/index)? I'm also still skeptical whether a daemon will improve overall performance. In my understanding its essentially a filesystem cache in user-mode. The difference to using the OS filesystem cache directly (via lstat/readdir) is that we replace ~50k sys-calls with a single IPC call (i.e. the git -- fswatch daemon communication is less 'chatty'). However, the 'chattyness' is still there between the fswatch daemon and the OS / inotify. Consider 'git status; make; make clean; git status'...that's a *lot* of changes to process for nothing (potentially slowing down make). Then there's the issue of stale data in the cache. Modifying porcelain commands that use 'git status --porcelain' to compile their changesets will want 100% exact data. I'm not saying its not doable, but adding another platform specific, caching daemon to the tool chain doesn't exactly simplify things... But perhaps I'm too pessimistic (or just stigmatized by inherently slow and out-of-date TGitCache/TSvnCache on Windows :-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] tag: --force is quiet about new tags
git tag --force is used to replace an existing tag with a new reference. Git helpfully tells the user the old ref when this happens. But if the tag name is new and does not exist, git tells the user the old ref anyway (00). Teach git to ignore --force if the tag is new. Add a test for this and also to ensure --force can replace tags at all. Signed-off-by: Phil Hord ho...@cisco.com --- builtin/tag.c | 2 +- t/t7004-tag.sh | 12 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/builtin/tag.c b/builtin/tag.c index f826688..af3af3f 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -582,7 +582,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) die(_(%s: cannot lock the ref), ref.buf); if (write_ref_sha1(lock, object, NULL) 0) die(_(%s: cannot update the ref), ref.buf); - if (force hashcmp(prev, object)) + if (force !is_null_sha1(prev) hashcmp(prev, object)) printf(_(Updated tag '%s' (was %s)\n), tag, find_unique_abbrev(prev, DEFAULT_ABBREV)); strbuf_release(buf); diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index f5a79b1..c8d6e9f 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -104,6 +104,18 @@ test_expect_success 'creating a tag using HEAD directly should succeed' ' tag_exists myhead ' +test_expect_success '--force can create a tag with the name of one existing' ' + tag_exists mytag + git tag --force mytag + tag_exists mytag' + +test_expect_success '--force is moot with a non-existing tag name' ' + git tag newtag expect + git tag --force forcetag actual + test_cmp expect actual +' +git tag -d newtag forcetag + # deleting tags: test_expect_success 'trying to delete an unknown tag should fail' ' -- 1.8.2.rc3.394.g2617418.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: Make directory diff symlink work tree
On Tue, Mar 12, 2013 at 05:12:28PM -0600, Matt McClure wrote: On Mar 12, 2013, at 1:25 PM, John Keeping j...@keeping.me.uk wrote: When I tried this I got the expected behaviour even without this patch. git diff --raw commit emits the null SHA1 if the working tree file's stat differs from the blob corresponding to commit. Is that the case you observed? Yes, although it's slightly more subtle than that - the null SHA1 only occurs if the working tree file has unstaged changes; if you add the changes to the index then the null SHA1 is no longer used since the blob is now available in Git's object store. John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions/investigations on git-subtree and tags
On Tue, Mar 12, 2013 at 10:02 AM, Jeremy Rosen jeremy.ro...@openwide.fr wrote: 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 Thanks Jérémy. Nothing in your patches are dependant on anything I'm working on. I'll gladly take them for my own tree but feel free to submit them (inline, rather than as attachments) to the list and cc: David Greene gree...@obbligato.org the subtree area maintainer. -- Paul [W] Campbell -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/19] Document pull-all and push-all
On Tue, Mar 12, 2013 at 3:12 PM, Holger Hellmuth (IKS) hellm...@ira.uka.de wrote: Am 09.03.2013 20:28, schrieb Paul Campbell: From 7dcd40ab8687a588b7b0c6ff914a7cfb601b6774 Mon Sep 17 00:00:00 2001 From: Herman van Rink r...@initfour.nl Date: Tue, 27 Mar 2012 13:59:16 +0200 Subject: [PATCH 14/19] Document pull-all and push-all --- contrib/subtree/git-subtree.txt | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt index e0957ee..c8fc103 100644 --- a/contrib/subtree/git-subtree.txt +++ b/contrib/subtree/git-subtree.txt @@ -92,13 +92,19 @@ pull:: Exactly like 'merge', but parallels 'git pull' in that it fetches the given commit from the specified remote repository. - + +pull-all:: + Perform a pull operation on all in .gittrees registered subtrees. + push:: Does a 'split' (see below) using the prefix supplied and then does a 'git push' to push the result to the repository and refspec. This can be used to push your subtree to different branches of the remote repository. +push-all:: + Perform a pull operation on all in .gittrees registered subtrees. - pull-push Thanks. + split:: Extract a new, synthetic project history from the history of the prefix subtree. The new history -- Paul [W] Campbell -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: difftool -d symlinks, under what conditions
On Tue, Mar 12, 2013 at 04:48:16PM -0600, Matt McClure wrote:
On Mar 12, 2013, at 4:16 PM, Junio C Hamano gits...@pobox.com wrote:
Matt McClure matthewlmccl...@gmail.com writes:
* If you are comparing two trees, and especially if your RHS is not
HEAD, you will send everything to a temporary without
symlinks. Any edit made by the user will be lost.
I think you're suggesting to use a symlink any time the content of any
given RHS revision is the same as the working tree.
I imagine that might confuse me as a user. It would create
circumstances where some files are symlinked and others aren't for
reasons that won't be straightforward.
I imagine solving that case, I might instead implement a copy back to
the working tree with conflict detection/resolution. Some earlier
iterations of the directory diff feature used copy back without
conflict detection and created situations where I clobbered my own
changes by finishing a directory diff after making edits concurrently.
The code to copy back working tree files is already there, it just
triggers using the same logic as the creation of symlinks in the first
place and doesn't attempt any conflict detection. I suspect that any
more comprehensive solution will need to restrict the use of git
difftool -d whenever the index contains unmerged entries or when there
are both staged and unstaged changes, since the merge resolution will
cause these states to be lost.
The implementation of Junio's suggestion is relatively straightforward
(this is untested, although t7800 passes, and can probably be improved
by someone better versed in Perl). Does this work for your original
scenario?
-- 8 --
diff --git a/git-difftool.perl b/git-difftool.perl
index 0a90de4..5f093ae 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,21 @@ sub exit_cleanup
exit($status | ($status 8));
}
+sub use_wt_file
+{
+ my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+ my $null_sha1 = '0' x 40;
+
+ if ($sha1 eq $null_sha1) {
+ return 1;
+ } elsif (not $symlinks) {
+ return 0;
+ }
+
+ my $wt_sha1 = $repo-command_oneline('hash-object', $workdir/$file);
+ return $sha1 eq $wt_sha1;
+}
+
sub setup_dir_diff
{
my ($repo, $workdir, $symlinks) = @_;
@@ -159,10 +174,10 @@ EOF
}
if ($rmode ne $null_mode) {
- if ($rsha1 ne $null_sha1) {
- $rindex .= $rmode $rsha1\t$dst_path\0;
- } else {
+ if (use_wt_file($repo, $workdir, $dst_path, $rsha1,
$symlinks)) {
push(@working_tree, $dst_path);
+ } else {
+ $rindex .= $rmode $rsha1\t$dst_path\0;
}
}
}
--
1.8.2.rc2.4.g7799588
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: Make directory diff symlink work tree
On Tue, Mar 12, 2013 at 3:24 PM, John Keeping j...@keeping.me.uk wrote: When I tried this I got the expected behaviour even without this patch. It turns out that an uncommitted, but *staged* change emits the SHA1 of the blob rather than the null SHA1. Do you get the desired behaviour if you git reset before using difftool? I tried this: $ git diff --raw HEAD^ :100644 100644 f35de0f... ead9399... M diff-lib.c $ git reset HEAD^ Unstaged changes after reset: M diff-lib.c $ git diff --raw :100644 100644 f35de0f... 000... M diff-lib.c $ git difftool -d and the last command did indeed create symlinks into my working tree rather than file copies. So... it seems that using git-reset is at least a workaround to get the symlink behavior I want as a user, though the dance I have to do is a little more awkward than `git difftool -d HEAD^` would be. If so I think you want some new mode of operation for difftool instead of this patch which will also affect unrelated commands. Are you suggesting that difftool do the reset work above given a new option or by default? -- Matt McClure http://www.matthewlmcclure.com http://www.mapmyfitness.com/profile/matthewlmcclure -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
On Wed, Mar 13, 2013 at 6:21 AM, Karsten Blees karsten.bl...@gmail.com wrote: Hmmm...I don't see how filesystem changes since last invocation can solve the problem, or am I missing something? I think what you mean to say is that the daemon should keep track of the filesystem *state* of the working copy, or alternatively the deltas/changes to some known state (such as .git/index)? I think git process can keep track of filesystem state (and save it down if necessary). But when git process is not running, system state changes and it cannot know about. The daemon helps filling this gap (and basically keeps git running (in a light form) throughout a development session). For example if we know only 5 files have changed since the last refresh, we only need to re-stat those 5. The same for untracked/ignored file checking, I'm also still skeptical whether a daemon will improve overall performance. In my understanding its essentially a filesystem cache in user-mode. The difference to using the OS filesystem cache directly (via lstat/readdir) is that we replace ~50k sys-calls with a single IPC call (i.e. the git -- fswatch daemon communication is less 'chatty'). However, the 'chattyness' is still there between the fswatch daemon and the OS / inotify. I think it attempts to reduce unnecessary system calls, not eliminate them all. In the 5 changed files above, a few IPC calls are done to retrieve the file list, then 5 lstat will be issued (by git, not the daemon) instead of thousands of them. Consider 'git status; make; make clean; git status'...that's a *lot* of changes to process for nothing (potentially slowing down make). Yeah. In my opinion, the daemon should realize that at some point accumulated changes are too much that it's not worth collecting anymore, and drop them all. Git will do it the normal/slow way. After that the daemon picks up again. We only optimize for the case when little changes are made in filesystem. Then there's the issue of stale data in the cache. Modifying porcelain commands that use 'git status --porcelain' to compile their changesets will want 100% exact data. I'm not saying its not doable, but adding another platform specific, caching daemon to the tool chain doesn't exactly simplify things... But perhaps I'm too pessimistic (or just stigmatized by inherently slow and out-of-date TGitCache/TSvnCache on Windows :-) Thanks. I didn't know about TGitCache. Will dig it up. Maybe we can learn something from it (or realize the daemon approach is futile after all). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/13] match_basename: use strncmp instead of strcmp
On Wed, Mar 13, 2013 at 12:40 AM, Antoine Pelisse apeli...@gmail.com wrote:
--- 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;
I can see that you kept strncmp(), was it worse with memcmp() ?
One step at a time. 05/13 replace strncmp_icase with memcmp (for when
ignore_case == 0).
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] match_basename: use strncmp instead of strcmp
On Wed, Mar 13, 2013 at 3:59 AM, Junio C Hamano gits...@pobox.com wrote: strncmp is provided length information which could be taken advantage by the underlying implementation. After all, strcmp() could also be optimized to fetch word-at-a-time while being careful about not overstepping the page boundary. It boils down to giving more information to the underlying implementation with hope that it can be useful for something. Although at this point I think strncmp vs strcmp vs memcmp may be not worth doing (we keep explicit length comparison to reduce *cmp calls though). The gain is relatively small and will become even smaller after we avoid running exclude on tracked files (which eliminates like 2/3 of the processed entries). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/3] mergetools/p4merge: create a base if none available
Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:
p4merge $LOCAL $LOCAL $REMOTE $MERGED
Commit 0a0ec7bd changed this to:
p4merge empty file $LOCAL $REMOTE $MERGED
to avoid the problem of being unable to save in some circumstances with
similar inputs.
Unfortunately this approach produces much worse results on differing
inputs. P4Merge really regards the blank file as the base, and once you
have just a couple of differences between the two branches you end up
with one a massive full-file conflict. The 3-way diff is not readable,
and you have to invoke difftool MERGE_HEAD HEAD manually to get a
useful view.
The original approach appears to have invoked special 2-way merge
behaviour in P4Merge that occurs only if the base filename is or
equal to the left input. You get a good visual comparison, and it does
not auto-resolve differences. (Normally if one branch matched the base,
it would autoresolve to the other branch).
But there appears to be no way of getting this 2-way behaviour and being
able to reliably save. Having base==left appears to be triggering other
assumptions. There are tricks the user can use to force the save icon
on, but it's not intuitive.
So we now follow a suggestion given in the original patch's discussion:
generate a virtual base, consisting of the lines common to the two
branches. This is the same as the technique used in resolve and octopus
merges, so we relocate that code to a shared function.
Note that if there are no differences at the same location, this
technique can lead to automatic resolution without conflict, combining
everything from the 2 files. As with the other merges using this
technique, we assume the user will inspect the result before saving.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
Documentation/git-sh-setup.txt | 6 ++
git-merge-one-file.sh | 18 +-
git-sh-setup.sh| 12
mergetools/p4merge | 6 +-
4 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 6a9f66d..5d709d0 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -82,6 +82,12 @@ get_author_ident_from_commit::
outputs code for use with eval to set the GIT_AUTHOR_NAME,
GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE variables for a given commit.
+create_virtual_base::
+ modifies the first file so only lines in common with the
+ second file remain. If there is insufficient common material,
+ then the first file is left empty. The result is suitable
+ as a virtual base input for a 3-way merge.
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index f612cb8..0f164e5 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,30 +104,22 @@ case ${1:-.}${2:-.}${3:-.} in
;;
esac
- src2=`git-unpack-file $3`
+ src1=$(git-unpack-file $2)
+ src2=$(git-unpack-file $3)
case $1 in
'')
echo Added $4 in both, but differently.
- # This extracts OUR file in $orig, and uses git apply to
- # remove lines that are unique to ours.
- orig=`git-unpack-file $2`
- sz0=`wc -c $orig`
- @@DIFF@@ -u -La/$orig -Lb/$orig $orig $src2 | git apply --no-add
- sz1=`wc -c $orig`
-
- # If we do not have enough common material, it is not
- # worth trying two-file merge using common subsections.
- expr $sz0 \ $sz1 \* 2 /dev/null || : $orig
+ orig=$(git-unpack-file $2)
+ create_virtual_base $orig $src2
;;
*)
echo Auto-merging $4
- orig=`git-unpack-file $1`
+ orig=$(git-unpack-file $1)
;;
esac
# Be careful for funny filename such as -L in $4, which
# would confuse merge greatly.
- src1=`git-unpack-file $2`
git merge-file $src1 $orig $src2
ret=$?
msg=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 795edd2..349a5d4 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -249,6 +249,18 @@ clear_local_git_env() {
unset $(git rev-parse --local-env-vars)
}
+# Generate a virtual base file for a two-file merge. Uses git apply to
+# remove lines from $1 that are not in $2, leaving only common lines.
+create_virtual_base() {
+ sz0=$(wc -c $1)
+ @@DIFF@@ -u -La/$1 -Lb/$1 $1 $2 | git apply --no-add
+ sz1=$(wc -c $1)
+
+ # If we do not have enough common material, it is not
+ # worth trying two-file merge using common subsections.
+ expr $sz0 \ $sz1 \* 2 /dev/null || : $1
+}
+
# Platform specific tweaks to work around some commands
case $(uname -s) in
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 46b3a5a..5a608ab
[PATCH v3 3/3] git-merge-one-file: revise merge error reporting
Commit 718135e improved the merge error reporting for the resolve
strategy's merge conflict and permission conflict cases, but led to a
malformed ERROR: in myfile.c message in the case of a file added
differently.
This commit reverts that change, and uses an alternative approach without
this flaw.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
git-merge-one-file.sh | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 0f164e5..78b07a8 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,11 +104,13 @@ case ${1:-.}${2:-.}${3:-.} in
;;
esac
+ ret=0
src1=$(git-unpack-file $2)
src2=$(git-unpack-file $3)
case $1 in
'')
- echo Added $4 in both, but differently.
+ echo ERROR: Added $4 in both, but differently.
+ ret=1
orig=$(git-unpack-file $2)
create_virtual_base $orig $src2
;;
@@ -121,10 +123,9 @@ case ${1:-.}${2:-.}${3:-.} in
# Be careful for funny filename such as -L in $4, which
# would confuse merge greatly.
git merge-file $src1 $orig $src2
- ret=$?
- msg=
- if [ $ret -ne 0 ]; then
- msg='content conflict'
+ if [ $? -ne 0 ]; then
+ echo ERROR: Content conflict in $4
+ ret=1
fi
# Create the working tree file, using our tree version from the
@@ -133,18 +134,11 @@ case ${1:-.}${2:-.}${3:-.} in
rm -f -- $orig $src1 $src2
if [ $6 != $7 ]; then
- if [ -n $msg ]; then
- msg=$msg,
- fi
- msg=${msg}permissions conflict: $5-$6,$7
- ret=1
- fi
- if [ $1 = '' ]; then
+ echo ERROR: Permissions conflict: $5-$6,$7
ret=1
fi
if [ $ret -ne 0 ]; then
- echo ERROR: $msg in $4
exit 1
fi
exec git update-index -- $4
--
1.8.2.rc3.7.g1100d09.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 05/13] match_{base,path}name: replace strncmp_icase with memequal_icase
On Tue, Mar 12, 2013 at 8:04 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
+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);
+}
Note, the ignore_case == 1 codepath was not tested and measured. I
suspect that strncasecmp may be more complex to support locales and
the LANG=C version should suffice in most case. But it's just
guesses at this moment.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE
Reverse LOCAL and REMOTE when invoking P4Merge as a mergetool, so that
the incoming branch is now in the left-hand, blue triangle pane, and the
current branch is in the right-hand, green circle pane.
This change makes use of P4Merge consistent with its built-in help, its
reference documentation, and Perforce itself. But most importantly, it
makes merge results clearer. P4Merge is not totally symmetrical between
left and right; despite changing a few text labels from theirs/ours to
left/right when invoked manually, it still retains its original
Perforce theirs/ours viewpoint.
Most obviously, in the result pane P4Merge shows changes that are common
to both branches in green. This is on the basis of the current branch
being green, as it is when invoked from Perforce; it means that lines in
the result are blue if and only if they are being changed by the merge,
making the resulting diff clearer.
Note that P4Merge now shows ours on the right for both diff and merge,
unlike other diff/mergetools, which always have REMOTE on the right.
But observe that REMOTE is the working tree (ie ours) for a diff,
while it's another branch (ie theirs) for a merge.
Ours and theirs are reversed for a rebase - see git help rebase.
However, this does produce the desired show the results of this commit
effect in P4Merge - changes that remain in the rebased commit (in your
branch, but not in the new base) appear in blue; changes that do not
appear in the rebased commit (from the new base, or common to both) are
in green. If Perforce had rebase, they'd probably not swap ours/theirs,
but make P4Merge show common changes in blue, picking out our changes in
green. We can't do that, so this is next best.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
mergetools/p4merge | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 8a36916..46b3a5a 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -22,7 +22,7 @@ diff_cmd () {
merge_cmd () {
touch $BACKUP
$base_present || $BASE
- $merge_tool_path $BASE $LOCAL $REMOTE $MERGED
+ $merge_tool_path $BASE $REMOTE $LOCAL $MERGED
check_unchanged
}
--
1.8.2.rc3.7.g1100d09.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Nike Free Damen
Der müde Nike Free Run 4.0 http://www.nikefreeoutletkaufen.org/ Nacht ohne Lanna, wie Bohnen Randeng. Smoke verschwommenes Monaten Yu Xuefei micro. Ich stand Xianting Central, Rauch Januar weiß gefärbt frost sehen, hören den Schnee von der Schulter, die Brise in eine Blume Blüte im Ohr, wie ein Hochhaus schlanke Mond singen, verweilen um die Ohren, an einem seidenen Faden. Angemeldet Freizeit Hochwasser, haben eine verstümmelte antike Hand Punkte gefunden, die Raum und Zeit öffnen Sie die Faser die Augenbraue Jincu, wie Tuosai betrachten Toggle Wellen meines Herzens, aus meiner Welt, die du Nieselregen Birne wie die Melancholie. Schnee, ich heiß auf dem alten Hof Krug Wein, der die flache einsamen anhaltenden Eingang, Seele-Rühren. Flipping Seiten, das Herz wird aneinanderreihen Sea Scrolls Text des Geschmacks gebunden, riechen, hören das Wort des alten Nike Free Outlet http://www.nikefreeoutletkaufen.org/ verletzten wenig Tinte v. geträumt Frau sieht aus wie eine absolute Schönheit, konnte die Stimmung nicht gleichgültig bleiben. Seitdem ist mein Herz mit dem der Emei wie deine Mutter Organisation Erstarrung, und dann ein paar Tage warten, die Freude an der Blüte wird es schmelzen, am blühenden Yidao Pflaume. Ich Weibi Augen, atemlos, das Leid Ihre Aussicht hat weit bis zur Abenddämmerung die Haut, dass die Flammen der unruhigen Zeiten zu verbergen, hält sich der Wind und regen immer im Frühjahr und Herbst Tag Samsara. Ich klappte das Buch, aus dem Körper aus dem Dachboden, kam zu dem zentralen Innenhof. Sky dunkel, kalt und windig, auch eine brennende Lampe auf dem Dachboden, Dengying wiegenden persönlichen schrumpfen. Ich hob meine Augen blickte zum Himmel auf, fehlt Ihnen das lächelnde Gesicht hing in der Moon Palace, fliegenden Schnee wenigen Strichen kurze Beschreibung für deine Augenbraue, Pflaume blühen Jiduo das Lächeln Innenhof. Ich trinke einen Wein in der Hand, wärmen Magen up, feine Stifte Welle, ein paar Zeilen von Tränen Sätzen an Nike Free Run Outlet http://www.nikefreeoutletkaufen.org/ der Wand. Am nächsten Tag, ich muss zurück zu kämpfen gelernt haben, ist auch vertraut mit dem Wort. Tolerieren den Geist Anteil spukt emotional, wenn Pinsel ein paar Striche, wird es als ein Sakrileg, eine Respektlosigkeit. Oft sitzen auf dem Dachboden, um den Klang des Frühlings-und Herbst-drifting snow Zala Schultern Geschmack im Süden, das Schmelzen der klassische Seele des Teeaufgusses Tee olfaktorischen Hof Mui Geschmack zu hören, setzte eine alte Ballade. Lesen Sie die alten Bücher der Zeit, zu wissen, dass Sie lachen, lachen Hong Rumei offen, der Klang des Lachens, wie Bäche, wie Klavier spielen den Klang des Lachens haben. Ich kenne einen Mann, der eine lächerliche Art und Weise zum Lachen zu denken. Er zündete sich eine Bake auf der Nike Free günstig http://www.nikefreeoutletkaufen.org/ Rauchsignale, eine peinliche Figur angezogen Dysfunktion. Als Ergebnis erhalten Sie lachen, er verlor, Geschichte hinterlassen. Für Ihr Lächeln, er die ganze Welt verloren, Geschichte Bücher, so dass nur ein paar Striche, so dass niemand kennt die Wahrheit dieser Geschichte. Smoke Monat Stille, keine Spuren von Wasser. Ich gehe in der Welt, hat weg gewesen, und schaute nicht zurück. So bin ich in eurer Welt zu Fuß zu weit, so dass ich tief unfähig, sich zu befreien, oft in eurer Welt, fügte sich in meinen Worten. Nach einer langen Zeit, werden Sie vorbei Figur, wenn auch nicht vergessen, und ich kann mir vorstellen, dass Sie Allure Lächeln. Worden bedanken Xuela Nacht. Eine dünne Kleidung Männer trinken ein paar Schlucke Wein, spielen ein paar Hände von Studenten verbringen Stifte, sind ihre Geschichten noch in den Nike Free Run 5.0 http://www.nikefreeoutletkaufen.org/ Schnee Welt stecken. - Nike Free Damen Nike Free Run Nike Free 4.0 Nike Free Run günstig Nike Free Run 2 -- View this message in context: http://git.661346.n2.nabble.com/Nike-Free-Damen-tp7579562.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE
On Tue, Mar 12, 2013 at 6:12 PM, Kevin Bracey ke...@bracey.fi wrote:
Reverse LOCAL and REMOTE when invoking P4Merge as a mergetool, so that
the incoming branch is now in the left-hand, blue triangle pane, and the
current branch is in the right-hand, green circle pane.
This change makes use of P4Merge consistent with its built-in help, its
reference documentation, and Perforce itself. But most importantly, it
makes merge results clearer. P4Merge is not totally symmetrical between
left and right; despite changing a few text labels from theirs/ours to
left/right when invoked manually, it still retains its original
Perforce theirs/ours viewpoint.
Most obviously, in the result pane P4Merge shows changes that are common
to both branches in green. This is on the basis of the current branch
being green, as it is when invoked from Perforce; it means that lines in
the result are blue if and only if they are being changed by the merge,
making the resulting diff clearer.
Note that P4Merge now shows ours on the right for both diff and merge,
unlike other diff/mergetools, which always have REMOTE on the right.
But observe that REMOTE is the working tree (ie ours) for a diff,
while it's another branch (ie theirs) for a merge.
Ours and theirs are reversed for a rebase - see git help rebase.
However, this does produce the desired show the results of this commit
effect in P4Merge - changes that remain in the rebased commit (in your
branch, but not in the new base) appear in blue; changes that do not
appear in the rebased commit (from the new base, or common to both) are
in green. If Perforce had rebase, they'd probably not swap ours/theirs,
but make P4Merge show common changes in blue, picking out our changes in
green. We can't do that, so this is next best.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
This seems sensible to apply. The commit message is a bit long,
but I think it's justified since this is exactly the kind of thing
I would tend to forget after enough time has passed.
Ditto on the create_virtual_base patch. Your latest patch
addressed Junio's note about making it take 2 args.
FWIW, please feel free to add:
Reviewed-by: David Aguilar dav...@gmail.com
Thanks.
mergetools/p4merge | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 8a36916..46b3a5a 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -22,7 +22,7 @@ diff_cmd () {
merge_cmd () {
touch $BACKUP
$base_present || $BASE
- $merge_tool_path $BASE $LOCAL $REMOTE $MERGED
+ $merge_tool_path $BASE $REMOTE $LOCAL $MERGED
check_unchanged
}
--
1.8.2.rc3.7.g1100d09.dirty
--
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
On Tue, Mar 12, 2013 at 02:30:04PM -0700, Junio C Hamano wrote: Theodore Ts'o ty...@mit.edu writes: [remote origin] url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git fetch = +refs/heads/master:refs/heads/master mergeoptions = --ff-only Is there an escape hatch for that rare case? IOW, how does a submaintainer who configured the above to override --ff-only? Hmm, maybe we would need to add a --no-ff-only? Or they could just do: git fetch origin git merge FETCH_HEAD On Tue, Mar 12, 2013 at 02:28:39PM -0700, Linus Torvalds wrote: Of course, I'm not really sure if we want to list the flags. Maybe it's better to just introduce the notion of upstream directly, and make that a flag, and make origin default to that when you clone. And then have git use different heurstics for pulling upstream (like warning by default when doing a back-merge, perhaps?) What if git automaticallly set up the origin branch to have a certain set of mergeoptions by default? That would probably be right for most users, but it makes it obvious what's going on when they take a look at the .git/config file, and doesn't make the remote that happens to have the name origin as having certain magic properties. Using a set of mergeoptions would also be bit more general, and might have applications in the future. - Ted -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: unneeded merge in the security tree
Theodore Ts'o ty...@mit.edu writes: On Tue, Mar 12, 2013 at 02:30:04PM -0700, Junio C Hamano wrote: Theodore Ts'o ty...@mit.edu writes: [remote origin] url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git fetch = +refs/heads/master:refs/heads/master mergeoptions = --ff-only Is there an escape hatch for that rare case? IOW, how does a submaintainer who configured the above to override --ff-only? Hmm, maybe we would need to add a --no-ff-only? Or they could just do: git fetch origin git merge FETCH_HEAD Hmm, neither feel quite nice. I haven't heard any comments on my alternative proposal, by the way. Did the message get lost? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: --force is quiet about new tags
Phil Hord ho...@cisco.com writes: git tag --force is used to replace an existing tag with a new reference. Git helpfully tells the user the old ref when this happens. But if the tag name is new and does not exist, git tells the user the old ref anyway (00). Teach git to ignore --force if the tag is new. Add a test for this and also to ensure --force can replace tags at all. Signed-off-by: Phil Hord ho...@cisco.com --- I think we would still want to allow the operation to go through, even when the --force option is given, to create a new tag. I agree that the message should not say Updated. So teaching Git not to issue the Updated message makes perfect sense. It is somewhat misleading to say we are teaching Git to ignore the option, though. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] require pathspec for git add -u/-A
On Tue, Mar 12, 2013 at 02:58:44PM +0100, Matthieu Moy wrote: 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? Somehow, but we may find a way to do so, as long as it temporary (i.e. something that will have no effect after the transition period), and that is is crystal clear that it's temporary. Yeah, I think this is easy as long as it is enable the new behavior now and not toggle between new and old behavior. That is, a boolean rather than a selector, with a note that it will go away at the behavior switch. The only downside I see is that a user may switch it on now, saying Yes, I understand and prefer the new behavior, but some script they run might not expect it. We can warn against that in the documentation, but that may or may not be enough. Here's a series which does that; if it's the direction we want to go, I think we'd want to rebase Junio's now add -u is full-tree patch on top. [1/2]: t2200: check that add -u limits itself to subdirectory [2/2]: add: respect add.updateroot config option -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t2200: check that add -u limits itself to subdirectory
This behavior is due to change in the future, but let's test it anyway. That helps make sure we do not accidentally switch the behavior too soon while we are working in the area, and it means that we can easily verify the change when we do make it. Signed-off-by: Jeff King p...@peff.net --- We didn't seem to be testing this transition at all. I think it's sane to do so now, and Junio's now it is 2.0, let's switch patch should update the test. t/t2200-add-update.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index 4cdebda..fe4f548 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -80,6 +80,17 @@ test_expect_success 'change gets noticed' ' ' +# Note that this is scheduled to change in Git 2.0, when +# git add -u will become full-tree by default. +test_expect_success 'update did not touch files at root' ' + cat expect -\EOF + check + top + EOF + git diff-files --name-only actual + test_cmp expect actual +' + test_expect_success SYMLINKS 'replace a file with a symlink' ' rm foo -- 1.8.2.rc2.7.gef06216 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] add: respect add.updateroot config option
The `git add -u` command operates on the current subdir of
the repository, but is transitioning to operate on the whole
repository (see commit 0fa2eb5 for details). During the
transition period, we issue a warning. For users who have
read and accepted the warning, there is no way to jump
directly to the future behavior and silence the warning. The
config option added by this patch gives them such an option.
Signed-off-by: Jeff King p...@peff.net
---
Documentation/config.txt | 19 +++
builtin/add.c| 10 --
t/t2200-add-update.sh| 14 ++
3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bbba728..be0f6fc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -657,6 +657,25 @@ add.ignoreErrors::
convention for configuration variables. Newer versions of Git
honor `add.ignoreErrors` as well.
+add.updateroot::
+ Historically, when the `git add -u` and `git add -A` commands
+ are run from a subdirectory of the repository and are not given
+ any pathspec, they have operated only on the subdirectory from
+ which they were called. In a future version of git, this behavior
+ will be switched to operate on the full repository instead. In
+ the meantime, issuing `git add -u` (or `-A`) from a subdirectory
+ continues to work on the subdirectory, but will now issue a
+ warning. If you are ready to move to the new behavior now,
+ accepting that you may be breaking existing scripts which expect
+ the old behavior, you can do so by setting `add.updateroot` to
+ `true`.
++
+Note that this is meant to let early adopters move to the new behavior
+immediately; it is not a toggle switch that will work forever. After
+git transitions to the new behavior, this option will become a no-op;
+`git add -u` will always update the whole tree, and `git add -u .` will
+remain the correct way to limit to the current directory.
+
alias.*::
Command aliases for the linkgit:git[1] command wrapper - e.g.
after defining alias.last = cat-file commit HEAD, the invocation
diff --git a/builtin/add.c b/builtin/add.c
index ab1c9e8..95fe35e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -22,6 +22,7 @@ static int take_worktree_changes;
};
static int patch_interactive, add_interactive, edit_interactive;
static int take_worktree_changes;
+static int update_root;
struct update_callback_data {
int flags;
@@ -297,6 +298,10 @@ static int add_config(const char *var, const char *value,
void *cb)
ignore_add_errors = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, add.updateroot)) {
+ update_root = git_config_bool(var, value);
+ return 0;
+ }
return git_default_config(var, value, cb);
}
@@ -390,12 +395,13 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
short_option_with_implicit_dot = -u;
}
if (option_with_implicit_dot !argc) {
+ static const char *root[2] = { :/, NULL };
static const char *here[2] = { ., NULL };
- if (prefix)
+ if (!update_root prefix)
warn_pathless_add(option_with_implicit_dot,
short_option_with_implicit_dot);
argc = 1;
- argv = here;
+ argv = update_root ? root : here;
}
add_new_files = !take_worktree_changes !refresh_only;
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index fe4f548..b9215d3 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -91,6 +91,20 @@ test_expect_success 'update did not touch files at root' '
test_cmp expect actual
'
+test_expect_success 'update with add.updateroot set' '
+ (
+ cd dir1
+ echo more sub2
+ git -c add.updateroot=true add -u
+ )
+'
+
+test_expect_success 'all paths are updated' '
+ expect
+ git diff-files --name-status actual
+ test_cmp expect actual
+'
+
test_expect_success SYMLINKS 'replace a file with a symlink' '
rm foo
--
1.8.2.rc2.7.gef06216
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: --force is quiet about new tags
On Tue, Mar 12, 2013 at 11:33 PM, Junio C Hamano gits...@pobox.com wrote: Phil Hord ho...@cisco.com writes: git tag --force is used to replace an existing tag with a new reference. Git helpfully tells the user the old ref when this happens. But if the tag name is new and does not exist, git tells the user the old ref anyway (00). Teach git to ignore --force if the tag is new. Add a test for this and also to ensure --force can replace tags at all. Signed-off-by: Phil Hord ho...@cisco.com --- I think we would still want to allow the operation to go through, even when the --force option is given, to create a new tag. I agree that the message should not say Updated. So teaching Git not to issue the Updated message makes perfect sense. It is somewhat misleading to say we are teaching Git to ignore the option, though. Thanks. My phrasing was too ambiguous. What you described is exactly what the patch does. --force is superfluous when the tag does not already exist. It is only checked in two places, and one of those is to decide whether to print the Updated message. How's this? Teach 'git tag --force' to suppress the update message if the tag is new. Add a test for this and also to ensure --force can replace tags at all. Phil -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
