[PATCH] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.

2013-10-11 Thread Yoshioka Tsuneo
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar, but if destination filename is long the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
This commit makes it visible like ...foo = ...bar.

Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
 diff.c | 54 +++---
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index a04a34d..9b55546 100644
--- a/diff.c
+++ b/diff.c
@@ -1643,13 +1643,53 @@ static void show_stats(struct diffstat_t *data, struct 
diff_options *options)
len = name_width;
name_len = strlen(name);
if (name_width  name_len) {
-   char *slash;
-   prefix = ...;
-   len -= 3;
-   name += name_len - len;
-   slash = strchr(name, '/');
-   if (slash)
-   name = slash;
+   char *arrow = strstr(name,  = );
+   if(arrow){
+   int prefix_len = (name_width- 4) / 2;
+   int f_omit;
+   char *pre_arrow = alloca(name_width + 10);
+   char *post_arrow = arrow + 4;
+   char *prefix_buf = alloca(name_width + 10);
+   char *pre_arrow_slash = NULL;
+
+   if(arrow - name  prefix_len){
+   prefix_len = (int)(arrow - name);
+   f_omit = 0;
+   }else{
+   prefix_len -= 3;
+   f_omit = 1;
+   }
+   strncpy(pre_arrow, arrow - prefix_len, 
prefix_len);
+   pre_arrow[prefix_len] = '¥0';
+   pre_arrow_slash = strchr(pre_arrow, '/');
+   if(f_omit  pre_arrow_slash){
+   pre_arrow = pre_arrow_slash;
+   }
+   sprintf(prefix_buf, %s%s = , (f_omit ? ... 
: ), pre_arrow);
+   prefix = prefix_buf;
+
+   if(strlen(post_arrow)  name_width - 
strlen(prefix)){
+   char *post_arrow_slash = NULL;
+
+   post_arrow += strlen(post_arrow) - 
(name_width - strlen(prefix) - 3);
+   strcat(prefix_buf, ...);
+   post_arrow_slash = strchr(post_arrow, 
'/');
+   if(post_arrow_slash){
+   post_arrow = post_arrow_slash;
+   }
+   name = post_arrow;
+   name_len = (int) (name_width - 
strlen(prefix));
+   }
+   len -= strlen(prefix);
+   }else{
+   char *slash = NULL;
+   prefix = ...;
+   len -= 3;
+   name += name_len - len;
+   slash = strchr(name, '/');
+   if (slash)
+   name = slash;
+   }
}
 
if (file-is_binary) {
-- 
1.8.4.475.g867697c


--
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: [BAD PATCH 0/9] v4-aware tree walker API

2013-10-11 Thread Duy Nguyen
On Wed, Oct 09, 2013 at 12:51:26PM -0400, Nicolas Pitre wrote:
 Now let's mitigate the deep delta chaining effect in the tree encoding:
 
 $ rm .git/objects/pack/pack-foo.*
 $ ../../git/test-packv4 --min-tree-copy=50 orig/pack-*.pack 
 .git/objects/pack/pack-foo.pack
 Scanning objects: 100% (162785/162785), done.
 Writing objects: 100% (162785/162785), done.
 $ time git rev-list --objects --all  /dev/null
 
 real0m9.451s
 user0m9.393s
 sys 0m0.036s
 
 Using --min-tree-copy=50 produces a pack which is still smaller than 
 pack v2 but any tree copy sequence must refer to a minimum of 50 
 entries.  This significantly reduces the CPU usage in decode_entries() 
 by reducing the needless chaining effect I mentioned here:
 
 http://article.gmane.org/gmane.comp.version-control.git/234975

Yeah I was frustrated and did not think about trying --min-tree-copy.

 So, there are 2 conclusions here:
 
 1: The current tree delta euristic is inefficient for pack v4.
 
 2- Something must be very wrong in your latest patches as they make it
close to 3 times more expensive than without them.

For one I know that get_tree_offset_cache() is called a lot more with
the new API. I do rev-list on v1.8.4. With compatibility layer on,
it's 211m calls (*). With new API it's 655m calls. The new API is
basically do decode_entries() on every tree_entry() call. Perhaps I
screwed up something in decode_entries() itself..

(*) for 15m tree entries, 211m are a lot of calls, which might
translate to a lot of copy sequences..

  Maybe we could make an exception and allow the tree walker to pass
  pv4_tree_cache* directly to decode_entries so it does not need to do
  the first lookup every time..
  
  Suggestions?
 
 I'll try to have a look at your patches in more details soon.

Shameful fixup (though it does not seem to impact the timing)

diff --git a/packv4-parse.c b/packv4-parse.c
index 6f6152c..9d7589e 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -825,8 +825,8 @@ static struct object **get_packed_objs(struct pv4_tree_desc 
*desc)
if (!desc-p || !desc-sha1_index)
return NULL;
if (desc-p-version = 4  !desc-p-objs)
-   desc-p-objs =
-   xmalloc(sizeof(struct object *) * desc-p-num_objects);
+   desc-p-objs = xcalloc(desc-p-num_objects,
+   sizeof(struct object *));
return desc-p-objs;
 }
 
--
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


[RFC/PATCHv3 0/3] Change git-svn's default --prefix in Git v2.0

2013-10-11 Thread Johan Herland
Hi,

Another iteration, identical to v2, except for the fixes suggested by
Eric Sunshine.

...Johan


Johan Herland (3):
  Documentation/git-svn: Promote the use of --prefix in docs + examples
  git-svn: Warn about changing default for --prefix in Git v2.0
  Git 2.0: git svn: Set default --prefix='origin/' if --prefix is not given

 Documentation/git-svn.txt| 27 ++
 git-svn.perl |  2 +-
 t/t9107-git-svn-migrate.sh   | 54 +--
 t/t9114-git-svn-dcommit-merge.sh |  4 +-
 t/t9116-git-svn-log.sh   | 46 
 t/t9117-git-svn-init-clone.sh| 67 
 t/t9118-git-svn-funky-branch-names.sh| 20 +++
 t/t9120-git-svn-clone-with-percent-escapes.sh| 14 ++---
 t/t9125-git-svn-multi-glob-branch-names.sh   |  6 +--
 t/t9128-git-svn-cmd-branch.sh| 18 +++
 t/t9135-git-svn-moved-branch-empty-file.sh   |  2 +-
 t/t9141-git-svn-multiple-branches.sh | 28 +-
 t/t9145-git-svn-master-branch.sh |  2 +-
 t/t9155-git-svn-fetch-deleted-tag.sh |  4 +-
 t/t9156-git-svn-fetch-deleted-tag-2.sh   |  6 +--
 t/t9161-git-svn-mergeinfo-push.sh| 22 
 t/t9163-git-svn-reset-clears-caches.sh   |  4 +-
 t/t9165-git-svn-fetch-merge-branch-of-branch.sh  |  2 +-
 t/t9166-git-svn-fetch-merge-branch-of-branch2.sh |  2 +-
 t/t9167-git-svn-cmd-branch-subproject.sh |  2 +-
 20 files changed, 203 insertions(+), 129 deletions(-)

-- 
1.8.4.653.g2df02b3

--
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


[RFC/PATCHv3 1/3] Documentation/git-svn: Promote the use of --prefix in docs + examples

