Git development
 help / color / mirror / Atom feed
* Re: [PATCH 3/3] match_basename: use strncmp instead of strcmp
From: Fredrik Gustafsson @ 2013-03-09  8:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7v4nglf1w3.fsf@alter.siamese.dyndns.org>

On Fri, Mar 08, 2013 at 11:50:04PM -0800, Junio C Hamano wrote:
> At the same time, I wonder if we can take advantage of the fact that
> these call sites only care about equality and not ordering.

I did an RFC-patch for that (that I mistakenly didn't sent as a reply to
this e-mail). And I believe that you're correct. My solution is inspired
of curl's strequal.

Is the reason for git not to care about lower/upper-case for beeing able
to support windows? Or is there any other smart reason?

I was also thinking about discarding files by looking at their
modification date. If the modification timestamp is older than/or equal to
the latest commit, there's probably no reason for examine that file any
further. I'm not sure about the side effects this may imply though. I
think they can be quite nasty. Is this something worth digging more in
or am I already on the wrong path?

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply

* Re: [PATCH] Replace strcmp_icase with strequal_icase
From: Fredrik Gustafsson @ 2013-03-09  8:57 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git, pclouds
In-Reply-To: <1362818574-16873-1-git-send-email-iveqy@iveqy.com>

Please ignore last e-mail. Sorry for the disturbance.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply

* Re: [PATCH 1/3] match_pathname: avoid calling strncmp if baselen is 0
From: Antoine Pelisse @ 2013-03-09  9:06 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1362802190-7331-2-git-send-email-pclouds@gmail.com>

> diff --git a/dir.c b/dir.c
> index 57394e4..669cf80 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -663,7 +663,7 @@ int match_pathname(const char *pathname, int pathlen,
>          */
>         if (pathlen < baselen + 1 ||
>             (baselen && pathname[baselen] != '/') ||
> -           strncmp_icase(pathname, base, baselen))
> +           (baselen && strncmp_icase(pathname, base, baselen)))

Shouldn't you factorize by baselen here ? For readability reasons, not
performance of course.

>                 return 0;
>
>         namelen = baselen ? pathlen - baselen - 1 : pathlen;

^ permalink raw reply

* Re: [PATCH 3/3] match_basename: use strncmp instead of strcmp
From: Duy Nguyen @ 2013-03-09  9:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nglf1w3.fsf@alter.siamese.dyndns.org>

On Sat, Mar 9, 2013 at 2:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> 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.
>
> I think I am a bit slower than my usual self tonight, but I am
> utterly confused by the above.
>
> strncmp() compares _only_ up to the first n bytes, so when you are
> using it for equality, it is not "we could check length", but is "we
> MUST check they match to the length of the shorter string", if you
> want to obtain not just faster but correct result.
>
> Am I mistaken?

Yeap, the description is a bit misleading. Although you could get away
with length check by doing !strncmp(a, b, strlen(a)+1).

> Even if you are using strcmp() that yields ordering not just
> equality, it can return a correct result as soon as it hits the
> first bytes that are different; I doubt using strncmp() contributes
> to the performance very much.  Comparing lengths before doing
> byte-for-byte comparison could help because you can reject two
> strings with different lengths without looking at them.
>
> At the same time, I wonder if we can take advantage of the fact that
> these call sites only care about equality and not ordering.

I tried to push it further and compare hash before do the actual
string comparison. It slowed things down (hopefully because the cost
of hashing, the same one from name-hash.c, not because I did it
wrong).
-- 
Duy

^ permalink raw reply

* Re: [PATCH] Replace strcmp_icase with strequal_icase
From: Duy Nguyen @ 2013-03-09 10:21 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git
In-Reply-To: <1362818574-16873-1-git-send-email-iveqy@iveqy.com>

On Sat, Mar 09, 2013 at 09:42:54AM +0100, Fredrik Gustafsson wrote:
> To improve performance.
> git status before:
> user    0m0.020s
> user    0m0.024s
> user    0m0.024s
> user    0m0.020s
> user    0m0.024s
> user    0m0.028s
> user    0m0.024s
> user    0m0.024s
> user    0m0.016s
> user    0m0.028s
> 
> git status after:
> user    0m0.012s
> user    0m0.008s
> user    0m0.008s
> user    0m0.008s
> user    0m0.008s
> user    0m0.008s
> user    0m0.008s
> user    0m0.004s
> user    0m0.008s
> user    0m0.016s

I tested a slightly different version that checks ignore_case, inlines
if possible and replaces one more strncmp_icase call site (the top
call site in webkit.git). The numbers are impressive (well not as
impressive as yours, but I guess it depends on the actual .gitignore
patterns). On top of my 3/3

        before      after
user    0m0.508s    0m0.392s
user    0m0.511s    0m0.394s
user    0m0.513s    0m0.405s
user    0m0.516s    0m0.407s
user    0m0.516s    0m0.407s
user    0m0.518s    0m0.410s
user    0m0.519s    0m0.412s
user    0m0.524s    0m0.415s
user    0m0.527s    0m0.415s
user    0m0.534s    0m0.417s

I still need to run the test suite. Then maybe reroll my series with
this.

