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