2013-10-11 Thread Johan Herland
Currently, the git-svn defaults to using an empty prefix, which ends
up placing the SVN-tracking refs directly in refs/remotes/*. This
placement runs counter to Git's convention of placing remote-tracking
branches in refs/remotes/$remote/*.

Furthermore, combining git-svn with regular Git remotes run the risk
of clobbering refs under refs/remotes (e.g. if you have a git remote
called tags with a v1 branch, it will overlap with the git-svn's
tracking branch for the v1 tag from Subversion.

Even though the git-svn refs stored in refs/remotes/* are not proper
remote-tracking branches (since they are not covered by a proper git
remote's refspec), they clearly represent a similar concept, and would
benefit from following the same convention.

For example, if git-svn tracks Subversion branch foo at
refs/remotes/foo, and you create a local branch refs/heads/foo to add
some commits to be pushed back to Subversion (using git svn dcommit),
then it is clearly unhelpful of Git to throw

  warning: refname 'foo' is ambiguous.

every time you checkout, rebase, or otherwise interact with the branch.

At this time, the user is better off using the --prefix=foo/ (the
trailing slash is important) to git svn init/clone, to cause the
SVN-tracking refs to be placed at refs/remotes/foo/* instead of
refs/remotes/*. This patch updates the documentation to encourage
use of --prefix.

This is also in preparation for changing the default value of --prefix
at some point in the future.

Cc: Eric Wong normalper...@yhbt.net
Signed-off-by: Johan Herland jo...@herland.net
---
 Documentation/git-svn.txt | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 4dd3bcb..ac0c72f 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -79,8 +79,13 @@ COMMANDS
trailing slash, so be sure you include one in the
argument if that is what you want.  If --branches/-b is
specified, the prefix must include a trailing slash.
-   Setting a prefix is useful if you wish to track multiple
-   projects that share a common repository.
+   Setting a prefix (with a trailing slash) is strongly
+   encouraged in any case, as your SVN-tracking refs will
+   then be located at refs/remotes/$prefix/*, which is
+   compatible with Git's own remote-tracking ref layout
+   (refs/remotes/$remote/*). Setting a prefix is also useful
+   if you wish to track multiple projects that share a common
+   repository.
 --ignore-paths=regex;;
When passed to 'init' or 'clone' this regular expression will
be preserved as a config key.  See 'fetch' for a description
@@ -804,16 +809,16 @@ Tracking and contributing to an entire Subversion-managed 
project
 
 
 # Clone a repo with standard SVN directory layout (like git clone):
-   git svn clone http://svn.example.com/project --stdlayout
+   git svn clone http://svn.example.com/project --stdlayout --prefix svn/
 # Or, if the repo uses a non-standard directory layout:
-   git svn clone http://svn.example.com/project -T tr -b branch -t tag
+   git svn clone http://svn.example.com/project -T tr -b branch -t tag 
--prefix svn/
 # View all branches and tags you have cloned:
git branch -r
 # Create a new branch in SVN
 git svn branch waldo
 # Reset your master to trunk (or any other branch, replacing 'trunk'
 # with the appropriate name):
-   git reset --hard remotes/trunk
+   git reset --hard svn/trunk
 # You may only dcommit to one branch/tag/trunk at a time.  The usage
 # of dcommit/rebase/show-ignore should be the same as above.
 
@@ -827,7 +832,7 @@ have each person clone that repository with 'git clone':
 
 
 # Do the initial import on a server
-   ssh server cd /pub  git svn clone http://svn.example.com/project
+   ssh server cd /pub  git svn clone http://svn.example.com/project 
[options...]
 # Clone locally - make sure the refs/remotes/ space matches the server
mkdir project
cd project
@@ -840,8 +845,9 @@ have each person clone that repository with 'git clone':
git config --remove-section remote.origin
 # Create a local branch from one of the branches just fetched
git checkout -b master FETCH_HEAD
-# Initialize 'git svn' locally (be sure to use the same URL and -T/-b/-t 
options as were used on server)
-   git svn init http://svn.example.com/project
+# Initialize 'git svn' locally (be sure to use the same URL and
+# --stdlayout/-T/-b/-t/--prefix options as were used on server)
+   git svn init http://svn.example.com/project [options...]
 # Pull the latest changes from Subversion
git svn rebase
 

[RFC/PATCHv3 3/3] Git 2.0: git svn: Set default --prefix='origin/' if --prefix is not given

2013-10-11 Thread Johan Herland
git-svn by default puts its Subversion-tracking refs directly in
refs/remotes/*. This runs counter to Git's convention of using
refs/remotes/$remote/* for storing remote-tracking branches.

Furthermore, combining git-svn with regular git remotes run the risk of
clobbering refs under refs/remotes (e.g. if you have a git remote
called tags with a v1 branch, it will overlap with the git-svn's
tracking branch for the v1 tag from Subversion.

Even though the git-svn refs stored in refs/remotes/* are not proper
remote-tracking branches (since they are not covered by a proper git
remote's refspec), they clearly represent a similar concept, and would
benefit from following the same convention.

For example, if git-svn tracks Subversion branch foo at
refs/remotes/foo, and you create a local branch refs/heads/foo to add
some commits to be pushed back to Subversion (using git svn dcommit),
then it is clearly unhelpful of Git to throw

  warning: refname 'foo' is ambiguous.

every time you checkout, rebase, or otherwise interact with the branch.

The existing workaround for this is to supply the --prefix=quux/ to
git svn init/clone, so that git-svn's tracking branches end up in
refs/remotes/quux/* instead of refs/remotes/*. However, encouraging
users to specify --prefix to work around a design flaw in git-svn is
suboptimal, and not a long term solution to the problem. Instead,
git-svn should default to use a non-empty prefix that saves
unsuspecting users from the inconveniences described above.

This patch will only affect newly created git-svn setups, as the
--prefix option only applies to git svn init (and git svn clone).
Existing git-svn setups will continue with their existing (lack of)
prefix. Also, if anyone somehow prefers git-svn's old layout, they
can recreate that by explicitly passing an empty prefix (--prefix )
on the git svn init/clone command line.

The patch changes the default value for --prefix from  to origin/,
updates the git-svn manual page, and fixes the fallout in the git-svn
testcases.

(Note that this patch might be easier to review using the --word-diff
and --word-diff-regex=. diff options.)

Suggested-by: Thomas Ferris Nicolaisen tfn...@gmail.com
Cc: Eric Wong normalper...@yhbt.net
Signed-off-by: Johan Herland jo...@herland.net
---
 Documentation/git-svn.txt| 19 +
 git-svn.perl | 12 +-
 t/t9107-git-svn-migrate.sh   | 54 
 t/t9114-git-svn-dcommit-merge.sh |  4 +-
 t/t9116-git-svn-log.sh   | 46 ++--
 t/t9117-git-svn-init-clone.sh| 16 +++
 t/t9118-git-svn-funky-branch-names.sh| 20 -
 t/t9120-git-svn-clone-with-percent-escapes.sh| 14 +++---
 t/t9125-git-svn-multi-glob-branch-names.sh   |  6 +--
 t/t9128-git-svn-cmd-branch.sh| 18 
 t/t9135-git-svn-moved-branch-empty-file.sh   |  2 +-
 t/t9141-git-svn-multiple-branches.sh | 28 ++--
 t/t9145-git-svn-master-branch.sh |  2 +-
 t/t9155-git-svn-fetch-deleted-tag.sh |  4 +-
 t/t9156-git-svn-fetch-deleted-tag-2.sh   |  6 +--
 t/t9161-git-svn-mergeinfo-push.sh| 22 +-
 t/t9163-git-svn-reset-clears-caches.sh   |  4 +-
 t/t9165-git-svn-fetch-merge-branch-of-branch.sh  |  2 +-
 t/t9166-git-svn-fetch-merge-branch-of-branch2.sh |  2 +-
 t/t9167-git-svn-cmd-branch-subproject.sh |  2 +-
 20 files changed, 128 insertions(+), 155 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 7980c20..43da05a 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -86,14 +86,7 @@ COMMANDS
(refs/remotes/$remote/*). Setting a prefix is also useful
if you wish to track multiple projects that share a common
repository.
-+
-NOTE: In Git v2.0, the default prefix will CHANGE from  (no prefix)
-to origin/. This is done to put SVN-tracking refs at
-refs/remotes/origin/* instead of refs/remotes/*, and make them
-more compatible with how Git's own remote-tracking refs are organized
-(i.e. refs/remotes/$remote/*). You can enjoy the same benefits today,
-by using the --prefix option.
-
+   By default, the prefix is set to 'origin/'.
 --ignore-paths=regex;;
When passed to 'init' or 'clone' this regular expression will
be preserved as a config key.  See 'fetch' for a description
@@ -987,16 +980,6 @@ without giving any repository layout options.  If the full 
history with
 branches and tags is required, the options '--trunk' / '--branches' /
 '--tags' must be used.
 
-When using the options for describing the repository layout (--trunk,
---tags, --branches, --stdlayout), please also specify the --prefix
-option (e.g. '--prefix=origin/') to cause your SVN-tracking refs to be
-placed at refs/remotes/origin/* rather than the default refs/remotes/*.
-The 

[RFC/PATCHv3 2/3] git-svn: Warn about changing default for --prefix in Git v2.0

2013-10-11 Thread Johan Herland
In Git v2.0, we will change the default --prefix for init/clone from
none/empty to origin/ (which causes SVN-tracking branches to be
placed at refs/remotes/origin/* instead of refs/remotes/*).

This patch warns users about the upcoming change, both in the git-svn
manual page, and on stderr when running init/clone in the multi-mode
without providing a --prefix.

Cc: Eric Wong normalper...@yhbt.net
Signed-off-by: Johan Herland jo...@herland.net
---
 Documentation/git-svn.txt | 11 ++-
 git-svn.perl  | 12 +++-
 t/t9117-git-svn-init-clone.sh | 67 +++
 3 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index ac0c72f..7980c20 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -86,6 +86,14 @@ COMMANDS
(refs/remotes/$remote/*). Setting a prefix is also useful
if you wish to track multiple projects that share a common
repository.
++
+NOTE: In Git v2.0, the default prefix will CHANGE from  (no prefix)
+to origin/. This is done to put SVN-tracking refs at
+refs/remotes/origin/* instead of refs/remotes/*, and make them
+more compatible with how Git's own remote-tracking refs are organized
+(i.e. refs/remotes/$remote/*). You can enjoy the same benefits today,
+by using the --prefix option.
+
 --ignore-paths=regex;;
When passed to 'init' or 'clone' this regular expression will
be preserved as a config key.  See 'fetch' for a description
@@ -986,7 +994,8 @@ placed at refs/remotes/origin/* rather than the default 
refs/remotes/*.
 The former is more compatible with the layout of Git's regular
 remote-tracking refs (refs/remotes/$remote/*), and may potentially
 prevent similarly named SVN branches and Git remotes from clobbering
-each other.
+each other. In Git v2.0 the default prefix used (i.e. when no --prefix
+is given) will change from  (no prefix) to origin/.
 
 When using multiple --branches or --tags, 'git svn' does not automatically
 handle name collisions (for example, if two branches from different paths have
diff --git a/git-svn.perl b/git-svn.perl
index ff1ce3d..0443a4f 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1389,7 +1389,17 @@ sub cmd_multi_init {
usage(1);
}
 
-   $_prefix = '' unless defined $_prefix;
+   unless (defined $_prefix) {
+   $_prefix = '';
+   warn EOF
+WARNING: --prefix is not given, defaulting to empty prefix.
+ This is probably not what you want! In order to stay compatible
+ with regular remote-tracking refs, provide a prefix like
+ --prefix=origin/ (remember the trailing slash), which will cause
+ the SVN-tracking refs to be placed at refs/remotes/origin/*.
+NOTE: In Git v2.0, the default prefix will change from empty to 'origin/'.
+EOF
+   }
if (defined $url) {
$url = canonicalize_url($url);
init_subdir(@_);
diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
index b7ef9e2..69e9c0d 100755
--- a/t/t9117-git-svn-init-clone.sh
+++ b/t/t9117-git-svn-init-clone.sh
@@ -52,4 +52,71 @@ test_expect_success 'clone to target directory with 
--stdlayout' '
rm -rf target
'
 
+test_expect_success 'init without -s/-T/-b/-t does not warn' '
+   test ! -d trunk 
+   git svn init $svnrepo/project/trunk trunk 2warning 
+   test_must_fail grep -q prefix warning 
+   rm -rf trunk 
+   rm -f warning
+   '
+
+test_expect_success 'clone without -s/-T/-b/-t does not warn' '
+   test ! -d trunk 
+   git svn clone $svnrepo/project/trunk 2warning 
+   test_must_fail grep -q prefix warning 
+   rm -rf trunk 
+   rm -f warning
+   '
+
+test_svn_configured_prefix () {
+   prefix=$1 
+   cat expect EOF 
+project/trunk:refs/remotes/${prefix}trunk
+project/branches/*:refs/remotes/${prefix}*
+project/tags/*:refs/remotes/${prefix}tags/*
+EOF
+   test ! -f actual 
+   git --git-dir=project/.git config svn-remote.svn.fetch actual 
+   git --git-dir=project/.git config svn-remote.svn.branches actual 
+   git --git-dir=project/.git config svn-remote.svn.tags actual 
+   test_cmp expect actual 
+   rm -f expect actual
+}
+
+test_expect_success 'init with -s/-T/-b/-t without --prefix warns' '
+   test ! -d project 
+   git svn init -s $svnrepo/project project 2warning 
+   grep -q prefix warning 
+   test_svn_configured_prefix  
+   rm -rf project 
+   rm -f warning
+   '
+
+test_expect_success 'clone with -s/-T/-b/-t without --prefix warns' '
+   test ! -d project 
+   git svn clone -s $svnrepo/project 2warning 
+   grep -q prefix warning 
+   test_svn_configured_prefix  
+   rm -rf project 
+   rm -f warning
+   '
+
+test_expect_success 'init with -s/-T/-b/-t and --prefix does not warn' '
+   test ! -d project 
+   

Re: [BAD PATCH 0/9] v4-aware tree walker API

2013-10-11 Thread Duy Nguyen
On Fri, Oct 11, 2013 at 07:22:59PM +0700, Duy Nguyen wrote:
   Maybe we could make an exception and allow the tree walker to pass
   pv4_tree_cache* directly to decode_entries so it does not need to do
   the first lookup every time..
   
   Suggestions?

Looking at decode_entries() traces I think the one decode_entries()
for one tree_entry() just amplifies the delta chain effect. If you
hide 3 entries behind 5 layers of copy sequences
(i.e. tree1-tree2-..-tree5-real-tree-entry), then every
decode_entries(count=1) will have to go through 5 layers.

It makes me wonder if we should cache shortcuts so that after the
first going through 5 layers, the second can jump directly to the tree
entries.

  
  I'll try to have a look at your patches in more details soon.
 
 Shameful fixup (though it does not seem to impact the timing)

And here's another one

-- 8 --
diff --git a/list-objects.c b/list-objects.c
index 39ad3e6..85dc14e 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -82,8 +82,10 @@ static void process_tree(struct rev_info *revs,
die(bad tree object);
if (obj-flags  (UNINTERESTING | SEEN))
return;
+#if 0
if (parse_tree(tree)  0)
die(bad tree object %s, sha1_to_hex(obj-sha1));
+#endif
obj-flags |= SEEN;
show(obj, path, name, cb_data);
me.up = path;
--
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 v2] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.

2013-10-11 Thread Yoshioka Tsuneo
git diff -M --stat can detect rename and show renamed file name like
foofoofoo = barbarbar, but if destination filename is long the line
is shortened like ...barbarbar so there is no way to know whether the
file is renamed or existed in the source commit.
This commit makes it visible like ...foo = ...bar.

Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
 diff.c | 58 +++---
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index a04a34d..3aeaf3e 100644
--- a/diff.c
+++ b/diff.c
@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data, struct 
diff_options *options)
len = name_width;
name_len = strlen(name);
if (name_width  name_len) {
-   char *slash;
-   prefix = ...;
-   len -= 3;
-   name += name_len - len;
-   slash = strchr(name, '/');
-   if (slash)
-   name = slash;
+   char *arrow = strstr(name,  = );
+   if (arrow) {
+   int prefix_len = (name_width - 4) / 2;
+   int f_omit;
+   int f_brace = 0;
+   char *pre_arrow = alloca(name_width + 10);
+   char *post_arrow = arrow + 4;
+   char *prefix_buf = alloca(name_width + 10);
+   char *pre_arrow_slash = NULL;
+
+   if (arrow - name  prefix_len) {
+   prefix_len = (int)(arrow - name);
+   f_omit = 0;
+   } else {
+   prefix_len -= 3;
+   f_omit = 1;
+   if (name[0] == '{') {
+   prefix_len -= 1;
+   f_brace = 1;
+   }
+   }
+   prefix_len = ((prefix_len = 0) ? prefix_len : 
0);
+   strncpy(pre_arrow, arrow - prefix_len, 
prefix_len);
+   pre_arrow[prefix_len] = '¥0';
+   pre_arrow_slash = strchr(pre_arrow, '/');
+   if (f_omit  pre_arrow_slash)
+   pre_arrow = pre_arrow_slash;
+   sprintf(prefix_buf, %s%s%s = , (f_brace ? 
{ : ), (f_omit ? ... : ), pre_arrow);
+   prefix = prefix_buf;
+
+   if (strlen(post_arrow)  name_width - 
strlen(prefix)) {
+   char *post_arrow_slash = NULL;
+
+   post_arrow += strlen(post_arrow) - 
(name_width - strlen(prefix) - 3);
+   strcat(prefix_buf, ...);
+   post_arrow_slash = strchr(post_arrow, 
'/');
+   if (post_arrow_slash)
+   post_arrow = post_arrow_slash;
+   name = post_arrow;
+   name_len = (int) (name_width - 
strlen(prefix));
+   }
+   len -= strlen(prefix);
+   } else {
+   char *slash = NULL;
+   prefix = ...;
+   len -= 3;
+   name += name_len - len;
+   slash = strchr(name, '/');
+   if (slash)
+   name = slash;
+   }
}
 
if (file-is_binary) {
-- 
1.8.4.475.g867697c



--
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: A workflow for local patch maintenance

2013-10-11 Thread Stephen Bash
- Original Message -
 From: Jeff King p...@peff.net
 Sent: Thursday, October 10, 2013 1:36:28 PM
 Subject: Re: A workflow for local patch maintenance
 
 ... snip ...

 That being said, there are some new downsides, as you noted:
 
   1. Resolving conflicts between your version and the reworked
   upstream version can be a pain.
 
 ... snip ...
 
 To mitigate problem 1, I will sometimes revert a local topic before
 doing the upstream merge, if I know it has been reworked.

Peff (slightly off topic) - A coworker of mine actually ran into this
problem earlier this week.  Is there recommended way to revert a merged
topic branch?  I assume it's essentially reverted each commit introduced
by the branch, but is there a convenient invocation of revert? (easy to 
remember and hard to screw up)

Thanks,
Stephen

--
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


Spurious warning when moving a file in presence of submodules

2013-10-11 Thread Matthieu Moy
Hi,

I'm getting this warning:

  warning: Could not find section in .gitmodules where path=XXX

whenever I use git mv to move a file in a repository containing a
submodule. The file is outside the submodule and is completely
unrelated, so I do not understand the intent of the warning.

My understanding (without looking at the code in detail) is that Git
tries to be clever about submodule renames, hence checks whether the
source file is a submodule. But then if the lookup fails, it should just
silently move on to normal file move mode I guess...

-- 
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: A workflow for local patch maintenance

2013-10-11 Thread Jeff King
On Fri, Oct 11, 2013 at 09:22:28AM -0400, Stephen Bash wrote:

  To mitigate problem 1, I will sometimes revert a local topic before
  doing the upstream merge, if I know it has been reworked.
 
 Peff (slightly off topic) - A coworker of mine actually ran into this
 problem earlier this week.  Is there recommended way to revert a merged
 topic branch?  I assume it's essentially reverted each commit introduced
 by the branch, but is there a convenient invocation of revert? (easy to 
 remember and hard to screw up)

If you merged the whole topic in at once, then you can use git revert
-m 1 $merge_commit to undo the merge. If it came in individual pieces,
then you have to revert each one individually (though if it was a series
of merges, you can in theory revert each merge in reverse order).

-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] status: show commit sha1 in You are currently cherry-picking message

2013-10-11 Thread Ralf Thielow
Signed-off-by: Ralf Thielow ralf.thie...@gmail.com
---

Especially helpful when cherry-picking multiple commits.

 t/t7512-status-help.sh | 10 ++
 wt-status.c|  7 +--
 wt-status.h|  1 +
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 0688d58..0a65db1 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -626,9 +626,10 @@ test_expect_success 'prepare for cherry-pick conflicts' '
 test_expect_success 'status when cherry-picking before resolving conflicts' '
test_when_finished git cherry-pick --abort 
test_must_fail git cherry-pick cherry_branch_second 
-   cat expected \EOF 
+   TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) 
+   cat expected EOF
 On branch cherry_branch
-You are currently cherry-picking.
+You are currently cherry-picking commit $TO_CHERRY_PICK.
   (fix conflicts and run git cherry-pick --continue)
   (use git cherry-pick --abort to cancel the cherry-pick operation)
 
@@ -648,11 +649,12 @@ test_expect_success 'status when cherry-picking after 
resolving conflicts' '
git reset --hard cherry_branch 
test_when_finished git cherry-pick --abort 
test_must_fail git cherry-pick cherry_branch_second 
+   TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) 
echo end main.txt 
git add main.txt 
-   cat expected \EOF 
+   cat expected EOF
 On branch cherry_branch
-You are currently cherry-picking.
+You are currently cherry-picking commit $TO_CHERRY_PICK.
   (all conflicts fixed: run git cherry-pick --continue)
   (use git cherry-pick --abort to cancel the cherry-pick operation)
 
diff --git a/wt-status.c b/wt-status.c
index cbdce72..b4e44ba 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -996,7 +996,8 @@ static void show_cherry_pick_in_progress(struct wt_status 
*s,
struct wt_status_state *state,
const char *color)
 {
-   status_printf_ln(s, color, _(You are currently cherry-picking.));
+   status_printf_ln(s, color, _(You are currently cherry-picking commit 
%s.),
+   find_unique_abbrev(state-cherry_pick_head_sha1, 
DEFAULT_ABBREV));
if (s-hints) {
if (has_unmerged(s))
status_printf_ln(s, color,
@@ -1169,8 +1170,10 @@ void wt_status_get_state(struct wt_status_state *state,
state-rebase_in_progress = 1;
state-branch = read_and_strip_branch(rebase-merge/head-name);
state-onto = read_and_strip_branch(rebase-merge/onto);
-   } else if (!stat(git_path(CHERRY_PICK_HEAD), st)) {
+   } else if (!stat(git_path(CHERRY_PICK_HEAD), st) 
+   !get_sha1(CHERRY_PICK_HEAD, sha1)) {
state-cherry_pick_in_progress = 1;
+   hashcpy(state-cherry_pick_head_sha1, sha1);
}
if (!stat(git_path(BISECT_LOG), st)) {
state-bisect_in_progress = 1;
diff --git a/wt-status.h b/wt-status.h
index 9341c56..6c29e6f 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -88,6 +88,7 @@ struct wt_status_state {
char *detached_from;
unsigned char detached_sha1[20];
unsigned char revert_head_sha1[20];
+   unsigned char cherry_pick_head_sha1[20];
 };
 
 void wt_status_prepare(struct wt_status *s);
-- 
1.8.4.652.g0d6e0ce

--
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] status: show commit sha1 in You are currently cherry-picking message

2013-10-11 Thread Matthieu Moy
Ralf Thielow ralf.thie...@gmail.com writes:

 Especially helpful when cherry-picking multiple commits.

I think this would deserve to be in the commit message (but don't
consider that blocking).

Other than that, looks good to me.

-- 
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


[PATCH] clone --branch: refuse to clone if upstream repo is empty

2013-10-11 Thread Ralf Thielow
Since 920b691 (clone: refuse to clone if --branch
points to bogus ref) we refuse to clone with option
-b if the specified branch does not exist in the
(non-empty) upstream. If the upstream repository is empty,
the branch doesn't exist, either. So refuse the clone too.

Signed-off-by: Ralf Thielow ralf.thie...@gmail.com
---
 builtin/clone.c | 4 
 t/t5706-clone-branch.sh | 8 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index ca3eb68..5af386e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -945,6 +945,10 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
our_head_points_at = remote_head_points_at;
}
else {
+   if (option_branch)
+   die(_(Remote branch %s not found in upstream %s),
+   option_branch, option_origin);
+
warning(_(You appear to have cloned an empty repository.));
mapped_refs = NULL;
our_head_points_at = NULL;
diff --git a/t/t5706-clone-branch.sh b/t/t5706-clone-branch.sh
index 56be67e..6e7a7be 100755
--- a/t/t5706-clone-branch.sh
+++ b/t/t5706-clone-branch.sh
@@ -20,7 +20,9 @@ test_expect_success 'setup' '
 echo one file  git add file  git commit -m one 
 git checkout -b two 
 echo two file  git add file  git commit -m two 
-git checkout master)
+git checkout master) 
+   mkdir empty 
+   (cd empty  git init)
 '
 
 test_expect_success 'vanilla clone chooses HEAD' '
@@ -61,4 +63,8 @@ test_expect_success 'clone -b with bogus branch' '
test_must_fail git clone -b bogus parent clone-bogus
 '
 
+test_expect_success 'clone -b not allowed with empty repos' '
+   test_must_fail git clone -b branch empty clone-branch-empty
+'
+
 test_done
-- 
1.8.4.652.g0d6e0ce

--
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] status: show commit sha1 in You are currently cherry-picking message

2013-10-11 Thread Jonathan Nieder
Ralf Thielow wrote:

 Especially helpful when cherry-picking multiple commits.

Neat, thanks.

[...]
 --- a/t/t7512-status-help.sh
 +++ b/t/t7512-status-help.sh
 @@ -626,9 +626,10 @@ test_expect_success 'prepare for cherry-pick conflicts' '
  test_expect_success 'status when cherry-picking before resolving conflicts' '
   test_when_finished git cherry-pick --abort 
   test_must_fail git cherry-pick cherry_branch_second 
 + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) 
 - cat expected \EOF 
 + cat expected EOF

Did you mean to drop the ''?

[...]
 @@ -648,11 +649,12 @@ test_expect_success 'status when cherry-picking after 
 resolving conflicts' '
   git reset --hard cherry_branch 
   test_when_finished git cherry-pick --abort 
   test_must_fail git cherry-pick cherry_branch_second 
 + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) 
   echo end main.txt 
   git add main.txt 
 - cat expected \EOF 
 + cat expected EOF

Likewise.

[...]
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -996,7 +996,8 @@ static void show_cherry_pick_in_progress(struct wt_status 
 *s,
   struct wt_status_state *state,
   const char *color)
  {
 - status_printf_ln(s, color, _(You are currently cherry-picking.));
 + status_printf_ln(s, color, _(You are currently cherry-picking commit 
 %s.),
 + find_unique_abbrev(state-cherry_pick_head_sha1, 
 DEFAULT_ABBREV));

This function is only called when -cherry_pick_in_progress is true, so
we know cherry_pick_head_sha1 is initialized.  Good.

I would be tempted to check anyway, so that if we ever regress in this,
the cause will be clear and users know to report a bug:

if (is_null_sha1(state-cherry_pick_head_sha1))
die(BUG: cherry-pick in progress but no valid 
CHERRY_PICK_HEAD?);
status_printf_ln(s, color, _(You are ...

I dunno.

Applied with the  fixes mentioned above on top of the following.

-- 8 --
Subject: status test: add missing  to EOF blocks

When a test forgets to include  after each command, it is possible
for an early command to succeed but the test to fail, which can hide
bugs.

Checked using the following patch to the test harness:

--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -425,7 +425,17 @@ test_eval_ () {
eval /dev/null 3 24 $*
 }

+check_command_chaining_ () {
+   eval 3 24 (exit 189)  $*
+   eval_chain_ret=$?
+   if test $eval_chain_ret != 189
+   then
+   error 'bug in test script: missing  in test 
commands'
+   fi
+}
+
 test_run_ () {
+   check_command_chaining_ $1
test_cleanup=:
expecting_failure=$2
setup_malloc_check

Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
 t/t7512-status-help.sh | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 0688d58..9905d43 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -669,7 +669,7 @@ EOF
 test_expect_success 'status showing detached at and from a tag' '
test_commit atag tagging 
git checkout atag 
-   cat expected \EOF
+   cat expected \EOF 
 HEAD detached at atag
 nothing to commit (use -u to show untracked files)
 EOF
@@ -677,7 +677,7 @@ EOF
test_i18ncmp expected actual 
 
git reset --hard HEAD^ 
-   cat expected \EOF
+   cat expected \EOF 
 HEAD detached from atag
 nothing to commit (use -u to show untracked files)
 EOF
@@ -695,7 +695,7 @@ test_expect_success 'status while reverting commit 
(conflicts)' '
test_commit new to-revert.txt 
TO_REVERT=$(git rev-parse --short HEAD^) 
test_must_fail git revert $TO_REVERT 
-   cat expected EOF
+   cat expected EOF 
 On branch master
 You are currently reverting commit $TO_REVERT.
   (fix conflicts and run git revert --continue)
@@ -716,7 +716,7 @@ EOF
 test_expect_success 'status while reverting commit (conflicts resolved)' '
echo reverted to-revert.txt 
git add to-revert.txt 
-   cat expected EOF
+   cat expected EOF 
 On branch master
 You are currently reverting commit $TO_REVERT.
   (all conflicts fixed: run git revert --continue)
@@ -735,7 +735,7 @@ EOF
 
 test_expect_success 'status after reverting commit' '
git revert --continue 
-   cat expected \EOF
+   cat expected \EOF 
 On branch master
 nothing to commit (use -u to show untracked files)
 EOF
-- 
1.8.4-50-g437ce60

--
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: Spurious warning when moving a file in presence of submodules

2013-10-11 Thread Jens Lehmann
Hi Matthieu,

Am 11.10.2013 16:29, schrieb Matthieu Moy:
 I'm getting this warning:
 
   warning: Could not find section in .gitmodules where path=XXX
 
 whenever I use git mv to move a file in a repository containing a
 submodule. The file is outside the submodule and is completely
 unrelated, so I do not understand the intent of the warning.
 
 My understanding (without looking at the code in detail) is that Git
 tries to be clever about submodule renames, hence checks whether the
 source file is a submodule. But then if the lookup fails, it should just
 silently move on to normal file move mode I guess...

Right. Thanks for reporting, I can reproduce that here and am currently
looking into that.
--
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] status: show commit sha1 in You are currently cherry-picking message

2013-10-11 Thread Ralf Thielow
On Fri, Oct 11, 2013 at 7:42 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ralf Thielow wrote:

 Especially helpful when cherry-picking multiple commits.

 Neat, thanks.

 [...]
 --- a/t/t7512-status-help.sh
 +++ b/t/t7512-status-help.sh
 @@ -626,9 +626,10 @@ test_expect_success 'prepare for cherry-pick conflicts' 
 '
  test_expect_success 'status when cherry-picking before resolving conflicts' 
 '
   test_when_finished git cherry-pick --abort 
   test_must_fail git cherry-pick cherry_branch_second 
 + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) 
 - cat expected \EOF 
 + cat expected EOF

 Did you mean to drop the ''?


No. The important thing was actually the \ character. Sry

 [...]
 @@ -648,11 +649,12 @@ test_expect_success 'status when cherry-picking after 
 resolving conflicts' '
   git reset --hard cherry_branch 
   test_when_finished git cherry-pick --abort 
   test_must_fail git cherry-pick cherry_branch_second 
 + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) 
   echo end main.txt 
   git add main.txt 
 - cat expected \EOF 
 + cat expected EOF

 Likewise.

 [...]
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -996,7 +996,8 @@ static void show_cherry_pick_in_progress(struct 
 wt_status *s,
   struct wt_status_state *state,
   const char *color)
  {
 - status_printf_ln(s, color, _(You are currently cherry-picking.));
 + status_printf_ln(s, color, _(You are currently cherry-picking commit 
 %s.),
 + find_unique_abbrev(state-cherry_pick_head_sha1, 
 DEFAULT_ABBREV));

 This function is only called when -cherry_pick_in_progress is true, so
 we know cherry_pick_head_sha1 is initialized.  Good.

 I would be tempted to check anyway, so that if we ever regress in this,
 the cause will be clear and users know to report a bug:

 if (is_null_sha1(state-cherry_pick_head_sha1))
 die(BUG: cherry-pick in progress but no valid 
 CHERRY_PICK_HEAD?);
 status_printf_ln(s, color, _(You are ...

 I dunno.

 Applied with the  fixes mentioned above on top of the following.


Thanks

 -- 8 --
 Subject: status test: add missing  to EOF blocks

 When a test forgets to include  after each command, it is possible
 for an early command to succeed but the test to fail, which can hide
 bugs.

 Checked using the following patch to the test harness:

 --- a/t/test-lib.sh
 +++ b/t/test-lib.sh
 @@ -425,7 +425,17 @@ test_eval_ () {
 eval /dev/null 3 24 $*
  }

 +check_command_chaining_ () {
 +   eval 3 24 (exit 189)  $*
 +   eval_chain_ret=$?
 +   if test $eval_chain_ret != 189
 +   then
 +   error 'bug in test script: missing  in test 
 commands'
 +   fi
 +}
 +
  test_run_ () {
 +   check_command_chaining_ $1
 test_cleanup=:
 expecting_failure=$2
 setup_malloc_check

 Signed-off-by: Jonathan Nieder jrnie...@gmail.com
 ---
  t/t7512-status-help.sh | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

 diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
 index 0688d58..9905d43 100755
 --- a/t/t7512-status-help.sh
 +++ b/t/t7512-status-help.sh
 @@ -669,7 +669,7 @@ EOF
  test_expect_success 'status showing detached at and from a tag' '
 test_commit atag tagging 
 git checkout atag 
 -   cat expected \EOF
 +   cat expected \EOF 
  HEAD detached at atag
  nothing to commit (use -u to show untracked files)
  EOF
 @@ -677,7 +677,7 @@ EOF
 test_i18ncmp expected actual 

 git reset --hard HEAD^ 
 -   cat expected \EOF
 +   cat expected \EOF 
  HEAD detached from atag
  nothing to commit (use -u to show untracked files)
  EOF
 @@ -695,7 +695,7 @@ test_expect_success 'status while reverting commit 
 (conflicts)' '
 test_commit new to-revert.txt 
 TO_REVERT=$(git rev-parse --short HEAD^) 
 test_must_fail git revert $TO_REVERT 
 -   cat expected EOF
 +   cat expected EOF 
  On branch master
  You are currently reverting commit $TO_REVERT.
(fix conflicts and run git revert --continue)
 @@ -716,7 +716,7 @@ EOF
  test_expect_success 'status while reverting commit (conflicts resolved)' '
 echo reverted to-revert.txt 
 git add to-revert.txt 
 -   cat expected EOF
 +   cat expected EOF 
  On branch master
  You are currently reverting commit $TO_REVERT.
(all conflicts fixed: run git revert --continue)
 @@ -735,7 +735,7 @@ EOF

  test_expect_success 'status after reverting commit' '
 git revert --continue 
 -   cat expected \EOF
 +   cat expected \EOF 
  On branch master
  nothing to commit (use -u to show untracked files)
  EOF
 --
 1.8.4-50-g437ce60

--
To unsubscribe from this list: 

[PATCH] howto/revert-a-faulty-merge.txt: Fix asciidoc formatting

2013-10-11 Thread Ramsay Jones

Several uses of the '^' operator are being interpreted by asciidoc
as requests to show the following text as a superscript. In order
to fix this problem, use backticks (`) to quote the text of the
affected git command invocations.

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---

Hi Jonathan,

Just noticed this in passing ... I don't know asciidoc that well, so
if there is a better way to fix this, just let me know. ;-)

ATB,
Ramsay Jones

 Documentation/howto/revert-a-faulty-merge.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/howto/revert-a-faulty-merge.txt 
b/Documentation/howto/revert-a-faulty-merge.txt
index 075418e..acf3e47 100644
--- a/Documentation/howto/revert-a-faulty-merge.txt
+++ b/Documentation/howto/revert-a-faulty-merge.txt
@@ -37,7 +37,7 @@ where A and B are on the side development that was not so 
good, M is the
 merge that brings these premature changes into the mainline, x are changes
 unrelated to what the side branch did and already made on the mainline,
 and W is the revert of the merge M (doesn't W look M upside down?).
-IOW, diff W^..W is similar to diff -R M^..M.
+IOW, `diff W^..W` is similar to `diff -R M^..M`.
 
 Such a revert of a merge can be made with:
 
@@ -121,9 +121,9 @@ If you reverted the revert in such a case as in the 
previous example:
---A---B   A'--B'--C'
 
 where Y is the revert of W, A' and B' are rerolled A and B, and there may
-also be a further fix-up C' on the side branch.  diff Y^..Y is similar
-to diff -R W^..W (which in turn means it is similar to diff M^..M),
-and diff A'^..C' by definition would be similar but different from that,
+also be a further fix-up C' on the side branch.  `diff Y^..Y` is similar
+to `diff -R W^..W` (which in turn means it is similar to `diff M^..M`),
+and `diff A'^..C'` by definition would be similar but different from that,
 because it is a rerolled series of the earlier change.  There will be a
 lot of overlapping changes that result in conflicts.  So do not do revert
 of revert blindly without thinking..
-- 
1.8.4
--
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: [spf:guess,mismatch] [PATCH v2] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.

2013-10-11 Thread Sam Vilain
On 10/11/2013 06:07 AM, Yoshioka Tsuneo wrote:
 + prefix_len = ((prefix_len = 0) ? prefix_len : 
 0);
 + strncpy(pre_arrow, arrow - prefix_len, 
 prefix_len);
 + pre_arrow[prefix_len] = '¥0';


This seems to be an encoding mistake; was this supposed to be an ASCII
arrow?

Sam

--
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


BUG - git clean

2013-10-11 Thread jones.noamle
Passing to git clean wrong (non-existent) paths together with valid 
ones, causes it to delete stuff that it shouldn't.

Am I right?
Script to reproduce:

mkdir test
cd test
git init .
mkdir ba
mkdir ba/ca

# So far so good.
# Should clean directory ba/ca
git clean -dn -- ba/ca

# Should clean ba/ca and ignore non-existent j
# Instead, it wants to delete ba totally.
git clean -dn -- ba/ca j

git --version

-
Output:

+ mkdir test
+ cd test
+ git init .
Initialized empty Git repository in /home/notroot/test/.git/
+ mkdir ba
+ mkdir ba/ca
+ git clean -dn -- ba/ca
Would remove ba/ca/
+ git clean -dn -- ba/ca j
Would remove ba/
+ git --version
git version 1.7.9.5

-

Thanks!
Noam
--
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


Bug in git clean

2013-10-11 Thread jones.noa...@gmail.com
Hi.

Passing to git clean wrong (non-existent) paths together with valid
ones, causes it to delete stuff that it shouldn't.
Am I right?
Script to reproduce:

mkdir test
cd test
git init .
mkdir ba
mkdir ba/ca

# So far so good.
# Should clean directory ba/ca
git clean -dn -- ba/ca

# Should clean ba/ca and ignore non-existent j
# Instead, it wants to delete ba totally.
git clean -dn -- ba/ca j

git --version

-
Output:

+ mkdir test
+ cd test
+ git init .
Initialized empty Git repository in /home/notroot/test/.git/
+ mkdir ba
+ mkdir ba/ca
+ git clean -dn -- ba/ca
Would remove ba/ca/
+ git clean -dn -- ba/ca j
Would remove ba/
+ git --version
git version 1.7.9.5

-

Tested on linux (version 1.7.9.5) and cygwin (1.7.9)

Thanks!
Noam Lewis
Hadas Nahon
--
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] howto/setup-git-server-over-http.txt: Fix asciidoc formatting

2013-10-11 Thread Ramsay Jones

The text contains two 'grep' invocations which include the 'start
of line' regular expression character '^'. Asciidoc mis-interprets
this use of '^' as a superscript request. In order to fix this
formatting problem, use backticks (`) to quote the text of the
affected 'grep' command invocations.

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---

Hi Jonathan,

$ git grep '\^' Documentation/howto

pointed me to some other candidates, but only this one needed a
similar fix ... :-D

ATB,
Ramsay Jones

 Documentation/howto/setup-git-server-over-http.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/howto/setup-git-server-over-http.txt 
b/Documentation/howto/setup-git-server-over-http.txt
index 7f4943e..981cbdd 100644
--- a/Documentation/howto/setup-git-server-over-http.txt
+++ b/Documentation/howto/setup-git-server-over-http.txt
@@ -81,8 +81,8 @@ Initialize a bare repository
 $ git --bare init
 
 
-Change the ownership to your web-server's credentials. Use grep ^User
-httpd.conf and grep ^Group httpd.conf to find out:
+Change the ownership to your web-server's credentials. Use `grep ^User
+httpd.conf` and `grep ^Group httpd.conf` to find out:
 
 $ chown -R www.www .
 
-- 
1.8.4
--
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] http: add option to enable 100 Continue responses

2013-10-11 Thread brian m. carlson
On Thu, Oct 10, 2013 at 01:14:28AM -0700, Shawn Pearce wrote:
 If a large enough percentage of users are stuck behind a proxy that
 doesn't support 100-continue, it is hard to rely on that part of HTTP
 1.1. You need to build the work-around for them anyway, so you might
 as well just make everyone use the work-around and assume 100-continue
 does not exist.

Well, the issue is that 100-continue is needed for functionality in some
cases, unless we want to restart the git-upload-pack command again or
force people to use outrageous sizes for http.postBuffer.  My preference
is generally to optimize for sane, standards-compliant behavior first,
and let the people with broken infrastructure turn on options to work
around that breakage.  I realize that git as a project is a little more
tolerant of people's myriad forms of breakage than I am personally.

Regardless, I have a reroll that leaves it disabled by default that I'll
send in a few minutes.

 100-continue is frequently used when there is a large POST body, but
 those suck for users on slow or unstable connections. Typically the
 POST cannot be resumed where the connection was broken. To be friendly
 to users on less reliable connections than your gigabit office
 ethernet, you need to design the client side with some sort of
 chunking and gracefully retrying. So Git is really doing it all wrong.
 :-)

Yeah, there's been requests for resumable pull/push before.  The proper
way to do it would probably to send lots of little mini-packs that each
depend on the previous pack sent; if the connection gets reset, then at
least some of the data has been transferred, and negotiation would
restart the next time.  The number of SHA-1s sent during negotiation
would have to increase though, because you couldn't be guaranteed that
an entire ref would be able to be transferred each time.  Large blobs
would still be a problem, though, and efficiency would plummet.

 Even if you want to live in the fairy land where all servers support
 100-continue, I'm not sure clients should pay that 100-160ms latency
 penalty during ancestor negotiation. Do 5 rounds of negotiation and
 its suddenly an extra half second for `git fetch`, and that is a
 fairly well connected client. Let me know how it works from India to a
 server on the west coast of the US, latency might be more like 200ms,
 and 5 rounds is now 1 full second of additional lag.

There shouldn't be that many rounds of negotiation.  HTTP retrieves the
list of refs over one connection, and then performs the POST over
another two.  Regardless, you should be using SSL over that connection,
and the number of round trips required for SSL negotiation in that case
completely dwarfs the overhead for the 100 continue, especially since
you'll do it thrice (even though the session is usually reused).  The
efficient way to do push is SSH, where you can avoid making multiple
connections and reuse the same encrypted connection at every stage.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


[PATCH v2 1/2] http: add option to enable 100 Continue responses

2013-10-11 Thread brian m. carlson
When using GSS-Negotiate authentication with libcurl, the authentication
provided will change every time, and so the probe that git uses to determine if
authentication is needed is not sufficient to guarantee that data can be sent.
If the data fits entirely in http.postBuffer bytes, the data can be rewound and
resent if authentication fails; otherwise, a 100 Continue must be requested in
this case.

By default, curl will send an Expect: 100-continue if a certain amount of data
is to be uploaded, but when using chunked data this is never triggered.  Add an
option http.continue, which defaults to disabled, to control whether this header
is sent.  The addition of an option is necessary because some proxies break
badly if sent this header.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 http.c| 6 ++
 http.h| 1 +
 remote-curl.c | 7 ++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/http.c b/http.c
index f3e1439..e4cd255 100644
--- a/http.c
+++ b/http.c
@@ -11,6 +11,7 @@
 int active_requests;
 int http_is_verbose;
 size_t http_post_buffer = 16 * LARGE_PACKET_MAX;
+int http_use_100_continue = 0;
 
 #if LIBCURL_VERSION_NUM = 0x070a06
 #define LIBCURL_CAN_HANDLE_AUTH_ANY
@@ -213,6 +214,11 @@ static int http_options(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (!strcmp(http.continue, var)) {
+   http_use_100_continue = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(http.useragent, var))
return git_config_string(user_agent, var, value);
 
diff --git a/http.h b/http.h
index d77c1b5..e72786e 100644
--- a/http.h
+++ b/http.h
@@ -102,6 +102,7 @@ extern void http_cleanup(void);
 extern int active_requests;
 extern int http_is_verbose;
 extern size_t http_post_buffer;
+extern int http_use_100_continue;
 
 extern char curl_errorstr[CURL_ERROR_SIZE];
 
diff --git a/remote-curl.c b/remote-curl.c
index b5ebe01..3b5e160 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -470,7 +470,12 @@ static int post_rpc(struct rpc_state *rpc)
 
headers = curl_slist_append(headers, rpc-hdr_content_type);
headers = curl_slist_append(headers, rpc-hdr_accept);
-   headers = curl_slist_append(headers, Expect:);
+
+   /* Force it either on or off, since curl will try to decide based on how
+* much data is to be uploaded and we want consistency.
+*/
+   headers = curl_slist_append(headers, http_use_100_continue ?
+   Expect: 100-continue : Expect:);
 
 retry:
slot = get_active_slot();
-- 
1.8.4.rc3

--
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 v2 2/2] Update documentation for http.continue option

2013-10-11 Thread brian m. carlson
Explain the reason for and behavior of the new http.continue option, and that it
is disabled by default.  Furthermore, explain that it is required for large
GSS-Negotiate requests but incompatible with some proxies.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/config.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fca7749..461c9dc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1516,6 +1516,15 @@ http.postBuffer::
massive pack file locally.  Default is 1 MiB, which is
sufficient for most requests.
 
+http.continue::
+   Ensure that authentication succeeds before sending the pack data when
+   POSTing data using the smart HTTP transport.  This is done by
+   requesting a 100 Continue response.  For requests larger than
+   'http.postBuffer', this is required when using GSS-Negotiate
+   (Kerberos) authentication over HTTP.  However, some proxies do not
+   handle the protocol exchange gracefully; for them, this option must be
+   disabled.  Defaults to disabled.
+
 http.lowSpeedLimit, http.lowSpeedTime::
If the HTTP transfer speed is less than 'http.lowSpeedLimit'
for longer than 'http.lowSpeedTime' seconds, the transfer is aborted.
-- 
1.8.4.rc3

--
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 v2 0/2] HTTP GSS-Negotiate improvements

2013-10-11 Thread brian m. carlson
This patch set adds an option, http.continue, to enable requests for 100
Continue responses when pushing over HTTP.  This is needed for large pushes
using GSS-Negotiate authentication.