-- 8< --
diff --git a/dir.c b/dir.c
index 2a91d14..6a9b4b7 100644
--- a/dir.c
+++ b/dir.c
@@ -21,6 +21,24 @@ 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 strnequal_icase(const char *first, const char *second, int length)
+{
+	if (ignore_case) {
+		while (length && toupper(*first) == toupper(*second)) {
+			first++;
+			second++;
+			length--;
+		}
+	} else {
+		while (length && *first == *second) {
+			first++;
+			second++;
+			length--;
+		}
+	}
+	return length == 0;
+}
+
 inline int git_fnmatch(const char *pattern, const char *string,
 		       int flags, int prefix)
 {
@@ -611,11 +629,11 @@ int match_basename(const char *basename, int basenamelen,
 {
 	if (prefix == patternlen) {
 		if (patternlen == basenamelen &&
-		    !strncmp_icase(pattern, basename, patternlen))
+		    strnequal_icase(pattern, basename, patternlen))
 			return 1;
 	} else if (flags & EXC_FLAG_ENDSWITH) {
 		if (patternlen - 1 <= basenamelen &&
-		    !strncmp_icase(pattern + 1,
+		    strnequal_icase(pattern + 1,
 				   basename + basenamelen - patternlen + 1,
 				   patternlen - 1))
 			return 1;
@@ -649,7 +667,7 @@ int match_pathname(const char *pathname, int pathlen,
 	 */
 	if (pathlen < baselen + 1 ||
 	    (baselen && pathname[baselen] != '/') ||
-	    (baselen && strncmp_icase(pathname, base, baselen)))
+	    (baselen && !strnequal_icase(pathname, base, baselen)))
 		return 0;
 
 	namelen = baselen ? pathlen - baselen - 1 : pathlen;
@@ -663,7 +681,7 @@ int match_pathname(const char *pathname, int pathlen,
 		if (prefix > namelen)
 			return 0;
 
-		if (strncmp_icase(pattern, name, prefix))
+		if (!strnequal_icase(pattern, name, prefix))
 			return 0;
 		pattern += prefix;
 		name    += prefix;
-- 8< --

^ permalink raw reply related

* Re: [PATCH] Replace strcmp_icase with strequal_icase
From: Duy Nguyen @ 2013-03-09 10:40 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git
In-Reply-To: <1362818574-16873-1-git-send-email-iveqy@iveqy.com>

On Sat, Mar 9, 2013 at 3:42 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> To improve performance.

BTW, by rolling our own string comparison, we may lose certain
optimizations done by C library. In case of glibc, it may choose to
run an sse4.2 version where 16 bytes are compared at a time. Maybe we
encounter "string not equal" much often than "string equal" and such
an optimization is unncessary, I don't know. Measured numbers say it's
unncessary as my cpu supports sse4.2.
-- 
Duy

^ permalink raw reply

* Re: Memory corruption when rebasing with git version 1.8.1.5 on arch
From: Bernhard Posselt @ 2013-03-09 10:54 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20130309044850.GB12167@sigill.intra.peff.net>

On 03/09/2013 05:48 AM, Jeff King wrote:
> On Sat, Mar 09, 2013 at 01:08:32AM +0100, Bernhard Posselt wrote:
>
>>> The problem is likely happening in a sub-command of git-pull, so
>>> valgrind isn't reporting it. Can you try re-running with
>>> "valgrind --trace-children=yes", or alternatively narrow down the
>>> problematic command by setting GIT_TRACE=1 in the environment?
>> Heres the output with GIT_TRACE=1, the valgrind log has 4000 lines.
>> If you should still require the valgrind log, please tell me.
> Hmm, the GIT_TRACE output was less clear than I had hoped; it's unclear
> to me which git program is actually dying (my guess is "git apply", and
> we are squelching stderr, which is where the GIT_TRACE output is going).
>
> Can you try it once again with something like GIT_TRACE=/tmp/foo.out,
> which will make sure we record the trace directly, even if stderr ends
> up redirected?
>
> Also, I can almost reproduce here, as PatrickHeller/core.git is public.
> However, I suspect the problem is particular to your work built on top,
> which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
> Is it possible for you to make that commit available on a temporary
> branch?
>
> -Peff
 > commit available on a temporary branch?
What do you mean exactly by that?

I've made copies of both repositories on github.

Heres a copy of the basic repo: 
https://github.com/Raydiation/memorycorruption
Heres my clone of the repo that i pull from: 
https://github.com/Raydiation/core

Basically:

git clone https://github.com/Raydiation/memorycorruption
cd memorycorruption
git pull --rebase https://github.com/Raydiation/core

Heres the output of the GIT_TRACE file

trace: built-in: git 'branch'
trace: built-in: git 'branch' '--no-color'
trace: built-in: git 'status'
trace: exec: 'git-pull' '--rebase' 'https://github.com/Raydiation/core' 
'master'
trace: run_command: 'git-pull' '--rebase' 
'https://github.com/Raydiation/core' 'master'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--is-bare-repository'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'ls-files' '-u'
trace: built-in: git 'symbolic-ref' '-q' 'HEAD'
trace: built-in: git 'config' '--bool' 'branch.master.rebase'
trace: built-in: git 'config' '--bool' 'pull.rebase'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'rev-parse' '--verify' 'HEAD'
trace: built-in: git 'update-index' '-q' '--ignore-submodules' '--refresh'
trace: built-in: git 'diff-files' '--quiet' '--ignore-submodules'
trace: built-in: git 'diff-index' '--cached' '--quiet' 
'--ignore-submodules' 'HEAD' '--'
trace: built-in: git 'rev-parse' '-q' '--git-dir'
trace: built-in: git 'rev-parse' '-q' '--verify' 
'refs/remotes/https://github.com/Raydiation/core/master'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'fetch' '--update-head-ok' 
'https://github.com/Raydiation/core' 'master'
trace: run_command: 'git-remote-https' 
'https://github.com/Raydiation/core' 'https://github.com/Raydiation/core'
trace: run_command: 'rev-list' '--objects' '--stdin' '--not' '--all' 
'--quiet'
trace: exec: 'git' 'rev-list' '--objects' '--stdin' '--not' '--all' 
'--quiet'
trace: built-in: git 'rev-list' '--objects' '--stdin' '--not' '--all' 
'--quiet'
trace: run_command: 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/Raydiation/core/'
trace: exec: 'git' 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/Raydiation/core/'
trace: built-in: git 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/Raydiation/core/'
trace: run_command: 'unpack-objects' '--pack_header=2,3'
trace: exec: 'git' 'unpack-objects' '--pack_header=2,3'
trace: built-in: git 'unpack-objects' '--pack_header=2,3'
trace: run_command: 'rev-list' '--objects' '--stdin' '--not' '--all'
trace: exec: 'git' 'rev-list' '--objects' '--stdin' '--not' '--all'
trace: built-in: git 'rev-list' '--objects' '--stdin' '--not' '--all'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'show-branch' '--merge-base' 'refs/heads/master' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'fmt-merge-msg'
trace: built-in: git 'rev-parse' '--parseopt' '--' '--onto' 
'd686039828089d53fb42e42046d7a9a3992a0507' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--is-bare-repository'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'config' '--bool' 'rebase.stat'
trace: built-in: git 'config' '--bool' 'rebase.autosquash'
trace: built-in: git 'rev-parse' '--verify' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'rev-parse' '--verify' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'symbolic-ref' '-q' 'HEAD'
trace: built-in: git 'rev-parse' '--verify' 'master^0'
trace: built-in: git 'rev-parse' '--verify' 'HEAD'
trace: built-in: git 'update-index' '-q' '--ignore-submodules' '--refresh'
trace: built-in: git 'diff-files' '--quiet' '--ignore-submodules'
trace: built-in: git 'diff-index' '--cached' '--quiet' 
'--ignore-submodules' 'HEAD' '--'
trace: built-in: git 'merge-base' 
'd686039828089d53fb42e42046d7a9a3992a0507' 
'0525bbd73c9015499ba92d1ac654b980aaca35b2'
trace: built-in: git 'checkout' '-q' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'update-ref' 'ORIG_HEAD' 
'0525bbd73c9015499ba92d1ac654b980aaca35b2'
trace: built-in: git 'format-patch' '-k' '--stdout' '--full-index' 
'--ignore-if-in-upstream' '--src-prefix=a/' '--dst-prefix=b/' 
'--no-renames' 
'd686039828089d53fb42e42046d7a9a3992a0507..0525bbd73c9015499ba92d1ac654b980aaca35b2'
trace: exec: 'git-am' '--rebasing' '--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: run_command: 'git-am' '--rebasing' '--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: built-in: git 'rev-parse' '--parseopt' '--' '--rebasing' 
'--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--show-prefix'
trace: built-in: git 'rev-parse' '--is-inside-work-tree'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'var' 'GIT_COMMITTER_IDENT'
trace: built-in: git 'rev-parse' '--verify' '-q' 'HEAD'
trace: built-in: git 'config' '--bool' '--get' 'am.keepcr'
trace: built-in: git 'mailsplit' '-d4' 
'-o/srv/http/owncloud/.git/rebase-apply' '-b' '--'
trace: built-in: git 'update-index' '-q' '--refresh'
trace: built-in: git 'diff-index' '--cached' '--name-only' 'HEAD' '--'
trace: built-in: git 'cat-file' '-t' 
'48bb53030c657e1133da47765c7c778a069af665'
trace: built-in: git 'cat-file' 'commit' 
'48bb53030c657e1133da47765c7c778a069af665'
trace: built-in: git 'config' 'i18n.commitencoding'
trace: built-in: git 'show' '-s' '--pretty=raw' '--encoding=UTF-8' 
'48bb53030c657e1133da47765c7c778a069af665' '--'
trace: built-in: git 'diff-tree' '--root' '--binary' '--full-index' 
'48bb53030c657e1133da47765c7c778a069af665'
trace: built-in: git 'apply' '--index' 
'/srv/http/owncloud/.git/rebase-apply/patch'
trace: built-in: git 'write-tree'
trace: built-in: git 'rev-parse' '--verify' '-q' 'HEAD'
trace: built-in: git 'commit-tree' 
'd785d568d8b4649dfdcc01e03d6c8e87b036ea5a' '-p' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'update-ref' '-m' 'pull --rebase 
https://github.com/Raydiation/core master: distinguish between touch and 
write' 'HEAD' '3b7aa1e847993b2afdbaf19cd8ed50b81d37fc5b'
trace: built-in: git 'cat-file' '-t' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
trace: built-in: git 'cat-file' 'commit' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
trace: built-in: git 'config' 'i18n.commitencoding'
trace: built-in: git 'show' '-s' '--pretty=raw' '--encoding=UTF-8' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24' '--'
trace: built-in: git 'diff-tree' '--root' '--binary' '--full-index' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
trace: built-in: git 'apply' '--index' 
'/srv/http/owncloud/.git/rebase-apply/patch'

^ permalink raw reply

* Re: [PATCH] Replace strcmp_icase with strequal_icase
From: Duy Nguyen @ 2013-03-09 10:54 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git
In-Reply-To: <CACsJy8CphBDKsAAKjCoze98jv=4U+3pN3cW1OYD5XNhYgfcVCA@mail.gmail.com>

On Sat, Mar 9, 2013 at 5:40 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sat, Mar 9, 2013 at 3:42 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
>> To improve performance.
>
> BTW, by rolling our own string comparison, we may lose certain
> optimizations done by C library. In case of glibc, it may choose to
> run an sse4.2 version where 16 bytes are compared at a time. Maybe we
> encounter "string not equal" much often than "string equal" and such
> an optimization is unncessary, I don't know. Measured numbers say it's
> unncessary as my cpu supports sse4.2.

Another problem is locale. Git's toupper() does not care about locale,
which should be fine in most cases. strcasecmp is locale-aware, our
new str[n]equal_icase is not. It probably does not matter for
(ascii-based) pathnames, I guess. core.ignorecase users, any comments?
-- 
Duy

^ permalink raw reply

* Re: [PATCH] Replace strcmp_icase with strequal_icase
From: Fredrik Gustafsson @ 2013-03-09 11:08 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Fredrik Gustafsson, gitster, git
In-Reply-To: <CACsJy8BbXjJeTgo0DzKKMY7B3NZB=r3r+Z-WsWJR=t00DkTVzQ@mail.gmail.com>

On Sat, Mar 09, 2013 at 05:54:45PM +0700, Duy Nguyen wrote:
> On Sat, Mar 9, 2013 at 5:40 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> > On Sat, Mar 9, 2013 at 3:42 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> >> To improve performance.
> >
> > BTW, by rolling our own string comparison, we may lose certain
> > optimizations done by C library. In case of glibc, it may choose to
> > run an sse4.2 version where 16 bytes are compared at a time. Maybe we
> > encounter "string not equal" much often than "string equal" and such
> > an optimization is unncessary, I don't know. Measured numbers say it's
> > unncessary as my cpu supports sse4.2.
> 
> Another problem is locale. Git's toupper() does not care about locale,
> which should be fine in most cases. strcasecmp is locale-aware, our
> new str[n]equal_icase is not. It probably does not matter for
> (ascii-based) pathnames, I guess. core.ignorecase users, any comments?
> -- 
> Duy

Actually when implemented a str[n]equal_icase that actually should work.
I break the test suite when trying to replace
strncmp_icase(pathname, base, baselen)) on line 711 in dir.c and I don't
get any significant improvements.

I like work in this area though, slow commit's are my worst git problem.
I often have to wait 10s. for a commit to be calculated.


-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com


From c5d1f436cdbe7b12c67e81cf1d2904d1fb2e9b6b Mon Sep 17 00:00:00 2001
From: Fredrik Gustafsson <iveqy@iveqy.com>
Date: Sat, 9 Mar 2013 09:27:16 +0100
Subject: [PATCH] Replace strcmp_icase with strequal_icase

To improve performance.
git status before:
user    0m0.020s
user    0m0.024s
user    0m0.024s
user    0m0.020s
user    0m0.024s
user    0m0.028s
user    0m0.024s
user    0m0.024s
user    0m0.016s
user    0m0.028s

git status after:
wip

Tried to replace strncmp_icase on line 711 in dir.c but then failed to
run the testsuite. Did not got any relevant speed improvements of this.
---
 dir.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index 57394e4..aace36a 100644
--- a/dir.c
+++ b/dir.c
@@ -37,6 +37,51 @@ int fnmatch_icase(const char *pattern, const char *string, int flags)
 	return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));
 }
 
+int strnequal_icase(const char *first, const char *second, size_t count)
+{
+	if (ignore_case) {
+		while (*first && *second && count) {
+			if( toupper(*first) != toupper(*second))
+				break;
+			first++;
+			second++;
+			count--;
+		}
+		return toupper(*first) == toupper(*second);
+	} else {
+		while (*first && *second && count) {
+			if( *first != *second)
+				break;
+			first++;
+			second++;
+			count--;
+		}
+		return *first == *second;
+	}
+
+}
+
+int strequal_icase(const char *first, const char *second)
+{
+	if (ignore_case) {
+		while (*first && *second) {
+			if( toupper(*first) != toupper(*second))
+				break;
+			first++;
+			second++;
+		}
+		return toupper(*first) == toupper(*second);
+	} else {
+		while (*first && *second) {
+			if( *first != *second)
+				break;
+			first++;
+			second++;
+		}
+		return *first == *second;
+	}
+}
+
 inline int git_fnmatch(const char *pattern, const char *string,
 		       int flags, int prefix)
 {
@@ -626,11 +671,11 @@ int match_basename(const char *basename, int basenamelen,
 		   int flags)
 {
 	if (prefix == patternlen) {
-		if (!strcmp_icase(pattern, basename))
+		if (strequal_icase(pattern, basename))
 			return 1;
 	} else if (flags & EXC_FLAG_ENDSWITH) {
 		if (patternlen - 1 <= basenamelen &&
-		    !strcmp_icase(pattern + 1,
+		    strequal_icase(pattern + 1,
 				  basename + basenamelen - patternlen + 1))
 			return 1;
 	} else {
-- 
1.7.2.5

^ permalink raw reply related

* Re: rebase: strange failures to apply patc 3-way
From: Max Horn @ 2013-03-09 11:26 UTC (permalink / raw)
  To: Andrew Wong; +Cc: git@vger.kernel.org
In-Reply-To: <CADgNja=Ej8jnYn027GX986VrmuqVemM7aE59rynHzUpToPVaEw@mail.gmail.com>


On 08.03.2013, at 20:20, Andrew Wong wrote:

> On 3/8/13, Max Horn <max@quendi.de> wrote:
>> Same result, it works fine.
> 
> Just shooting in the dark here... I wonder if there's some background
> process running in OS X that's messing with the files/directories
> while rebase is working... backup, virus scan, etc? Or maybe some
> programs that you're using at the same time? Maybe also make sure you
> don't have any programs (shells, editors, etc.) opened that's
> accessing those files/directories?

I am pretty sure no other programs are accessing those files at the same time. But just to make sure I quite most programs. No virus scanner running. No backup running -- although, OS X automatically runs hourly backups as part of Time Machine... So just to be triple certain, I black listed the repos dir in both the "Time Machine" backup service and the "Spotlight" indexing service.

No diference. In the end I even did a reboot, but that made no differences either (which I am quite relieved about, I must say ;-).


> 
> Does the error always happen at COMMIT A and COMMIT B? Or is it all
> over the place?

It tends to fail in separate places, but eventually "stabilizes". E.g. I just did a couple test rebases, and it failed twice in commit 14, then the third time in commit 15 (which underlines once more that the failures are inappropriate).

The fourth time, something new and weird happened:

$ git rebase --abort
$ git rebase NEW-PARENT 
Cannot rebase: You have unstaged changes.
Please commit or stash them.
$

This is quite suspicious. It appears that git for some reason things a file is dirty when it isn't. That could explain the other rebase failures too, couldn't it? But what might cause such a thing?


I checked with "git st" and it reported no changed files. Executing the "rebase" once again then "worked" as before... I.e. it got stuck in commit 15. The next time it got till commit 16. Then back to commit 15. Etc. Now it is getting stuck on commit 17 (but it doesn't always go up as it did right now).


> 
> In cases where COMMIT A succeeded, did it say it did a 3-way merge? Or
> was it exactly as the output in your original message? i.e. no message
> at all

It's always a variation of the same message as shown in my original email. I.e.:

Applying: ...
...
Applying: commit XYZ
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
error: Your local changes to the following files would be overwritten by merge:
	some/file
Please, commit your changes or stash them before you can merge.
Aborting
Failed to merge in the changes.
Patch failed at 0015 commit XYZ
The copy of the patch that failed is found in:
   /path/to/repos/.git/rebase-apply/patch





Thanks,
Max

^ permalink raw reply

* Re: [PATCH] Replace strcmp_icase with strequal_icase
From: Duy Nguyen @ 2013-03-09 12:05 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git
In-Reply-To: <20130309110815.GA8328@paksenarrion.iveqy.com>

On Sat, Mar 9, 2013 at 6:08 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> Actually when implemented a str[n]equal_icase that actually should work.
> I break the test suite when trying to replace
> strncmp_icase(pathname, base, baselen)) on line 711 in dir.c and I don't
> get any significant improvements.

Hmm.. mine passed the test suite.

> I like work in this area though, slow commit's are my worst git problem.
> I often have to wait 10s. for a commit to be calculated.

Personally I don't accept any often used git commands taking more than
1 second (in hot cache case). What commands do you use? What's the
size of the repository in terms of tracked/untracked files?
-- 
Duy

^ permalink raw reply

* Re: [PATCH] Replace strcmp_icase with strequal_icase
From: Fredrik Gustafsson @ 2013-03-09 12:22 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Fredrik Gustafsson, gitster, git
In-Reply-To: <CACsJy8D4Yqm3s+ALf=KnMQRQ6SrVcM5jjktpGXiGcOaqtEsyMg@mail.gmail.com>

On Sat, Mar 09, 2013 at 07:05:37PM +0700, Duy Nguyen wrote:
> On Sat, Mar 9, 2013 at 6:08 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> > Actually when implemented a str[n]equal_icase that actually should work.
> > I break the test suite when trying to replace
> > strncmp_icase(pathname, base, baselen)) on line 711 in dir.c and I don't
> > get any significant improvements.
> 
> Hmm.. mine passed the test suite.

Using my patch or your own code? Maybe I just did something wrong. Could
you see any improvements in speed?

> 
> > I like work in this area though, slow commit's are my worst git problem.
> > I often have to wait 10s. for a commit to be calculated.
> 
> Personally I don't accept any often used git commands taking more than
> 1 second (in hot cache case). What commands do you use? What's the
> size of the repository in terms of tracked/untracked files?

It's a small repository, 100 MB. However I have a slow hdd which is
almost full. I often add one file and make an one-line change to an
other file and then do a git commit -a. That will make git to look
through the whole repo, which isn't in the kernel RAM cache but needs to
be reed from the hdd.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply

* Re: [PATCH] Replace strcmp_icase with strequal_icase
From: Duy Nguyen @ 2013-03-09 12:40 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git
In-Reply-To: <20130309122200.GA7755@paksenarrion.iveqy.com>

On Sat, Mar 9, 2013 at 7:22 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> On Sat, Mar 09, 2013 at 07:05:37PM +0700, Duy Nguyen wrote:
>> On Sat, Mar 9, 2013 at 6:08 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
>> > Actually when implemented a str[n]equal_icase that actually should work.
>> > I break the test suite when trying to replace
>> > strncmp_icase(pathname, base, baselen)) on line 711 in dir.c and I don't
>> > get any significant improvements.
>>
>> Hmm.. mine passed the test suite.
>
> Using my patch or your own code? Maybe I just did something wrong. Could
> you see any improvements in speed?

It's the one I posted in [1] and yes it improves speed, numbers in [1].

[1] http://thread.gmane.org/gmane.comp.version-control.git/217712/focus=217724

>> > I like work in this area though, slow commit's are my worst git problem.
>> > I often have to wait 10s. for a commit to be calculated.
>>
>> Personally I don't accept any often used git commands taking more than
>> 1 second (in hot cache case). What commands do you use? What's the
>> size of the repository in terms of tracked/untracked files?
>
> It's a small repository, 100 MB. However I have a slow hdd which is
> almost full. I often add one file and make an one-line change to an
> other file and then do a git commit -a. That will make git to look
> through the whole repo, which isn't in the kernel RAM cache but needs to
> be reed from the hdd.

"commit -a" does not run exclude (what I'm improving here). It's
probably stat problem. If you already know what files you have
changed, "git add path..." then commit without -a might help. Or turn
on core.ignorestat (read doc about it first).
-- 
Duy

^ permalink raw reply

* Re: [PATCH] connect.c: Tell *PLink to always use ssh protocol
From: Sven Strickroth @ 2013-03-09 14:08 UTC (permalink / raw)
  To: Jeff King; +Cc: msysgit, git, gitster
In-Reply-To: <20130206232214.GN27507@sigill.intra.peff.net>

Am 07.02.2013 00:22 schrieb Jeff King:
> On Wed, Feb 06, 2013 at 10:58:49PM +0100, Sven Strickroth wrote:
> 
>> Default values for *plink can be set using PuTTY. If a user makes
>> telnet the default in PuTTY this breaks ssh clones in git.
>>
>> Since git clones of the type user@host:path use ssh, tell *plink
>> to use ssh and override PuTTY defaults for the protocol to use.
>>
>> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> 
> Makes sense to me, though I'd expect to see this cc'd to the msysgit
> list (which I'm doing on this response) for comment from people who
> might be more familiar with the area.

The msysgit people have a git-wrapper which already enables this (see
https://github.com/msysgit/msysgit/blob/master/src/git-wrapper/git-wrapper.c#L62).

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply

* [ANNOUNCE] TopGit 0.9
From: Robin Green @ 2013-03-09 14:27 UTC (permalink / raw)
  To: git

Hello,

I'm pleased to announce that TopGit 0.9 was released today.

TopGit aims to make handling of large amounts of interdependent topic
branches easier. In fact, it is designed especially for the case where
you maintain a queue of third-party patches on top of another (perhaps
Git-controlled) project and want to easily organize, maintain and 
submit
them - TopGit achieves that by keeping a separate topic branch for each
patch and providing some tools to maintain the branches.

After a long hiatus, TopGit has a new release, a new maintainer (me), 
and
has moved to GitHub:

   https://github.com/greenrd/topgit

(note that patches are still required to be signed off.)

The 0.9 release is tagged and available for download at: 
https://github.com/greenrd/topgit/tags
(Yes, I know GitHub's display of the tag description is terrible, but 
it
shows up fine in git.)

Because it's been 3 years since the last release, there are quite a few
patches since 0.8, but most of them are quite minor changes.

If you are upgrading from the HEAD of the old TopGit repository, all of 
and
only the patches by me, Andrey Borzenkov and Heiko Hund are new 
compared to
that revision.

Patches can be submitted either to me, to this list, or to the GitHub
issue tracker. There is also an IRC channel, #topgit on 
irc.freenode.net.

Regards,
Robin Green

^ permalink raw reply

* rebase --merge question
From: Stijn Souffriau @ 2013-03-09 15:15 UTC (permalink / raw)
  To: git

Hi all,

 From help rebase:

--merge
	Use merging strategies to rebase. When the recursive (default) merge 
strategy is used, this allows rebase to be aware of renames on the 
upstream side.

Renames of what? Files I assume. Are there any disadvantages compared to 
the normal rebase? If not, why isn't --merge the default behaviour.

Thanks,

Stijn

-- 
Stijn Souffriau
Embedded Software Developer - Mind Embedded Software Division

ESSENSIUM nv
Mind - Embedded Software Division
Gaston Geenslaan 9 - B-3001 Leuven
email : stijn.souffriau@essensium.com
Web: www.essensium.com   /   www.mind.be
BE 872 984 063 RPR Leuven

^ permalink raw reply

* Re: [PATCH] format-patch: RFC 2047 says multi-octet character may not be split
From: Kirill Smelkov @ 2013-03-09 15:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Dmitry Komissarov, git, Jan H. Schönherr, kirr
In-Reply-To: <7vobevm6fp.fsf@alter.siamese.dyndns.org>

On Thu, Mar 07, 2013 at 10:05:30AM -0800, Junio C Hamano wrote:
> Kirill Smelkov <kirr@mns.spb.ru> writes:
> 
> >> > @@ -367,16 +376,18 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
> >> >  		 * causes ' ' to be encoded as '=20', avoiding this problem.
> >> >  		 */
> >> >  
> >> > +		if (line_len + 2 + (is_special ? 3*chrlen : 1) > max_encoded_length) {
> >> 
> >> Always have SP around binary operators such as '*' (multiplication).
> >
> > ok, but note that's just a matter of style, and if one is used to code
> > formulas,...
> 
> Well, when working on a project with others, what _you_ are used to
> does not matter.  Also please never call coding style "just a matter
> of".  Keeping things consistent with the style around the area is a
> prerequisite.
> 
>    When you have time:
>    Cf. https://www.youtube.com/watch?feature=player_embedded&v=fMeH7wqOwXA

Junio, what Greg says here is all known and good and respected. I agree
coding style is not a "just a matter of" and is important to follow for
project to stay consisting. My note here was just a sentiment about
spaces around operators, which I didn't know was in the coding style
because it is not in Documentation/CodingGuidelines, and especially if
the project sometimes uses my style

    *offset = 60*off;                                       date.c      5e2a78a4
    diff += 7*n;                                            date.c      6b7b0427
    sub_size < 2*window && i+1 < delta_search_threads       pack_objects.c  bf874896


But anyway, I'm ok with any style the project chooses - it's not so important
for me to insist here, so let it be "3 * chrlen" and lets forget about it.


> > Actually if we add encoded_len, adding encoded_fmt is tempting
> >
> >     const char *encoded_fmt = is_special ? "=%02X"    : "%c";
> >
> > and then encoding part simplifies to just unconditional
> >
> >     for (i = 0; i < chrlen; i++)
> >             strbuf_addf(sb, encoded_fmt, p[i]);
> >     line_len += encoded_len;
> 
> Sounds very sensible ;-)

Thanks.

> >  		 * for now, let's treat encodings != UTF-8 as one-byte
> >  		 */
> >  		chrlen = 1;
> >
> > ---- 8< ----
> > From 46b9cddc63c07cb5513cfbf6d20aaaa98c66bcdf Mon Sep 17 00:00:00 2001
> > From: Kirill Smelkov <kirr@mns.spb.ru>
> > Date: Wed, 6 Mar 2013 14:28:46 +0400
> > Subject: [PATCH v2] format-patch: RFC 2047 says multi-octet character may not be split
> 
> Good use of scissors line; but please drop these four lines after
> it.  The first is unwanted and the rest are redundant.
> 
> > Even though an earlier attempt (bafc478..41dd00bad) cleaned
> > up RFC 2047 encoding, pretty.c::add_rfc2047() still decides
> > where to split the output line by going through the input
> > ...
> > @@ -367,18 +380,15 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
> >  		 * causes ' ' to be encoded as '=20', avoiding this problem.
> >  		 */
> >  
> > -		if (line_len + 2 + (is_special ? 3 : 1) > max_encoded_length) {
> > +		if (line_len + encoded_len + /* ?= */2 > max_encoded_length) {
> > +			/* It will not fit---break the line */
> 
> It doesn't look much clearer with /* ?= */ unless we say something
> that contains the word "close", e.g. "?= to close the encoded part".
> Maybe it is just me.

How about

        if (line_len + encoded_len + 2 > max_encoded_length) {
                /* It won't fit with trailing "?=" --- break the line */

?

> 
> > -		if (is_special) {
> > -			strbuf_addf(sb, "=%02X", ch);
> > -			line_len += 3;
> > -		} else {
> > -			strbuf_addch(sb, ch);
> > -			line_len++;
> > -		}
> > +		for (i = 0; i < chrlen; i++)
> > +			strbuf_addf(sb, encoded_fmt, p[i]);
> > +		line_len += encoded_len;
> 
> Nice code reduction.

Thanks.

Interdiff and updated patch follow. Note I'm sending this from home, so
'From:' line after scissors is kept as necessary.

Kirill

P.S. sorry for the delay - I harmed my arm yesterday.


diff --git a/pretty.c b/pretty.c
index 8fce619..41f04e6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -380,8 +380,8 @@ static void add_rfc2047(struct strbuf *sb, const char *line, size_t len,
 		 * causes ' ' to be encoded as '=20', avoiding this problem.
 		 */
 
-		if (line_len + encoded_len + /* ?= */2 > max_encoded_length) {
-			/* It will not fit---break the line */
+		if (line_len + encoded_len + 2 > max_encoded_length) {
+			/* It won't fit with trailing "?=" --- break the line */
 			strbuf_addf(sb, "?=\n =?%s?q?", encoding);
 			line_len = strlen(encoding) + 5 + 1; /* =??q? plus SP */
 		}

---- 8< ----
From: Kirill Smelkov <kirr@mns.spb.ru>
 split

Even though an earlier attempt (bafc478..41dd00bad) cleaned
up RFC 2047 encoding, pretty.c::add_rfc2047() still decides
where to split the output line by going through the input
one byte at a time, and potentially splits a character in
the middle.  A subject line may end up showing like this:

     ".... fö?? bar".   (instead of  ".... föö bar".)

if split incorrectly.

RFC 2047, section 5 (3) explicitly forbids such behaviour

    Each 'encoded-word' MUST represent an integral number of
    characters.  A multi-octet character may not be split across
    adjacent 'encoded- word's.

that means that e.g. for

    Subject: .... föö bar

encoding

    Subject: =?UTF-8?q?....=20f=C3=B6=C3=B6?=
     =?UTF-8?q?=20bar?=

is correct, and

    Subject: =?UTF-8?q?....=20f=C3=B6=C3?=      <-- NOTE ö is broken here
     =?UTF-8?q?=B6=20bar?=

is not, because "ö" character UTF-8 encoding C3 B6 is split here across
adjacent encoded words.

To fix the problem, make the loop grab one _character_ at a time and
determine its output length to see where to break the output line.  Note
that this version only knows about UTF-8, but the logic to grab one
character is abstracted out in mbs_chrlen() function to make it possible
to extend it to other encodings with the help of iconv in the future.

(With help from Junio C Hamano <gitster@pobox.com>)
Cc: Jan H. Schönherr <schnhrr@cs.tu-berlin.de>
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
---
 pretty.c                | 34 ++++++++++++++++++++++------------
 t/t4014-format-patch.sh | 27 ++++++++++++++-------------
 utf8.c                  | 39 +++++++++++++++++++++++++++++++++++++++
 utf8.h                  |  2 ++
 4 files changed, 77 insertions(+), 25 deletions(-)

diff --git a/pretty.c b/pretty.c
index b57adef..41f04e6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -345,7 +345,7 @@ static int needs_rfc2047_encoding(const char *line, int len,
 	return 0;
 }
 
-static void add_rfc2047(struct strbuf *sb, const char *line, int len,
+static void add_rfc2047(struct strbuf *sb, const char *line, size_t len,
 		       const char *encoding, enum rfc2047_type type)
 {
 	static const int max_encoded_length = 76; /* per rfc2047 */
@@ -355,9 +355,22 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
 	strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
 	strbuf_addf(sb, "=?%s?q?", encoding);
 	line_len += strlen(encoding) + 5; /* 5 for =??q? */
-	for (i = 0; i < len; i++) {
-		unsigned ch = line[i] & 0xFF;
-		int is_special = is_rfc2047_special(ch, type);
+
+	while (len) {
+		/*
+		 * RFC 2047, section 5 (3):
+		 *
+		 * Each 'encoded-word' MUST represent an integral number of
+		 * characters.  A multi-octet character may not be split across
+		 * adjacent 'encoded- word's.
+		 */
+		const unsigned char *p = (const unsigned char *)line;
+		int chrlen = mbs_chrlen(&line, &len, encoding);
+		int is_special = (chrlen > 1) || is_rfc2047_special(*p, type);
+
+		/* "=%02X" * chrlen, or the byte itself */
+		const char *encoded_fmt = is_special ? "=%02X"    : "%c";
+		int	    encoded_len = is_special ? 3 * chrlen : 1;
 
 		/*
 		 * According to RFC 2047, we could encode the special character
@@ -367,18 +380,15 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
 		 * causes ' ' to be encoded as '=20', avoiding this problem.
 		 */
 
-		if (line_len + 2 + (is_special ? 3 : 1) > max_encoded_length) {
+		if (line_len + encoded_len + 2 > max_encoded_length) {
+			/* It won't fit with trailing "?=" --- break the line */
 			strbuf_addf(sb, "?=\n =?%s?q?", encoding);
 			line_len = strlen(encoding) + 5 + 1; /* =??q? plus SP */
 		}
 
-		if (is_special) {
-			strbuf_addf(sb, "=%02X", ch);
-			line_len += 3;
-		} else {
-			strbuf_addch(sb, ch);
-			line_len++;
-		}
+		for (i = 0; i < chrlen; i++)
+			strbuf_addf(sb, encoded_fmt, p[i]);
+		line_len += encoded_len;
 	}
 	strbuf_addstr(sb, "?=");
 }
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 78633cb..b993dae 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -837,25 +837,26 @@ Subject: [PATCH] =?UTF-8?q?f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
- =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
+ =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
+ =?UTF-8?q?bar?=
 EOF
 test_expect_success 'format-patch wraps extremely long subject (rfc2047)' '
 	rm -rf patches/ &&
diff --git a/utf8.c b/utf8.c
index 8f6e84b..7f64857 100644
--- a/utf8.c
+++ b/utf8.c
@@ -531,3 +531,42 @@ char *reencode_string(const char *in, const char *out_encoding, const char *in_e
 	return out;
 }
 #endif
+
+/*
+ * Returns first character length in bytes for multi-byte `text` according to
+ * `encoding`.
+ *
+ * - The `text` pointer is updated to point at the next character.
+ * - When `remainder_p` is not NULL, on entry `*remainder_p` is how much bytes
+ *   we can consume from text, and on exit `*remainder_p` is reduced by returned
+ *   character length. Otherwise `text` is treated as limited by NUL.
+ */
+int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding)
+{
+	int chrlen;
+	const char *p = *text;
+	size_t r = (remainder_p ? *remainder_p : SIZE_MAX);
+
+	if (r < 1)
+		return 0;
+
+	if (is_encoding_utf8(encoding)) {
+		pick_one_utf8_char(&p, &r);
+
+		chrlen = p ? (p - *text)
+			   : 1 /* not valid UTF-8 -> raw byte sequence */;
+	}
+	else {
+		/*
+		 * TODO use iconv to decode one char and obtain its chrlen
+		 * for now, let's treat encodings != UTF-8 as one-byte
+		 */
+		chrlen = 1;
+	}
+
+	*text += chrlen;
+	if (remainder_p)
+		*remainder_p -= chrlen;
+
+	return chrlen;
+}
diff --git a/utf8.h b/utf8.h
index 501b2bd..1f8ecad 100644
--- a/utf8.h
+++ b/utf8.h
@@ -22,4 +22,6 @@ char *reencode_string(const char *in, const char *out_encoding, const char *in_e
 #define reencode_string(a,b,c) NULL
 #endif
 
+int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
+
 #endif
-- 
1.8.2.rc2.366.g3bc8dda

^ permalink raw reply related

* [PATCH] perf: update documentation of GIT_PERF_REPEAT_COUNT
From: Antoine Pelisse @ 2013-03-09 15:29 UTC (permalink / raw)
  To: git; +Cc: Antoine Pelisse

Currently the documentation of GIT_PERF_REPEAT_COUNT says the default is
five while "perf-lib.sh" uses a value of three as a default.

Update the documentation so that it is consistent with the code.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 t/perf/README |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/perf/README b/t/perf/README
index b2dbad4..c552f56 100644
--- a/t/perf/README
+++ b/t/perf/README
@@ -56,7 +56,7 @@ You can set the following variables (also in your config.mak):
 
     GIT_PERF_REPEAT_COUNT
 	Number of times a test should be repeated for best-of-N
-	measurements.  Defaults to 5.
+	measurements.  Defaults to 3.
 
     GIT_PERF_MAKE_OPTS
 	Options to use when automatically building a git tree for
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] format-patch: RFC 2047 says multi-octet character may not be split
From: Kirill Smelkov @ 2013-03-09 15:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Dmitry Komissarov, git, Jan H. Schönherr
In-Reply-To: <20130309152722.GA32248@mini.zxlink>

On Sat, Mar 09, 2013 at 07:27:23PM +0400, Kirill Smelkov wrote:
> ---- 8< ----
> From: Kirill Smelkov <kirr@mns.spb.ru>
>  split

Sorry for the confusion...

---- 8< ----
From: Kirill Smelkov <kirr@mns.spb.ru>

Even though an earlier attempt (bafc478..41dd00bad) cleaned
up RFC 2047 encoding, pretty.c::add_rfc2047() still decides
where to split the output line by going through the input
one byte at a time, and potentially splits a character in
the middle.  A subject line may end up showing like this:

     ".... fö?? bar".   (instead of  ".... föö bar".)

if split incorrectly.

RFC 2047, section 5 (3) explicitly forbids such beaviour

    Each 'encoded-word' MUST represent an integral number of
    characters.  A multi-octet character may not be split across
    adjacent 'encoded- word's.

that means that e.g. for

    Subject: .... föö bar

encoding

    Subject: =?UTF-8?q?....=20f=C3=B6=C3=B6?=
     =?UTF-8?q?=20bar?=

is correct, and

    Subject: =?UTF-8?q?....=20f=C3=B6=C3?=      <-- NOTE ö is broken here
     =?UTF-8?q?=B6=20bar?=

is not, because "ö" character UTF-8 encoding C3 B6 is split here across
adjacent encoded words.

To fix the problem, make the loop grab one _character_ at a time and
determine its output length to see where to break the output line.  Note
that this version only knows about UTF-8, but the logic to grab one
character is abstracted out in mbs_chrlen() function to make it possible
to extend it to other encodings with the help of iconv in the future.

(With help from Junio C Hamano <gitster@pobox.com>)
Cc: Jan H. Schönherr <schnhrr@cs.tu-berlin.de>
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
---
 pretty.c                | 34 ++++++++++++++++++++++------------
 t/t4014-format-patch.sh | 27 ++++++++++++++-------------
 utf8.c                  | 39 +++++++++++++++++++++++++++++++++++++++
 utf8.h                  |  2 ++
 4 files changed, 77 insertions(+), 25 deletions(-)

diff --git a/pretty.c b/pretty.c
index b57adef..41f04e6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -345,7 +345,7 @@ static int needs_rfc2047_encoding(const char *line, int len,
 	return 0;
 }
 
-static void add_rfc2047(struct strbuf *sb, const char *line, int len,
+static void add_rfc2047(struct strbuf *sb, const char *line, size_t len,
 		       const char *encoding, enum rfc2047_type type)
 {
 	static const int max_encoded_length = 76; /* per rfc2047 */
@@ -355,9 +355,22 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
 	strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
 	strbuf_addf(sb, "=?%s?q?", encoding);
 	line_len += strlen(encoding) + 5; /* 5 for =??q? */
-	for (i = 0; i < len; i++) {
-		unsigned ch = line[i] & 0xFF;
-		int is_special = is_rfc2047_special(ch, type);
+
+	while (len) {
+		/*
+		 * RFC 2047, section 5 (3):
+		 *
+		 * Each 'encoded-word' MUST represent an integral number of
+		 * characters.  A multi-octet character may not be split across
+		 * adjacent 'encoded- word's.
+		 */
+		const unsigned char *p = (const unsigned char *)line;
+		int chrlen = mbs_chrlen(&line, &len, encoding);
+		int is_special = (chrlen > 1) || is_rfc2047_special(*p, type);
+
+		/* "=%02X" * chrlen, or the byte itself */
+		const char *encoded_fmt = is_special ? "=%02X"    : "%c";
+		int	    encoded_len = is_special ? 3 * chrlen : 1;
 
 		/*
 		 * According to RFC 2047, we could encode the special character
@@ -367,18 +380,15 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
 		 * causes ' ' to be encoded as '=20', avoiding this problem.
 		 */
 
-		if (line_len + 2 + (is_special ? 3 : 1) > max_encoded_length) {
+		if (line_len + encoded_len + 2 > max_encoded_length) {
+			/* It won't fit with trailing "?=" --- break the line */
 			strbuf_addf(sb, "?=\n =?%s?q?", encoding);
 			line_len = strlen(encoding) + 5 + 1; /* =??q? plus SP */
 		}
 
-		if (is_special) {
-			strbuf_addf(sb, "=%02X", ch);
-			line_len += 3;
-		} else {
-			strbuf_addch(sb, ch);
-			line_len++;
-		}
+		for (i = 0; i < chrlen; i++)
+			strbuf_addf(sb, encoded_fmt, p[i]);
+		line_len += encoded_len;
 	}
 	strbuf_addstr(sb, "?=");
 }
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 78633cb..b993dae 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -837,25 +837,26 @@ Subject: [PATCH] =?UTF-8?q?f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
- =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
+ =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
+ =?UTF-8?q?bar?=
 EOF
 test_expect_success 'format-patch wraps extremely long subject (rfc2047)' '
 	rm -rf patches/ &&
diff --git a/utf8.c b/utf8.c
index 8f6e84b..7f64857 100644
--- a/utf8.c
+++ b/utf8.c
@@ -531,3 +531,42 @@ char *reencode_string(const char *in, const char *out_encoding, const char *in_e
 	return out;
 }
 #endif
+
+/*
+ * Returns first character length in bytes for multi-byte `text` according to
+ * `encoding`.
+ *
+ * - The `text` pointer is updated to point at the next character.
+ * - When `remainder_p` is not NULL, on entry `*remainder_p` is how much bytes
+ *   we can consume from text, and on exit `*remainder_p` is reduced by returned
+ *   character length. Otherwise `text` is treated as limited by NUL.
+ */
+int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding)
+{
+	int chrlen;
+	const char *p = *text;
+	size_t r = (remainder_p ? *remainder_p : SIZE_MAX);
+
+	if (r < 1)
+		return 0;
+
+	if (is_encoding_utf8(encoding)) {
+		pick_one_utf8_char(&p, &r);
+
+		chrlen = p ? (p - *text)
+			   : 1 /* not valid UTF-8 -> raw byte sequence */;
+	}
+	else {
+		/*
+		 * TODO use iconv to decode one char and obtain its chrlen
+		 * for now, let's treat encodings != UTF-8 as one-byte
+		 */
+		chrlen = 1;
+	}
+
+	*text += chrlen;
+	if (remainder_p)
+		*remainder_p -= chrlen;
+
+	return chrlen;
+}
diff --git a/utf8.h b/utf8.h
index 501b2bd..1f8ecad 100644
--- a/utf8.h
+++ b/utf8.h
@@ -22,4 +22,6 @@ char *reencode_string(const char *in, const char *out_encoding, const char *in_e
 #define reencode_string(a,b,c) NULL
 #endif
 
+int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
+
 #endif
-- 
1.8.2.rc2.366.g3bc8dda

^ permalink raw reply related

* Re: Merging submodules - best merge-base
From: Jens Lehmann @ 2013-03-09 17:45 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: Daniel Bratell, git
In-Reply-To: <20130307185906.GA9661@sandbox-ub.fritz.box>

Am 07.03.2013 19:59, schrieb Heiko Voigt:
> On Thu, Mar 07, 2013 at 10:49:09AM +0100, Daniel Bratell wrote:
>> Den 2013-03-06 19:12:05 skrev Heiko Voigt <hvoigt@hvoigt.net>:
>>
>>> On Mon, Feb 25, 2013 at 05:44:05PM +0100, Daniel Bratell wrote:
>>>> A submodule change can be merged, but only if the merge is a
>>>> "fast-forward" which I think is a fair demand, but currently it
>>>> checks if
>>>> it's a fast-forward from a commit that might not be very interesting
>>>> anymore.
>>>>
>>>> If two branches A and B split at a point when they used submodule commit
>>>> S1 (based on S), and both then switched to S2 (also based on S)
>>>> and B then
>>>> switched to S21, then it's today not possible to merge B into A, despite
>>>> S21 being a descendant of S2 and you get a conflict and this warning:
>>>>
>>>> warning: Failed to merge submodule S (commits don't follow merge-base)
>>>>
>>>> (attempt at ASCII gfx:
>>>>
>>>> Submodule tree:
>>>>
>>>> S ---- S1
>>>>   \
>>>>    \ - S2 -- S21
>>>>
>>>> Main tree:
>>>>
>>>> A' (uses S1) --- A (uses S2)
>>>>   \
>>>>    \ --- B' (uses S2) -- B (uses S21)
>>>>
>>>>
>>>> I would like it to end up as:
>>>>
>>>> A' (uses S1) --- A (uses S2) ------------ A+ (uses S21)
>>>>   \                                     /
>>>>    \ --- B' (uses S2) -- B (uses S21)- /
>>>>
>>>> And that should be legal since S21 is a descendant of S2.
>>>
>>> So to summarize what you are requesting: You want a submodule merge be
>>> two way in the view of the superproject and calculate the merge base
>>> in the submodule from the two commits that are going to be merged?
>>>
>>> It currently sounds logical but I have to think about it further and
>>> whether that might break other use cases.
>>
>> Maybe both could be legal even. The current code can't be all wrong,
>> and this case also seems to be straightforward.
> 
> Ok I have thought about it further and I did not come up with a simple
> (and stable) enough strategy that would allow your use case to merge
> cleanly without user interaction.
> 
> The problem is that your are actually doing a rewind from base to both
> tips. The fact that a rewind is there makes git suspicious and we simply
> give up. IMO, thats the right thing to do in such a situation.
> 
> What should a merge strategy do? It infers from two changes what the
> final intention might be. For submodules we can do that when the changes
> on both sides point forward. Since thats the typical progress of
> development. If not there is some reason for it we do not know about. So
> the merge gives up.
> 
> Please see this post about why we need to forbid rewinds from the
> initial design discussion:
> 
> http://article.gmane.org/gmane.comp.version-control.git/149003

I agree that rewinds are a very good reason not merge two branches using
a fast-forward strategy, but I believe Daniel's use case is a (and maybe
the only) valid exception to that rule: both branches contain *exactly*
the same rewind. In that case I don't see any problem to just do a fast
forward to S21, as both agree on the commits to rewind.

^ permalink raw reply

* Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well
From: Jens Lehmann @ 2013-03-09 18:18 UTC (permalink / raw)
  To: Phil Hord
  Cc: Junio C Hamano, Eric Cousineau, Heiko Voigt, git@vger.kernel.org
In-Reply-To: <CABURp0psgofX=ean+KKooN74pF4Ns-gDGt68vc5Exs6NmDRUyw@mail.gmail.com>

Am 05.03.2013 22:17, schrieb Phil Hord:
> On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 05.03.2013 19:34, schrieb Junio C Hamano:
>>> Eric Cousineau <eacousineau@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}'
>    <up><home><del>{30-ish}<end><backspace><enter>
> 
> 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? ;-)

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 $@\"

^ permalink raw reply

* Re: rebase: strange failures to apply patc 3-way
From: Andrew Wong @ 2013-03-09 18:32 UTC (permalink / raw)
  To: Max Horn; +Cc: git@vger.kernel.org
In-Reply-To: <3B5EA38E-9603-4321-AA3C-74354BBC8BFC@quendi.de>

On 03/09/13 06:26, Max Horn wrote:
> It tends to fail in separate places, but eventually "stabilizes". E.g. I just did a couple test rebases, and it failed twice in commit 14, then the third time in commit 15 (which underlines once more that the failures are inappropriate).
>
> The fourth time, something new and weird happened:
>
> $ git rebase --abort
> $ git rebase NEW-PARENT 
> Cannot rebase: You have unstaged changes.
> Please commit or stash them.
> $
>
> This is quite suspicious. It appears that git for some reason things a file is dirty when it isn't. That could explain the other rebase failures too, couldn't it? But what might cause such a thing?
Yea, that's really suspicious. This could mean there's an issue with
when git is checking the index. Try running these a couple times in a
clean work tree:
    $ git update-index --refresh
    $ git diff-files

In a clean work tree, these commands should print nothing. But in your
case, these might print random files that git thinks have been modified...

If the commands do print out some files, check the timestamp from the
git index and the filesystem:
    $ git ls-files --debug file1 file2
    $ stat -f "%N %m %c" file1 file2

Is this repo on a network drive? Or is it local drive in your Mac?

^ permalink raw reply

* Re: rebase: strange failures to apply patc 3-way
From: Andrew Wong @ 2013-03-09 18:50 UTC (permalink / raw)
  To: Max Horn; +Cc: git@vger.kernel.org
In-Reply-To: <513B8037.7060107@gmail.com>

On 03/09/13 13:32, Andrew Wong wrote:
> Yea, that's really suspicious. This could mean there's an issue with
> when git is checking the index. Try running these a couple times in a
> clean work tree:
>     $ git update-index --refresh
>     $ git diff-files
>
> In a clean work tree, these commands should print nothing. But in your
> case, these might print random files that git thinks have been modified...
Before you run those commands each time, you probably have to "touch"
couple files to trigger the issue:
    $ touch file1 file2

Maybe use touch on the files that git rebase has been reporting error?

^ permalink raw reply

* [PATCH 0/19] git-subtree updates
From: Paul Campbell @ 2013-03-09 19:19 UTC (permalink / raw)
  To: git
  Cc: David Michael Barr, Kindjal, bibendi, Herman van Rink,
	James Roper, mhart, mhoffman, Nate Jones, Paul Cartwright,
	Peter Jaros

The following set of commits are taken from the collection collated by
Herman van Rink and offered to the list in May of last year
($gmane/196667).

Where updating a commit to resolve a conflict has cleared the original
author of the commit I've noted the original author in the commit
message. Thus I've left the From lines in the format-patch output as
none of the work here is mine and I don't want to assume another's
credit.

I have tried to leave the commit's in a state as close to their
original, with only enough modification to allow them to be applied to
the current HEAD of master.

The git-subtree tests work (make test), but they don't cover any of
the new commands added nor the use of the .gittrees file for storing
the subtree metadata.

They provide the following:

In git-subtree.sh:

* OPTS_SPEC
  * pull/push: options for repository and refspec are both optional,
[<repository> [<refspec>]]
  * new sub-command: pull-all
  * new sub-command: push-all
  * new sub-command: list
  * new sub-command: from-submodule
  * new sub-command: prune
  * new sub-command: diff
  * new option for push: --force
* Trailing slash on prefix is removed
* Stores subtree metadata in .gittrees as:
  [subtree "$dir"]
    url = $repository
    path = $dir
    branch = $refspec
* Replaces a non-visible carriage return character with a properly escaped one
* pull and push: reads options from .gitrees if not provided on the command line
* Implementation of diff
  Fetches remote repo as a temporary git-remote then uses
git-diff-tree to compare before removing the temporary git-remote
* Implementation of list as plain wrapper to new functions subtree_list
  Iterates over subtrees listed in .gittrees and prints out their details
* Implementation of from-submodule
  Converts a git-submodule into a git-subtree
* Implementation of prune
  Removes entries from .gittrees where the $dir is missing
* Implementation of pull-all
  Performs a git-subtree pull for each subtree
* Implementation of push-all
  Performs a git-subtree push for each subtree

In git-subtree.txt:

* Adds brief descriptions for commands:
  * pull-all
  * push-all
  * list
  * from-submodule
  * prune
  * diff ("TO BE DOCUMENTED")
* Notes optional -f|--force for push sub-command
* fixes a typo in text for Example 1 (s/incldued/included/)

-- 
Paul [W] Campbell

^ permalink raw reply

* [PATCH 01/19] spell checking
From: Paul Campbell @ 2013-03-09 19:19 UTC (permalink / raw)
  To: git
  Cc: David Michael Barr, Kindjal, bibendi, Herman van Rink,
	James Roper, mhart, mhoffman, Nate Jones, Paul Cartwright,
	Peter Jaros

>From 72fc84b6e5085b328cc90e664c9f85a1f5cde36c Mon Sep 17 00:00:00 2001
From: Paul Cartwright <paul.cartwright@ziilabs.com>
Date: Thu, 27 Jan 2011 22:33:06 +0800
Subject: [PATCH 01/19] spell checking

---
 contrib/subtree/git-subtree.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 7ba853e..e0957ee 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -270,7 +270,7 @@ git-extensions repository in ~/git-extensions/:
 name

 You can omit the --squash flag, but doing so will increase the number
-of commits that are incldued in your local repository.
+of commits that are included in your local repository.

 We now have a ~/git-extensions/git-subtree directory containing code
 from the master branch of git://github.com/apenwarr/git-subtree.git
-- 
1.8.2.rc1


-- 
Paul [W] Campbell

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox