login
Search
I'll do the full-list at the end of my git Wednesday, but here are the current highlights I am sending out to hear opinions from people. Please note that I haven't caught up with the patches from past 12 hours or so. ---------------------------------------------------------------- [New Topics] * jn/help-everywhere (2009-11-09) 21 commits - diff --no-index: make the usage string less scary - ... - Show usage string for 'git grep -h' (this branch uses jn/maint-http-fetch-mingw and jn/remove-fetch--tool.) There were unrelated but still worthy fixes, so I reordered some of them; also the "usage()" change is different from the one that was posted (see my comment in $gmane/132592). * jn/maint-http-fetch-mingw (2009-11-09) 1 commit. - http-fetch: add missing initialization of argv0_path (this branch is used by jn/help-everywhere.) * jn/remove-fetch--tool (2009-11-09) 1 commit - Retire fetch--tool helper to contrib/examples (this branch is used by jn/help-everywhere.) These two were originally part of the "help-everywhere" topic but they can stand on their own. * jc/log-stdin (2009-11-03) 1 commit - Teach --stdin option to "log" family This is not signed-off (see $gmane/131971 for list of things you can do to help advancing this topic). -------------------------------------------------- [Stalled] * sc/protocol-doc (2009-10-29) 1 commit - Update packfile transfer protocol documentation * jl/submodule-add-noname (2009-09-22) 1 commit. - git submodule add: make the <path> parameter optional Dscho started an interesting discussion regarding the larger workflow in which the "submodule add" is used. I think the patch itself makes sense but at the same time it probably makes sense to also take the <path> and infer the <repository> as Dscho suggested, probably in "git submodule add", not in "git add" proper, at least initially. Any objections against merging this to 'next'? * jc/fix-tree-walk (2009-10-22) 11 commits. (merged to 'next' on ...
The only note here is that these patches are dependent on the ones in jn/editor-pager, so if that doesn't get merged, this shouldn't either. From my POV, jn/editor-pager does what I was after, so I'm happy. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 GPG Key Id: 8E89F6D2; Key Server: pgp.mit.edu Contact me to arrange for a CAcert assurance meeting.
Sorry for the silence, I've been out with the flu, and been caught up with meatspace issues in general. Will try to send another iteration of the later API part in a few days. ...Johan -- Johan Herland, <johan@herland.net> www.herland.net --
Heya, I ran out of git time due to the start of my uni's next quarter, this Daniel, are you going to send a follow-up to the memory-leaking patch? If not, this needs to stay out of next until I have time to do so. My gitdir patch might need to be evicted as it is connected to sr/gfi-options which is not yet done. Also, we need to update the documentation, but I think we can at least start cooking it in next Does the current version require me to 'cd contrib/completion && make' each time we update completion? If so, that's a (very annoying) regression that needs to be fixed before it's merged to master IMHO. -- Cheers, Sverre Rabbelier --
Okay, finally got to it just now. This is entirely untested (because I
don't have anything that uses it), but it compiles and should be correct.
It would be nice to get a clean bill of health on it from valgrind.
If it works, please squash it into my other patch, keep that commit
message, and add my sign-off.
commit 5fdebd88a8e5c2714470c86a2c13ee2c795210cb
Author: Daniel Barkalow <barkalow@iabervon.org>
Date: Wed Nov 11 13:36:36 2009 -0500
Free memory used by remote helper importing refspecs, document
"Allow helper to map private ref names into normal names" was just an
RFD which turned out to be surprisingly correct. This adds the necessary
memory-management and documentation, and generally makes it a suitable
patch.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 2c5130f..f4b6a5a 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -46,7 +46,11 @@ Supported if the helper has the "fetch" capability.
'import' <name>::
Produces a fast-import stream which imports the current value
of the named ref. It may additionally import other refs as
- needed to construct the history efficiently.
+ needed to construct the history efficiently. The script writes
+ to a helper-specific private namespace. The value of the named
+ ref should be written to a location in this namespace derived
+ by applying the refspecs from the "refspec" capability to the
+ name of the ref.
+
Supported if the helper has the "import" capability.
@@ -67,6 +71,16 @@ CAPABILITIES
'import'::
This helper supports the 'import' command.
+'refspec' 'spec'::
+ When using the import command, expect the source ref to have
+ been written to the destination ref. The earliest applicable
+ refspec takes precedence. For example
+ "refs/heads/*:refs/svn/origin/branches/*" means that, after an
+ "import ...Heya,
After a lot of manual comparison of valgrind traces (comparing cloning
a local git repo and a local hg repo), I have found the following
before applying your patch (I removed the ==proc== and "by 0xDEADBEEF"
noise):
20 bytes in 1 blocks are definitely lost in loss record 24 of 95
at : calloc (vg_replace_malloc.c:418)
by : xcalloc (wrapper.c:75)
by : get_importer (transport-helper.c:132)
by : fetch_with_import (transport-helper.c:150)
by : fetch (transport-helper.c:198)
by : transport_fetch_refs (transport.c:977)
by : cmd_clone (builtin-clone.c:538)
by : run_builtin (git.c:251)
by : handle_internal_command (git.c:396)
by : run_argv (git.c:438)
by : main (git.c:509)
60 bytes in 1 blocks are definitely lost in loss record 43 of 95
at : realloc (vg_replace_malloc.c:476)
by : xrealloc (wrapper.c:59)
by : strbuf_grow (strbuf.c:61)
by : strbuf_getwholeline (strbuf.c:335)
by : strbuf_getline (strbuf.c:349)
by : get_helper (transport-helper.c:52)
by : get_refs_list (transport-helper.c:228)
by : transport_get_remote_refs (transport.c:943)
by : cmd_clone (builtin-clone.c:535)
by : run_builtin (git.c:251)
by : handle_internal_command (git.c:396)
by : run_argv (git.c:438)
65 bytes in 1 blocks are definitely lost in loss record 47 of 95
at : malloc (vg_replace_malloc.c:195)
by : realloc (vg_replace_malloc.c:476)
by : xrealloc (wrapper.c:59)
by : strbuf_grow (strbuf.c:61)
by : strbuf_addf (strbuf.c:199)
by : get_helper (transport-helper.c:69)
by : get_refs_list (transport-helper.c:228)
by : transport_get_remote_refs (transport.c:943)
by : cmd_clone (builtin-clone.c:535)
by : run_builtin (git.c:251)
by : handle_internal_command (git.c:396)
by : run_argv (git.c:438)
65 bytes in 1 blocks are definitely lost in loss record 50 of 95
at : malloc (vg_replace_malloc.c:195)
by : realloc (vg_replace_malloc.c:476)
...Looks like I'm discarding the information before I use it, instead of after. Try moving those lines to release_helper(), and a "!data->refspecs &&" to the !prefixcmp(... "refspec ") if condition. (That is, keep the refspecs from the first call to the helper until the whole thing is released, and ignore those capabilities in later calls.) -Daniel *This .sig left intentionally blank* --
As a distro user, I don’t think I would be able to use it until there
is a command to update the installed completion, to call after adding
a new git command to my $PATH. This could mean:
- git-completion.bash.generate learns to read the .in file and
write the completion script to arbitrary paths (or just always
uses stdin and stdout?)
- distros install git-completion.bash.{generate,in} to /usr/share/git-core
- distros install a simple completion script to /etc/bash_completion.d
that passes the buck, e.g.
-- %< --
# bash completion support for core Git.
#
# Run update-git-completion to generate these files.
#
__git_user_completion=~/.cache/git-core/git-completion.bash
__git_system_completion=/var/cache/git-core/git-completion.bash
if test -r "$__git_user_completion"
then
. "$__git_user_completion"
elif test -r "$__git_system_completion"
then
. "$__git_system_completion"
fi
-- >% --
- new update-git-completion script, something like this:
-- %< --
#!/bin/sh
USAGE="update-git-completion {--system | --user | <filename>}"
datadir=/usr/share/git-core
die() {
echo >&2 "$*"
exit 1
}
if ! test $# -eq 1
then
die "usage: $USAGE"
fi
if test "$1" = "--system"
then
output=/var/cache/git-core/git-completion.bash
elif test "$1" = "--user"
then
output=$HOME/.cache/git-core/git-completion.bash
else
output=$1
fi
rm -f "$output+" || die "cannot remove $output+"
sh "$datadir"/git-completion.bash.generate \
< "$datadir"/git-completion.bash.in \
> "$output+" || die "failed to generate completion script"
bash -n "$output+" || {
rm -f "$output+"
die "generated script fails syntax check"
}
mv -f "$output+" "$output" || {
rm -f "$output+"
die "failed to install completion script"
}
-- >% --
Thoughts?
--
I'm confused. Shouldn't your distro take care of updating the completion for you? And wouldn't update-git-completion be more suited as part of a makefile if it was needed at all? --
The problem is that I have git commands the distro did not install in my $PATH. For example, I currently have git-new-workdir in ~/bin, and once I bzr-fastexport works a little better, I will install git-bzr. Even without such commands, in many distributions the completion should not be one size fits all, since git-svn (for example) belongs to a different package. I would expect the distribution to take care of populating the initial completion list and updating it on upgrades. Jonathan --
Ah ok. I think this proves even more that pregenerating the completion is a bad idea. With dynamic population we don't have these problems and it only takes 250ms more to load on a P3 700Mhz. Maybe we should try and speedup 'git help -a' and 'git merge -s help' instead? Perhaps options for the command/porcelain lists and available strategies formatted for script consumption? I doubt it will be as fast as compiling, but every bit helps apparently. Side Note: Running git merge -s help outside a git repository fails, so caching of the merge strategies isn't very effective. --
On my slow laptop (P3 600MHz), system-wide bash completions take
too much time to load (> 2s), and a significant fraction of this
time is spent loading git-completion.bash:
$ time bash -c ". ./git-completion.bash" # hot cache, before this patch
real 0m0.509s
user 0m0.310s
sys 0m0.180s
Kirill tracked down that the most time is spent warming up
merge_strategy, all_command & porcelain_command caches.
Since git is not used in each and every interactive xterm, it
seems best to load completion support with cold caches, and then
load each needed thing lazily. This has most of the speed
advantage of pre-generating everything at build time, without the
complication of figuring out at build time what commands will be
available at run time.
The result is that loading completion is significantly faster
now:
$ time bash -c ". ./git-completion.bash" # cold cache, after this patch
real 0m0.171s
user 0m0.060s
sys 0m0.040s
Suggested-by: Kirill Smelkov <kirr@mns.spb.ru>
Cc: Shawn O. Pearce <spearce@spearce.org>
Cc: Stephen Boyd <bebarino@gmail.com>
Cc: Sverre Rabbelier <srabbelier@gmail.com>
Cc: Junio C Hamano <junio@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hmm, 250 ms is a lot.
Strictly speaking, even with the existing completion script, it might
be nice to provide a "hash -r"-like command to bring the caches up to
date. Such a function could be:
git-completion-rehash ()
{
__git_compute_all_commands
__git_compute_merge_strategies
__git_compute_porcelain_commands
}
But that is a separate topic.
contrib/completion/git-completion.bash | 35 +++++++++++++++++--------------
1 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9e8b607..7088ec7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -330,10 +330,9 @@ __git_list_merge_strategies ()
}'
}
...On my slow laptop (P3 600MHz), system-wide bash completions take too much time to load (> 2s), and a significant fraction of this time is spent loading git-completion.bash: $ time bash -c ". ./git-completion.bash" # hot cache, before this patch real 0m0.509s user 0m0.310s sys 0m0.180s Kirill noticed that most of the time is spent warming up merge_strategy, all_command & porcelain_command caches. Since git is not used in each and every interactive xterm, it seems best to load completion support with cold caches, and then load each needed thing lazily. This has most of the speed advantage of pre-generating everything at build time, without the complication of figuring out at build time what commands will be available at run time. The result is that loading completion is significantly faster now: $ time bash -c ". ./git-completion.bash" # cold cache, after this patch real 0m0.171s user 0m0.060s sys 0m0.040s Suggested-by: Kirill Smelkov <kirr@mns.spb.ru> Cc: Shawn O. Pearce <spearce@spearce.org> Cc: Stephen Boyd <bebarino@gmail.com> Cc: Sverre Rabbelier <srabbelier@gmail.com> Cc: Junio C Hamano <junio@pobox.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Here’s the real patch. Sorry about that. Jonathan contrib/completion/git-completion.bash | 67 +++++++++++++++---------------- 1 files changed, 32 insertions(+), 35 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e3ddecc..7088ec7 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -21,13 +21,7 @@ # 2) Added the following line to your .bashrc: # source ~/.git-completion.sh # -# 3) You may want to make sure the git executable is available -# in your PATH before this script is sourced, as some caching -# is performed while the script loads. If git isn't found -# at source time then all lookups will be done on ...
s/__git_all_commands/\$__git_all_commands/ Yikes. Next patch I’ll let sit for a day. Good night, Jonathan --
I was under the impression that setting variables during completion
meant they were lost at the end of the completion cycle. So to be safe I
put a 'sleep 5' in __git_list_porcelain_commands() and it only stalled
me once :-)
I see a small problem, but it can be fixed in another patch. git merge
-s <TAB><TAB> the first time when you're not in a git directory will
make git merge -s <TAB><TAB> after never complete correctly even in a
git directory. I guess this become more serious if git isn't in your
path initially and you do git <TAB><TAB> and then git becomes part of
your path and you try again. Here you lose porcelain completion. This is
probably never going to happen though, right?
Plus it seems that __git_all_commands is computed twice if I git
<TAB><TAB> and then git help <TAB><TAB> after. Can that be avoided?
Squashing this on top fixes the two typos you mentioned.
---->8----
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7088ec7..6817953 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1082,7 +1082,7 @@ _git_help ()
;;
esac
__git_compute_all_commands
- __gitcomp "__git_all_commands
+ __gitcomp "$__git_all_commands
attributes cli core-tutorial cvs-migration
diffcore gitk glossary hooks ignore modules
repository-layout tutorial tutorial-2
@@ -1541,7 +1541,7 @@ _git_config ()
local pfx="${cur%.*}."
cur="${cur#*.}"
__git_compute_all_commands
- __gitcomp "__git_all_commands" "$pfx" "$cur"
+ __gitcomp "$__git_all_commands" "$pfx" "$cur"
return
;;
remote.*.*)
--
Sounds like a bug; thanks. I’ll squash in something like the following
for the next iteration.
-- %< --
Subject: completion: avoid computing command list twice
__git_all_commands is being computed twice on git <TAB><TAB> with
git help <TAB><TAB> after.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
contrib/completion/git-completion.bash | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6817953..748d4f9 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -588,6 +588,7 @@ __git_list_porcelain_commands ()
__git_compute_porcelain_commands ()
{
+ __git_compute_all_commands
: ${__git_porcelain_commands=$(__git_list_porcelain_commands)}
}
--
1.6.5.2
--
Not good. I think the sanest thing to do is avoid caching the merge
strategy list entirely. Listing merge strategies takes about 100 ms
Right, if this happened to me I would not be too surprised. A similar
problem occurs when someone installs a new git subcommand in the
middle of a session. Starting a new session fixes the completion, but
should the completion script do something to help people before then?
It could provide a shell function with a slightly friendlier name than
"__git_compute_porcelain_commands" for the user to invoke explicitly.
It could retry "git help -a" each time completion was needed if it
gave no results last time (i.e. use "${__git_all_commands:=" instead
of "${__git_all_commands="). This would help with the missing git
problem (which seems unusual), but not the missing git-svn.
Maybe it could detect signs of user frustration (repeated attempts to
complete the same thing?) and add to the frustration by silently
fixing the cache.
-- %< --
Subject: completion: do not cache merge strategy list
If "git merge -s help" fails the first time we try it (e.g. if you're
not in a git directory), git merge -s <TAB><TAB> never completes
correctly within the same session.
Reported-by: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 748d4f9..634941f 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -332,7 +332,7 @@ __git_list_merge_strategies ()
__git_compute_merge_strategies ()
{
- : ${__git_merge_strategies=$(__git_list_merge_strategies)}
+ __git_merge_strategies=$(__git_list_merge_strategies)
}
__git_complete_file ()
--
1.6.5.2
--
Hi, Why? Don't get overly creative here, the command . /path/to/git-completion.bash already does that, plus it fixes the merge strategy completion issue, and it's friendly enough for the users. Best, Gábor --
Hi Gábor,
Sounds like a good approach. Squashing this in should get that
working again.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
In this patch, I assume the merge strategy list is not being cached
any more. Something like this would allow recovering from the merge
strategy completion issue, but the victim would have to notice what
went wrong first.
contrib/completion/git-completion.bash | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 634941f..ae39373 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -495,6 +495,7 @@ __git_list_all_commands ()
done
}
+unset __git_all_commands
__git_compute_all_commands ()
{
: ${__git_all_commands=$(__git_list_all_commands)}
@@ -586,6 +587,7 @@ __git_list_porcelain_commands ()
done
}
+unset __git_porcelain_commands
__git_compute_porcelain_commands ()
{
__git_compute_all_commands
--
1.6.5.2
--
I was thinking of adding an option to git merge (like --strategies) to list the strategies without requiring you to be in a git directory. It doesn't look that easy at first glance so it might not be worth the trouble. --
Right, that would require support from run_builtin(). --
Wouldn't
: ${__gms:=$(command)}
run command only until it gives a non-empty string?
--
On my slow laptop (P3 600MHz), system-wide bash completions take too much time to load (> 2s), and a significant fraction of this time is spent loading git-completion.bash: $ time bash -c ". ./git-completion.bash" # hot cache, before this patch real 0m0.509s user 0m0.310s sys 0m0.180s Kirill noticed that most of the time is spent warming up the merge_strategy, all_command and porcelain_command caches. Since git is not used in each and every interactive xterm, it seems best to load completion support with cold caches and then load each needed thing lazily. This has most of the speed advantage of pre-generating everything at build time, without the complication of figuring out at build time what commands will be available at run time. The result is that loading completion is significantly faster now: $ time bash -c ". ./git-completion.bash" # cold cache, after this patch real 0m0.171s user 0m0.060s sys 0m0.040s Suggested-by: Kirill Smelkov <kirr@mns.spb.ru> Cc: Shawn O. Pearce <spearce@spearce.org> Cc: Stephen Boyd <bebarino@gmail.com> Cc: SZEDER Gábor <szeder@ira.uka.de> Cc: Sverre Rabbelier <srabbelier@gmail.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- This version incorporates the small improvements previously sent, plus the following nice one (which makes caching merge strategies safe again). Thanks for the advice, everyone. Yes. :) contrib/completion/git-completion.bash | 76 +++++++++++++++++--------------- 1 files changed, 41 insertions(+), 35 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e3ddecc..43d76b7 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -21,13 +21,7 @@ # 2) Added the following line to your .bashrc: # source ~/.git-completion.sh # -# 3) You may want to make sure the git executable is available -# in your PATH before this script is sourced, as ...
Acked-By: Shawn O. Pearce <spearce@spearce.org> -- Shawn. --
Why not do this for all of the lists and not just the merge strategies? --
Since git is not used in each and every interactive xterm, it
seems best to load completion support with cold caches and then
load each needed thing lazily. This has most of the speed
advantage of pre-generating everything at build time, without the
complication of figuring out at build time what commands will be
available at run time.
On this slow laptop, this decreases the time to load
git-completion.bash from about 500 ms to about 175 ms.
Suggested-by: Kirill Smelkov <kirr@mns.spb.ru>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Cc: Stephen Boyd <bebarino@gmail.com>
Cc: SZEDER Gábor <szeder@ira.uka.de>
Cc: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
I do not know whether it is kosher to carry over an ack like this.
The interdiff is small, for what it’s worth:
$ git branch jn/faster-completion-startup ee41299
$ git diff jn/faster-completion-startup
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bd22802..f67254d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -330,7 +330,7 @@ __git_list_merge_strategies ()
}'
}
-unset __git_merge_strategies
+__git_merge_strategies=
# 'git merge -s help' (and thus detection of the merge strategy
# list) fails, unfortunately, if run outside of any git working
# tree. __git_merge_strategies is set to the empty string in
@@ -501,10 +501,10 @@ __git_list_all_commands ()
done
}
-unset __git_all_commands
+__git_all_commands=
__git_compute_all_commands ()
{
- : ${__git_all_commands=$(__git_list_all_commands)}
+ : ${__git_all_commands:=$(__git_list_all_commands)}
}
__git_list_porcelain_commands ()
@@ -593,11 +593,11 @@ __git_list_porcelain_commands ()
done
}
-unset __git_porcelain_commands
+__git_porcelain_commands=
__git_compute_porcelain_commands ()
{
__git_compute_all_commands
- : ...Usually you leave it in if all you've done is address minor reviewer comments and they had actually supplied an Acked-by line for the prior version. Its also usually fine to leave it like this, because there is a good chance that the reviewer will agree with the new version and it saves Junio from needing to insert the line himself later. But it would be bad form to leave an Acked-by in if there was a major rewrite. E.g. this particular fix has gone through some really major rework to reach this point, keeping an Acked-by from the very first "speedup loading" patch (which IIRC computed them at build time) into this one would be quite unfriendly. -- Shawn. --
A bit OT, I've noticed the following output today: % git clone git://repo.or.cz/girocco.git Initialized empty Git repository in /home/nicolas/dev/official_packages/girocco/.git/ remote: Counting objects: 3017, done. g objects: 100% (994/994), done. remote: Total 3017 (delta 1911), reused 2988 (delta 1896) Receiving objects: 100% (3017/3017), 403.99 KiB | 309 KiB/s, done. Resolving deltas: 100% (1911/1911), done. % Notice the "g " at the begining at the 3th line. This is reproducible. % git --version git version 1.6.5.2.352.g8b8744 I can provide more test or bissect it if wanted. -- Nicolas Sebrecht --
I get much worse: Initialized empty Git repository in /home/nico/git/girocco/.git/ remote: Counting objects: 3017, done. remote: Compressing objects: 5% (50/994) Receiving objects: 3% (91/3017) remote: Compressing objects: 15% (150/994) Receiving objects: 7% (212/3017) remote: Compressing oReceiving objects: 14% (423/3017), 76.00 KiB | 135 KiB/s remote: Compressing objects: 35Receiving objects: 16% (483/3017), 76.00 KiB | remote: Compressing objects: 38% (378/994)Receiving objects: 17% (513/3017), 7 remote: Compressing objectReceiving objects: 20% (604/3017), 76.00 KiB | 135 Ki remote: Compressing objects: 48% (47Receiving objects: 22% (664/3017), 76.00 K remote: Compressing objects: 5Receiving objects: 23% (694/3017), 76.00 KiB | 1 remote: Compressing objects: Receiving objects: 25% (755/3017), 76.00 KiB | 13 remote: Compressing objects: 84% (835/99Receiving objects: 26% (785/3017), 76. remote: Compressing objeReceiving objects: 33% (996/3017), 76.00 KiB | 135 KiB/ remote: Compressing objects: 94% (Receiving objects: 36% (1087/3017), 76.00 Ki remote: Compressing objects: 97% (965/994) Receiving objects: 39% (1177/3017 g objects: 100% (994/994), done. remote: Total 3017 (delta 1911), reused 2988 (delta 1896) Receiving objects: 100% (3017/3017), 403.99 KiB | 335 KiB/s, done. Resolving deltas: 100% (1911/1911), done. According to strace, data from sideband channel #2 (prefixed with "remote: ") pertaining to object compression is printed way after pack data has already started to arrive locally. This is really weird. And this occurs only when fetching from repo.or.cz and not from git.kernel.org for example. So there is something to investigate on the server side. Pasky: anything you changed in your git installation lately? Nicolas --
Yes, but nothing should have changed in git-daemon, that's the only part of the infrastructure that uses system-wide git (which it perhaps shouldn't). I cannot reproduce this problem, though. I have changed git-daemon to use my local git version (about one week old master), does this still happen for you? -- Petr "Pasky" Baudis A lot of people have my books on their bookshelves. That's the problem, they need to read them. -- Don Knuth --
No, it doesn't happen anymore. What was the git-daemon version before? Nicolas --
1.5.6.5, the default version in debian lenny -- Petr "Pasky" Baudis A lot of people have my books on their bookshelves. That's the problem, they need to read them. -- Don Knuth --
Go figure. I don't see anything that would explain the difference in behavior with latest git from a quick look. Nicolas --
In theory it is possible for sideband channel #2 to be delayed if
pack data is quick to come up for sideband channel #1. And because
data for channel #2 is read only 128 bytes at a time while pack data
is read 8192 bytes at a time, it is possible for many pack blocks to
be sent to the client before the progress message fifo is emptied,
making the situation even worse. This would result in totally garbled
progress display on the client's console as local progress gets mixed
with partial remote progress lines.
Let's prevent such situations by giving transmission priority to
progress messages over pack data at all times.
Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
---
I don't see why the issue couldn't happen with latest git as well.
This patch however would plug any possibilities for this to happen again
though.
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index c4cd1e1..29446e8 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -132,7 +132,6 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
while (1) {
struct pollfd pfd[2];
- ssize_t processed[2] = { 0, 0 };
int status;
pfd[0].fd = fd1[0];
@@ -147,15 +146,14 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
}
continue;
}
- if (pfd[0].revents & POLLIN)
- /* Data stream ready */
- processed[0] = process_input(pfd[0].fd, 1);
if (pfd[1].revents & POLLIN)
/* Status stream ready */
- processed[1] = process_input(pfd[1].fd, 2);
- /* Always finish to read data when available */
- if (processed[0] || processed[1])
- continue;
+ if (process_input(pfd[1].fd, 2))
+ continue;
+ if (pfd[0].revents & POLLIN)
+ /* Data stream ready */
+ if (process_input(pfd[0].fd, 1))
+ continue;
if (waitpid(writer, &status, 0) < 0)
error_clnt("%s", lostchild);
diff --git a/upload-pack.c b/upload-pack.c
index 70badcf..6bfb500 100644
--- a/upload-pack.c
+++ ...I have reordered two top commits (those that are only in 'pu'), but I didn't improve "Create links..." commit. Current version works, but is suboptimal. I wonder if Packy has any comments about 'blame_incremental' series... -- Jakub Narebski Poland ShadeHawk on #git --