Changes from v1:
* Default to disabled.

brian m. carlson (2):
  http: add option to enable 100 Continue responses
  Update documentation for http.continue option

 Documentation/config.txt | 9 +
 http.c   | 6 ++
 http.h   | 1 +
 remote-curl.c| 7 ++-
 4 files changed, 22 insertions(+), 1 deletion(-)

-- 
1.8.4.rc3

--
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/2] http: add option to enable 100 Continue responses

2013-10-11 Thread Jonathan Nieder
brian m. carlson wrote:

 When using GSS-Negotiate authentication with libcurl, the authentication
 provided will change every time, and so the probe that git uses to determine 
 if
 authentication is needed is not sufficient to guarantee that data can be sent.
 If the data fits entirely in http.postBuffer bytes, the data can be rewound 
 and
 resent if authentication fails; otherwise, a 100 Continue must be requested in
 this case.

 By default, curl will send an Expect: 100-continue if a certain amount of data
 is to be uploaded, but when using chunked data this is never triggered.  Add 
 an
 option http.continue, which defaults to disabled, to control whether this 
 header
 is sent.  The addition of an option is necessary because some proxies break
 badly if sent this header.

By default means when allowed to make its own choice, right?  (i.e.,
the behavior git never gave libcurl a chance to try :))

Makes sense.

[...]
 --- a/http.c
 +++ b/http.c
 @@ -11,6 +11,7 @@
  int active_requests;
  int http_is_verbose;
  size_t http_post_buffer = 16 * LARGE_PACKET_MAX;
 +int http_use_100_continue = 0;

Style: git tends to omit the '= 0' implicit for globals, since they are
already 0 by default.

[...]
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -470,7 +470,12 @@ static int post_rpc(struct rpc_state *rpc)
  
   headers = curl_slist_append(headers, rpc-hdr_content_type);
   headers = curl_slist_append(headers, rpc-hdr_accept);
 - headers = curl_slist_append(headers, Expect:);
 +
 + /* Force it either on or off, since curl will try to decide based on how
 +  * much data is to be uploaded and we want consistency.
 +  */

Style:

/*
 * Multi-line comments in git have the starting and ending comment
 * delimiters on their own lines, like this.
 */

Will make the fixups mentioned above, squash with documentation, and
apply.  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 v2 2/2] Update documentation for http.continue option

2013-10-11 Thread Jonathan Nieder
brian m. carlson wrote:

 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -1516,6 +1516,15 @@ http.postBuffer::
   massive pack file locally.  Default is 1 MiB, which is
   sufficient for most requests.
  
 +http.continue::
 + Ensure that authentication succeeds before sending the pack data when
 + POSTing data using the smart HTTP transport.  This is done by
 + requesting a 100 Continue response.  For requests larger than
 + 'http.postBuffer', this is required when using GSS-Negotiate
 + (Kerberos) authentication over HTTP.  However, some proxies do not
 + handle the protocol exchange gracefully; for them, this option must be
 + disabled.  Defaults to disabled.

It's not only your company's proxy that might mishandle 100-continue
but the target server's reverse proxy (or from the point of view of
the user, the target server), right?

I think the wording could be clearer about the impact of the setting
(some proxies and reverse proxies or something).

Perhaps this should be conditional on the authentication method used,
so affected people could still contact broken servers without having
different configuration per server and without having to wait a second
for the timeout.

Thanks,
Jonathan
--
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/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Felipe Contreras
Richard Hansen wrote:
 On 2013-09-09 18:49, Felipe Contreras wrote:
  On Mon, Sep 9, 2013 at 4:23 PM, Richard Hansen rhan...@bbn.com wrote:
  On 2013-09-08 21:23, Felipe Contreras wrote:
  The old configurations still work, but get deprecated.
 
  Should some tests for the deprecated configs be added?  We wouldn't want
  to accidentally break those.
  
  Probably, but Junio is not picking this patch anyway.
 
 It sounds to me like he would with some mods:
 http://thread.gmane.org/gmane.comp.version-control.git/233554/focus=234488

The modifications are already in this series.

-- 
Felipe Contreras
--
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/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Felipe Contreras
Matthieu Moy wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:
 
  On Mon, Sep 9, 2013 at 9:21 PM, Jeff King p...@peff.net wrote:
  On Mon, Sep 09, 2013 at 05:49:36PM -0500, Felipe Contreras wrote:
 
   These deprecation warning messages should be written to stderr, and
   should probably be prefixed with WARNING: .
 
  Is there any deprecation warning that works this way?
 
  The ones in C code typically use warning(), which will prefix with
  warning: and write to stderr. They do not use all-caps, though.
 
  Try git log --grep=deprecate -Swarning for some examples.
 
  I'm asking about the ones in shell, because this is a shell script.
 
 No user cares whether git pull is written in shell. shell Vs C is an
 implementation detail, stdout Vs stderr is user-visible.

You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, in the
meantime no shell script does that, and that's no reason to reject this patch
series.

-- 
Felipe Contreras
--
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/2] http: add option to enable 100 Continue responses

2013-10-11 Thread brian m. carlson
On Fri, Oct 11, 2013 at 04:43:07PM -0700, Jonathan Nieder wrote:
 By default means when allowed to make its own choice, right?  (i.e.,
 the behavior git never gave libcurl a chance to try :))
 
 Makes sense.

Yes, at least according to Daniel Stenberg.  I don't believe it is ever
triggered the way that git calls curl with CURLOPT_READFUNCTION, but I
can't be certain since I haven't looked at the source code.

 Style: git tends to omit the '= 0' implicit for globals, since they are
 already 0 by default.

Okay.

   /*
* Multi-line comments in git have the starting and ending comment
* delimiters on their own lines, like this.
*/

It wasn't clear from the existing code which way was being used, and the
CodingGuidelines file didn't seem to cover it.  I'll send a patch.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v2 2/2] Update documentation for http.continue option

2013-10-11 Thread brian m. carlson
On Fri, Oct 11, 2013 at 04:50:52PM -0700, Jonathan Nieder wrote:
 brian m. carlson wrote:
  +http.continue::
  +   Ensure that authentication succeeds before sending the pack data when
  +   POSTing data using the smart HTTP transport.  This is done by
  +   requesting a 100 Continue response.  For requests larger than
  +   'http.postBuffer', this is required when using GSS-Negotiate
  +   (Kerberos) authentication over HTTP.  However, some proxies do not
  +   handle the protocol exchange gracefully; for them, this option must be
  +   disabled.  Defaults to disabled.
 
 It's not only your company's proxy that might mishandle 100-continue
 but the target server's reverse proxy (or from the point of view of
 the user, the target server), right?
 
 I think the wording could be clearer about the impact of the setting
 (some proxies and reverse proxies or something).

I'm unclear about what systems are actually broken, since I don't deal
with any such systems.  One of Shawn Pearce's commit messages definitely
covered broken proxies, but it could really be anything beyond the
client: proxies, reverse proxies, servers, or even rogue intermediaries
(for non-TLS connections).

 Perhaps this should be conditional on the authentication method used,
 so affected people could still contact broken servers without having
 different configuration per server and without having to wait a second
 for the timeout.

curl determines what method, but I guess it's safe to assume that the
authentication method used for the probe will be the same as the one
used for the final connection.  Unfortunately, all curl will tell us is
what was offered, not what it would have chosen, so if GSSAPI and
something non-Basic are both offered, we'd have to make a guess.  (curl
will always prefer non-Basic to Basic for the obvious reasons.)

If you can argue for some sane semantics in what we should do in that
case, I'll reroll with that fix and a clearer wording for the docs.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v3 1/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Richard Hansen
On 2013-10-11 19:56, Felipe Contreras wrote:
 Matthieu Moy wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 On Mon, Sep 9, 2013 at 9:21 PM, Jeff King p...@peff.net wrote:
 On Mon, Sep 09, 2013 at 05:49:36PM -0500, Felipe Contreras wrote:

 These deprecation warning messages should be written to stderr, and
 should probably be prefixed with WARNING: .

 Is there any deprecation warning that works this way?

 The ones in C code typically use warning(), which will prefix with
 warning: and write to stderr. They do not use all-caps, though.

 Try git log --grep=deprecate -Swarning for some examples.

 I'm asking about the ones in shell, because this is a shell script.

 No user cares whether git pull is written in shell. shell Vs C is an
 implementation detail, stdout Vs stderr is user-visible.
 
 You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, in the
 meantime no shell script does that, and that's no reason to reject this patch
 series.

Sure, but is is a reason to reroll the patch series.  Maybe it's not a
strong enough reason on its own to reroll, but if you're going to reroll
anyway, this should be fixed.

-Richard
--
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] CodingGuidelines: style for multi-line comments

2013-10-11 Thread brian m. carlson
The style for multi-line comments is often mentioned and should be documented
for clarity.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/CodingGuidelines | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index e5ca3b7..a584062 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -145,6 +145,14 @@ For C programs:
they were describing changes.  Often splitting a function
into two makes the intention of the code much clearer.
 
+ - Multi-line comments should include their delimiters on separate lines from
+   the text. E.g.
+
+   /*
+* A very long
+* multi-line comment.
+*/
+
  - Double negation is often harder to understand than no negation
at all.
 
-- 
1.8.4

--
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/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Jeff King
On Fri, Oct 11, 2013 at 06:56:23PM -0500, Felipe Contreras wrote:

These deprecation warning messages should be written to stderr, and
should probably be prefixed with WARNING: .
  
   Is there any deprecation warning that works this way?
  
   The ones in C code typically use warning(), which will prefix with
   warning: and write to stderr. They do not use all-caps, though.
  
   Try git log --grep=deprecate -Swarning for some examples.
  
   I'm asking about the ones in shell, because this is a shell script.
  
  No user cares whether git pull is written in shell. shell Vs C is an
  implementation detail, stdout Vs stderr is user-visible.
 
 You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, in the
 meantime no shell script does that, and that's no reason to reject this patch
 series.

You are completely missing Matthieu's point that we attempt to be
consistent in the format of messages, as well as where they are output,
and from a user's perspective it does not matter what language the tool
is implemented in.

Also, you are wrong that there are no other shell scripts that behave as
Richard said:

  $ git grep '2' | grep -i deprecate
  contrib/completion/git-completion.bash: echo WARNING: this script is 
deprecated, please see git-completion.zsh 12
  contrib/examples/git-resolve.sh:echo 'WARNING: This command is DEPRECATED and 
will be removed very soon.' 2
  git-lost-found.sh:echo WARNING: '$0' is deprecated in favor of 'git fsck 
--lost-found' 2

Please, can we just squash in the patch below and stop talking about
this?

diff --git a/git-pull.sh b/git-pull.sh
index a9cf7ac..9c4091c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -58,8 +58,8 @@ then
if test $rebase = 'true'
then
mode=rebase
-   echo The configurations pull.rebase and branch.name.rebase 
are deprecated.
-   echo Please use pull.mode and branch.name.pullmode instead.
+   echo 2 warning: The configurations pull.rebase and 
branch.name.rebase are deprecated.
+   echo 2 Please use pull.mode and branch.name.pullmode 
instead.
fi
 fi
 test -z $mode  mode=merge
--
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/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Felipe Contreras
On Fri, Oct 11, 2013 at 7:50 PM, Jeff King p...@peff.net wrote:
 On Fri, Oct 11, 2013 at 06:56:23PM -0500, Felipe Contreras wrote:

These deprecation warning messages should be written to stderr, and
should probably be prefixed with WARNING: .
  
   Is there any deprecation warning that works this way?
  
   The ones in C code typically use warning(), which will prefix with
   warning: and write to stderr. They do not use all-caps, though.
  
   Try git log --grep=deprecate -Swarning for some examples.
  
   I'm asking about the ones in shell, because this is a shell script.
 
  No user cares whether git pull is written in shell. shell Vs C is an
  implementation detail, stdout Vs stderr is user-visible.

 You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, in 
 the
 meantime no shell script does that, and that's no reason to reject this patch
 series.

 You are completely missing Matthieu's point that we attempt to be
 consistent in the format of messages, as well as where they are output,
 and from a user's perspective it does not matter what language the tool
 is implemented in.

If we truly did that, there should be a warning () function, like in C.

 Also, you are wrong that there are no other shell scripts that behave as
 Richard said:

   $ git grep '2' | grep -i deprecate
   contrib/completion/git-completion.bash: echo WARNING: this script is 
 deprecated, please see git-completion.zsh 12
   contrib/examples/git-resolve.sh:echo 'WARNING: This command is DEPRECATED 
 and will be removed very soon.' 2
   git-lost-found.sh:echo WARNING: '$0' is deprecated in favor of 'git fsck 
 --lost-found' 2

 Please, can we just squash in the patch below and stop talking about
 this?

 diff --git a/git-pull.sh b/git-pull.sh
 index a9cf7ac..9c4091c 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -58,8 +58,8 @@ then
 if test $rebase = 'true'
 then
 mode=rebase
 -   echo The configurations pull.rebase and branch.name.rebase 
 are deprecated.
 -   echo Please use pull.mode and branch.name.pullmode 
 instead.
 +   echo 2 warning: The configurations pull.rebase and 
 branch.name.rebase are deprecated.
 +   echo 2 Please use pull.mode and branch.name.pullmode 
 instead.
 fi
  fi
  test -z $mode  mode=merge

Are you sure you want me to squash that in? Because the warnings
wouldn't be consistent. Some would be WARNING:  and others would be
warning: . Personally I don't care, but if your argument is
consistency, you should. If we had a warning () function, we could
truly be consistent.

-- 
Felipe Contreras
--
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/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Jeff King
On Fri, Oct 11, 2013 at 08:15:46PM -0500, Felipe Contreras wrote:

  You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, in 
  the
  meantime no shell script does that, and that's no reason to reject this 
  patch
  series.
 
  You are completely missing Matthieu's point that we attempt to be
  consistent in the format of messages, as well as where they are output,
  and from a user's perspective it does not matter what language the tool
  is implemented in.
 
 If we truly did that, there should be a warning () function, like in C.

Or people could hand-code them to look similar, which is exactly what
has happened.

If you want to factor out a warning function to clean up the code, be my
guest. But the lack of one does not provide an argument that you should
break consistency.

  -   echo The configurations pull.rebase and 
  branch.name.rebase are deprecated.
  -   echo Please use pull.mode and branch.name.pullmode 
  instead.
  +   echo 2 warning: The configurations pull.rebase and 
  branch.name.rebase are deprecated.
  +   echo 2 Please use pull.mode and branch.name.pullmode 
  instead.
 [...]
 
 Are you sure you want me to squash that in? Because the warnings
 wouldn't be consistent. Some would be WARNING:  and others would be
 warning: . Personally I don't care, but if your argument is
 consistency, you should. If we had a warning () function, we could
 truly be consistent.

It is significant in the most important ways, which are labeling it at
all, and sending it to stderr. Capitalization is less important, in my
opinion.

Using a lowercase version is much more consistent with the warnings
produced by C code, which is why I chose it over the capitalized
version. Again, if you want to change the existing WARNING cases in the
shell scripts to be consistent with C output, be my guest.

Do you actually have some reason for wanting to output to go to stdout?

-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] http: add option to enable 100 Continue responses

2013-10-11 Thread Shawn Pearce
On Fri, Oct 11, 2013 at 3:31 PM, brian m. carlson
sand...@crustytoothpaste.net wrote:
 On Thu, Oct 10, 2013 at 01:14:28AM -0700, Shawn Pearce wrote:
 Even if you want to live in the fairy land where all servers support
 100-continue, I'm not sure clients should pay that 100-160ms latency
 penalty during ancestor negotiation. Do 5 rounds of negotiation and
 its suddenly an extra half second for `git fetch`, and that is a
 fairly well connected client. Let me know how it works from India to a
 server on the west coast of the US, latency might be more like 200ms,
 and 5 rounds is now 1 full second of additional lag.

 There shouldn't be that many rounds of negotiation.  HTTP retrieves the
 list of refs over one connection, and then performs the POST over
 another two.

Why two connections? This should be a single HTTP connection with HTTP
Keep-Alive semantics allowing the same TCP stream and the same SSL
stream to be used for all requests. Which is nearly equivalent to SSH.
Where SSH wins is the multi_ack protocol allowing the server to talk
while the client is talking.

  Regardless, you should be using SSL over that connection,
 and the number of round trips required for SSL negotiation in that case
 completely dwarfs the overhead for the 100 continue, especially since
 you'll do it thrice (even though the session is usually reused).  The
 efficient way to do push is SSH, where you can avoid making multiple
 connections and reuse the same encrypted connection at every stage.

SSH setup is also not free. Like SSL its going to require a round trip
or two on top of what Git needs.
--
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/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Felipe Contreras
On Fri, Oct 11, 2013 at 8:25 PM, Jeff King p...@peff.net wrote:
 On Fri, Oct 11, 2013 at 08:15:46PM -0500, Felipe Contreras wrote:

  You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, 
  in the
  meantime no shell script does that, and that's no reason to reject this 
  patch
  series.
 
  You are completely missing Matthieu's point that we attempt to be
  consistent in the format of messages, as well as where they are output,
  and from a user's perspective it does not matter what language the tool
  is implemented in.

 If we truly did that, there should be a warning () function, like in C.

 Or people could hand-code them to look similar, which is exactly what
 has happened.

And by doing that be prone to make mistakes, like using 'WARNING',
instead of 'warning'. But I guess you don't care about consistency
_that much_.

 If you want to factor out a warning function to clean up the code, be my
 guest. But the lack of one does not provide an argument that you should
 break consistency.

Consistency is already broken.

  -   echo The configurations pull.rebase and 
  branch.name.rebase are deprecated.
  -   echo Please use pull.mode and branch.name.pullmode 
  instead.
  +   echo 2 warning: The configurations pull.rebase and 
  branch.name.rebase are deprecated.
  +   echo 2 Please use pull.mode and branch.name.pullmode 
  instead.
 [...]

 Are you sure you want me to squash that in? Because the warnings
 wouldn't be consistent. Some would be WARNING:  and others would be
 warning: . Personally I don't care, but if your argument is
 consistency, you should. If we had a warning () function, we could
 truly be consistent.

 It is significant in the most important ways, which are labeling it at
 all, and sending it to stderr. Capitalization is less important, in my
 opinion.

It's still inconsistent.

 Using a lowercase version is much more consistent with the warnings
 produced by C code, which is why I chose it over the capitalized
 version. Again, if you want to change the existing WARNING cases in the
 shell scripts to be consistent with C output, be my guest.

It seems you are not that interested in consistency after all.

 Do you actually have some reason for wanting to output to go to stdout?

I'm fine with 'echo warning: foo 2', but still, if you really
cared about consistency, there would be a warn() function, precisely
to avoid the mistakes of WARNING vs. warning which are already there,
plus future ones.

-- 
Felipe Contreras
--
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


pack-object's try_delta fast path for v2 trees?

2013-10-11 Thread Duy Nguyen
Hi,

Just wondering if this has been considered and dropped before.
Currently we use try_delta() for every object including trees. But
trees are special. All tree entries must be unique and sorted. That
helps simplify diff algorithm, as demonstrated by diff_tree() and
pv4_encode_tree(). A quick and dirty test with test-delta shows that
tree_diff only needs half the time of diff_delta(). As trees account
for like half the objects in a repo, speeding up delta search might
help performance, I think.
-- 
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


Git send-email fail on Mac OS X Lion

2013-10-11 Thread Fernando Ortiz (e2k)
I'm getting the following error when I do:

git send-email --compose --from Fernando Ortiz eratos2...@gmail.com --to 
forti...@gmail.com --cc forti...@gmail.com 
0001-Change-zcat-to-gzcat-to-fix-build-restore-steps.patch

Net::SSLeay version 1.46 required--this is only version 1.36 at 
/Users/fortiz/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/IO/Socket/SSL.pm
 line 17.
BEGIN failed--compilation aborted at 
/Users/fortiz/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/IO/Socket/SSL.pm
 line 17.
Compilation failed in require at 
/Users/fortiz/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Net/SMTP/SSL.pm
 line 8.
BEGIN failed--compilation aborted at 
/Users/fortiz/perl5/perlbrew/perls/perl-5.14.4/lib/site_perl/5.14.4/Net/SMTP/SSL.pm
 line 8.
Compilation failed in require at 
/usr/local/Cellar/git/1.8.4/libexec/git-core/git-send-email line 1232.--
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/5] pull: rename pull.rename to pull.mode

2013-10-11 Thread Richard Hansen
On 2013-10-11 22:08, Felipe Contreras wrote:
 I'm fine with 'echo warning: foo 2', but still, if you really
 cared about consistency, there would be a warn() function

So add one!  It's only one simple line:

warning() { printf %s\\n warning: $* 2; }

So much discussion for something so trivial...

-Richard
--
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 v2] mergetools/diffmerge: support DiffMerge as a git mergetool

2013-10-11 Thread Stefan Saasen
DiffMerge is a non-free (but gratis) tool that supports OS X, Windows and Linux.

See http://www.sourcegear.com/diffmerge/

DiffMerge includes a script `/usr/bin/diffmerge` that can be used to launch the
graphical compare tool.

This change adds mergetool support for DiffMerge and adds 'diffmerge' as an
option to the mergetool help.

Signed-off-by: Stefan Saasen ssaa...@atlassian.com
Acked-by: David Aguilar dav...@gmail.com
---
 contrib/completion/git-completion.bash |  2 +-
 git-mergetool--lib.sh  |  3 ++-
 mergetools/diffmerge   | 15 +++
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 mergetools/diffmerge

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e1b7313..07b0ba5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1188,7 +1188,7 @@ _git_diff ()
__git_complete_revlist_file
 }
 
-__git_mergetools_common=diffuse ecmerge emerge kdiff3 meld opendiff
+__git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 
codecompare
 
 
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index feee6a4..0fcb253 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -250,7 +250,8 @@ list_merge_tool_candidates () {
else
tools=opendiff kdiff3 tkdiff xxdiff meld $tools
fi
-   tools=$tools gvimdiff diffuse ecmerge p4merge araxis bc3 
codecompare
+   tools=$tools gvimdiff diffuse diffmerge ecmerge 
+   tools+=p4merge araxis bc3 codecompare
fi
case ${VISUAL:-$EDITOR} in
*vim*)
diff --git a/mergetools/diffmerge b/mergetools/diffmerge
new file mode 100644
index 000..85ac720
--- /dev/null
+++ b/mergetools/diffmerge
@@ -0,0 +1,15 @@
+diff_cmd () {
+   $merge_tool_path $LOCAL $REMOTE /dev/null 21
+}
+
+merge_cmd () {
+   if $base_present
+   then
+   $merge_tool_path --merge --result=$MERGED \
+   $LOCAL $BASE $REMOTE
+   else
+   $merge_tool_path --merge \
+   --result=$MERGED $LOCAL $REMOTE
+   fi
+   status=$?
+}
-- 
1.8.2.3

--
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: [spf:guess,mismatch] [PATCH v2] diff.c: keep arrow(=) on show_stats()'s shortened filename part to make rename visible.

2013-10-11 Thread Keshav Kini
Sam Vilain s...@vilain.net writes:

 On 10/11/2013 06:07 AM, Yoshioka Tsuneo wrote:
 +prefix_len = ((prefix_len = 0) ? prefix_len : 
 0);
 +strncpy(pre_arrow, arrow - prefix_len, 
 prefix_len);
 +pre_arrow[prefix_len] = '¥0';


 This seems to be an encoding mistake; was this supposed to be an ASCII
 arrow?

That's supposed to be a null terminator character, '\0'. See
https://en.wikipedia.org/wiki/Yen_symbol#Code_page_932

-Keshav

--
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