* Re: git over rsync+ssh
From: Michael J Gruber @ 2008-07-10 10:06 UTC (permalink / raw)
To: git
In-Reply-To: <e2b179460807090802j2dbb72efo4f03c0a9e79241c0@mail.gmail.com>
Mike Ralphson venit, vidit, dixit 09.07.2008 17:02:
> 2008/7/9 Michael J Gruber <michaeljgruber+gmane@fastmail.fm>:
>> I want to put a git repo on a server where I have ssh access but failed to
>> compile git (AIX 5.1, has libz.a but no .so nor headers; compiling
>> prerequisite zlib failed, probably due to a botched build environment).
>
> I can send you a binary to try if you'd like. It would be compiled on
> AIX 5.3 but I have to jump through hoops on several non-identically
> set-up servers here, so one might work for you.
Thanks for the offer. I found an AIX 4.3 box with a working build
environment there, including zlib-devel, and I got git 1.5.6.2 compiled
now (no tcltk, don't care). Runs fine on the AIX 5.1 box, I can clone
and push happily. Yeah.
Michael
^ permalink raw reply
* Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Johannes Schindelin @ 2008-07-10 10:04 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, Michael Haggerty, Jeff King, git
In-Reply-To: <20080710054152.b051989c.chriscool@tuxfamily.org>
Hi,
On Thu, 10 Jul 2008, Christian Couder wrote:
> Before this patch, "git bisect", when it was given some good revs that
> are not ancestor of the bad rev, didn't check if the merge bases were
> good. "git bisect" just supposed that the user knew what he was doing,
> and that, when he said the revs were good, he knew that it meant that
> all the revs in the history leading to the good revs were also
> considered good.
Well, it is not completely relying on the user.
The common scenario before a bisect is this: something used to work _all
the time_, and all of a sudden, it does not anymore.
So it is expected that there is no fix in the history. Not in the current
branch, not in the "good" branch, not wherever.
In that case, you are literally guaranteed that all ancestors of a good
commit are good, too, because if there was a bad one, there would be a
fix, too.
The whole idea of "bisect" relies on that idea, that any ancestor of a
good commit is good. Otherwise you'd have to check the commits one by
one, not in a bisecting manner.
Ciao,
Dscho
^ permalink raw reply
* Re: THREADED_DELTA_SEARCH
From: Pierre Habouzit @ 2008-07-10 9:33 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Git ML
In-Reply-To: <m3hcaydswg.fsf@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]
On Thu, Jul 10, 2008 at 08:51:06AM +0000, Jakub Narebski wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > I'm using it in production for quite a long time now, and I wonder if
> > there is any specific reason why it's not default. Debian activated it
> > by default too a couple of weeks ago, which means that it's in
> > production on quite a large scale, and there are no issues reported
> > either. Would a patch making it default be accepted ? It's quite handy
> > given that SMP machines are really pervasive nowadays…
>
> Doues our ./configure detects if it makes sense (i.e. if it is SMP
> machine)?
I don't know if it does, but it wouldn't make sense for distros.
> What is the cost of THREADED_DELTA_SEARCH for single processor
> single core machine?
Threading has to be enabled in the configuration, and there is a
--thread=0 mode that enables autodetection of the number of CPUs online.
so there is no overhead for single CPU systems.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Bug: /usr/local/bin/git-daemon starts /usr/sbin/git-upload-pack?
From: Jeff King @ 2008-07-10 8:59 UTC (permalink / raw)
To: Brian Foster; +Cc: Git Mailing List, Jan Wielemaker
In-Reply-To: <200807100932.06569.brian.foster@innova-card.com>
On Thu, Jul 10, 2008 at 09:32:06AM +0200, Brian Foster wrote:
> GIT_TRACE=/tmp/LOG-git-daemon
>
> gave some valuable clews. I'm unaware of GIT_TRACE
> being documented. I also concur it's annoying neither
It's in git(1).
-Peff
^ permalink raw reply
* Re: THREADED_DELTA_SEARCH
From: Jakub Narebski @ 2008-07-10 8:51 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Git ML
In-Reply-To: <20080710075327.GD24819@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> I'm using it in production for quite a long time now, and I wonder if
> there is any specific reason why it's not default. Debian activated it
> by default too a couple of weeks ago, which means that it's in
> production on quite a large scale, and there are no issues reported
> either. Would a patch making it default be accepted ? It's quite handy
> given that SMP machines are really pervasive nowadays…
Doues our ./configure detects if it makes sense (i.e. if it is SMP
machine)? What is the cost of THREADED_DELTA_SEARCH for single processor
single core machine?
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] Fix problem with authentification on http repository.
From: Jeff King @ 2008-07-10 8:47 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Mike Hommey, jean.guyader, git, Jean Guyader
In-Reply-To: <7vk5fuqjn8.fsf@gitster.siamese.dyndns.org>
On Thu, Jul 10, 2008 at 12:32:27AM -0700, Junio C Hamano wrote:
> > Wouldn't there still be a race condition for publicly broadcasting your
> > password via ps?
>
> In addition, I think on some Unices overwriting argv[0] does not have the
> effect Dscho is aiming for.
I thought so, too, but I couldn't find any mention of it online. For
curiosity's sake, do you happen to know offhand any platform where this
doesn't work (of course, Dscho is really proposing overwriting argv[i],
but I don't know if that makes a difference to the portability)?
-Peff
^ permalink raw reply
* [PATCH] Add testcase for merge.log
From: Miklos Vajna @ 2008-07-10 8:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7ibuqisc.fsf@gitster.siamese.dyndns.org>
Make sure that the merge.log boolean config setting is the same as --log
and --no-log.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Thu, Jul 10, 2008 at 12:50:59AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> The command forgot the configuration variable when rewritten in C.
Thanks. Here is a testcase.
t/t7600-merge.sh | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index dfddd86..a3c17a9 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -456,6 +456,17 @@ test_expect_success 'merge log message' '
verify_diff msg.log msg.act "[OOPS] bad merge log message"
'
+test_expect_success 'merge log message (from config)' '
+ git reset --hard c0 &&
+ git merge c2 &&
+ git show -s --pretty=format:%b HEAD >msg.act &&
+ verify_diff msg.nolog msg.act "[OOPS] bad merge log message" &&
+ git config merge.log true &&
+ git merge c3 &&
+ git show -s --pretty=format:%b HEAD >msg.act &&
+ verify_diff msg.log msg.act "[OOPS] bad merge log message"
+'
+
test_debug 'gitk --all'
test_done
--
1.5.6.2.450.g8d367.dirty
^ permalink raw reply related
* Re: THREADED_DELTA_SEARCH
From: Johannes Sixt @ 2008-07-10 8:01 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Git ML
In-Reply-To: <20080710075327.GD24819@artemis.madism.org>
Pierre Habouzit schrieb:
> I'm using it in production for quite a long time now, and I wonder if
> there is any specific reason why it's not default. Debian activated it
> by default too a couple of weeks ago, which means that it's in
> production on quite a large scale, and there are no issues reported
> either. Would a patch making it default be accepted ? It's quite handy
> given that SMP machines are really pervasive nowadays…
Don't forget to turn it off in the MINGW section in the Makefile since we
don't have pthreads on Windows.
-- Hannes
^ permalink raw reply
* THREADED_DELTA_SEARCH
From: Pierre Habouzit @ 2008-07-10 7:53 UTC (permalink / raw)
To: Git ML
[-- Attachment #1: Type: text/plain, Size: 589 bytes --]
I'm using it in production for quite a long time now, and I wonder if
there is any specific reason why it's not default. Debian activated it
by default too a couple of weeks ago, which means that it's in
production on quite a large scale, and there are no issues reported
either. Would a patch making it default be accepted ? It's quite handy
given that SMP machines are really pervasive nowadays…
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH] Teach merge.log to "git-merge" again
From: Junio C Hamano @ 2008-07-10 7:50 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
The command forgot the configuration variable when rewritten in C.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-merge.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 821edcd..bc3f921 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -519,8 +519,10 @@ int git_merge_config(const char *k, const char *v, void *cb)
else if (!strcmp(k, "pull.twohead"))
return git_config_string(&pull_twohead, k, v);
else if (!strcmp(k, "pull.octopus"))
return git_config_string(&pull_octopus, k, v);
+ else if (!strcmp(k, "merge.log") || !strcmp(k, "merge.summary"))
+ option_log = git_config_bool(k, v);
return git_diff_ui_config(k, v, cb);
}
static int read_tree_trivial(unsigned char *common, unsigned char *head,
^ permalink raw reply related
* Re: Bug: /usr/local/bin/git-daemon starts /usr/sbin/git-upload-pack?
From: Jan Wielemaker @ 2008-07-10 7:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vlk0a7obl.fsf@gitster.siamese.dyndns.org>
Hi Junio,
On Wednesday 09 July 2008 23:14:06 Junio C Hamano wrote:
> Jan Wielemaker <J.Wielemaker@uva.nl> writes:
> > After upgrade to SuSE 11.0 I was forced to update GIT (changed libcurl).
> > I did (with some trickery) a pull of the latest git and built it using
> > make prefix=/usr/local ..., to find out that /usr/local/bin/git-daemon
> > starts /usr/sbin/git-upload-pack.
> >
> > After creating a symlink to /usr/local/bin/git-upload-pack all works fine
> > again, but I guess this is a mistake?
>
> I think this is a distro issue.
>
> Pre-1.6.0 git installs git-everything in $(bindir) and knows things are
> there. Current 'master' we are preparing for 1.6.0 instead installs
> git-mostofthem in $(prefix)/libexec/git-core and git-someofthem in
> $(bindir); our executables know which binaries are installed in $(bindir)
> and which ones are installed in $(prefix)/libexec/git-core.
>
> The point is we never install $anywhere/sbin unless the person who is
> building explicitly does so (either by futzing the Makefile or giving
> bindir=$somewhere from the command line of "make"). The reason your
> /usr/local/bin/git-daemon (we do not know who compiled it and how) spawns
> something in /usr/sbin is not our doing. Find out who configured it and
> why, and more importantly, what _other_ changes are made in the build and
> installation procedure to support that change in location.
If you read carefully you see some "I ..." :-) Summarising to avoid all
confusion:
* I had git installed in /usr/local/bin, this was git from git,
close to 1.5.6. The machine was running SuSE 10.2.
* I installed SuSE 11.0, whiping /, but git remained as
/usr/local is on a seperate partition.
* Git refused to run (dependency on libcurl.so.3; 11.0 ships with
libcurl.so.4).
* I copied libcurl.so.3 from backup into /usr/lib64, which at least
made git run again.
* Then I did
% git pull
% git clean -x -f
% make prefix=/usr/local all doc info
# make prefix=/usr/local install install-doc install-info
* All went smooth, but clients to git:// complained that the other site
hung up unexpectedly.
* Ran git-daemon --verbose --export-all --user=git --port=4000 /some/path
* Ran strace -f -p <pid>
* Ran git clone git://localhost:4000/some/path to find it was trying to
start /usr/sbin/git-upload-pack.
I understand bit changes are underway, so it might not be worthwhile to try
and
get to the bottom of this.
Cheers --- Jan
^ permalink raw reply
* Re: [PATCH] revisions: refactor handle_revision_opt into parse_revision_opt.
From: Pierre Habouzit @ 2008-07-10 7:40 UTC (permalink / raw)
To: Jeff King, git, gitster
In-Reply-To: <20080710073450.GA24819@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 635 bytes --]
On Thu, Jul 10, 2008 at 07:34:50AM +0000, Pierre Habouzit wrote:
> and have a full parse-opt thing from top to bottom. the "--reverse" hack
> could be done really differently, because we really know what
> "--children" does and we could directly do what the revision option
> parser does.
I mean in a clean way, given what --children does, it would probably
implemented by a callback in revision.c, so it's easy for git-blame to
call it too.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] revisions: refactor handle_revision_opt into parse_revision_opt.
From: Junio C Hamano @ 2008-07-10 7:36 UTC (permalink / raw)
To: Jeff King; +Cc: Pierre Habouzit, git, gitster
In-Reply-To: <20080710071418.GD3195@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> but it also seems like the top bit of that for loop is boilerplate, too:
>
>> for (;;) {
>> switch (parse_options_step(&ctx, options, shortlog_usage)) {
>> case PARSE_OPT_HELP:
>> exit(129);
>> case PARSE_OPT_DONE:
>> goto parse_done;
>> }
Another thing we should be able to refactor is the option help formatter.
Cf. 071438a (Teach git-merge -X<option> again., 2008-07-09)
^ permalink raw reply
* Re: [PATCH] revisions: refactor handle_revision_opt into parse_revision_opt.
From: Pierre Habouzit @ 2008-07-10 7:34 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster
In-Reply-To: <20080710071418.GD3195@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2887 bytes --]
On Thu, Jul 10, 2008 at 07:14:18AM +0000, Jeff King wrote:
> On Wed, Jul 09, 2008 at 11:38:34PM +0200, Pierre Habouzit wrote:
>
> > It seems we're using handle_revision_opt the same way each time, have a
> > wrapper around it that does the 9-liner we copy each time instead.
> >
> > handle_revision_opt can be static in the module for now, it's always
> > possible to make it public again if needed.
>
> I have been on the road, and I finally got the chance to read through
> your whole parseopt/blame refactoring. I think it looks good overall, as
> do these patches.
>
> I have one comment, though I am not sure it is worth implementing.
>
> I was happy to see this refactoring, which I think improves readability:
>
> > - n = handle_revision_opt(&revs, ctx.argc, ctx.argv,
> > - &ctx.cpidx, ctx.out);
> > - if (n <= 0) {
> > - error("unknown option `%s'", ctx.argv[0]);
> > - usage_with_options(blame_opt_usage, options);
> > - }
> > - ctx.argv += n;
> > - ctx.argc -= n;
> > + parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
>
> but it also seems like the top bit of that for loop is boilerplate, too:
>
> > for (;;) {
> > switch (parse_options_step(&ctx, options, shortlog_usage)) {
> > case PARSE_OPT_HELP:
> > exit(129);
> > case PARSE_OPT_DONE:
> > goto parse_done;
> > }
>
> AFAICT, the main reason for not folding this into your refactored
> function is that after the parse_options_step, but before we handle the
> revision arg to parse_revision_opt, there needs to be an opportunity for
> the caller to intercept and do something based on revision opts (like
> blame does with --reverse):
>
> if (!strcmp(ctx.argv[0], "--reverse")) {
> ctx.argv[0] = "--children";
> reverse = 1;
> }
The other thing I would like to do is remove the exit(129) and replace
it with proper documentation for the rev-list options, and this will
depend upon the fact that we parse revisions or something else inside
the loop. Of course this is boilerplate, but well, I wouldn't like to
hide it and prevent people from thinking they can hook other things in
there.
And I'm not very keen on adding more options to parse-options like you
propose, our endgame is to get rid of parse_revision_opt in this form
and have a full parse-opt thing from top to bottom. the "--reverse" hack
could be done really differently, because we really know what
"--children" does and we could directly do what the revision option
parser does. But oh well... For now I'm more interested to see more
commands migrated thanks to this facility, and see what we can refactor
to get rid of the old parsers at once.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Fix problem with authentification on http repository.
From: Junio C Hamano @ 2008-07-10 7:32 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Mike Hommey, jean.guyader, git, Jean Guyader
In-Reply-To: <20080710072420.GA5167@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jul 09, 2008 at 01:43:07PM +0200, Johannes Schindelin wrote:
>
>> > Note that http://user:pass@server/path/ /should/ work (but that would
>> > need validation), though not a good idea on command line.
>>
>> Well, now that the programs using URLs are all builtins, we can actually
>> do something about it. We can edit out the "user[:pass]@" part out of
>> argv, which usually means that "ps" will not see it anymore.
>
> Wouldn't there still be a race condition for publicly broadcasting your
> password via ps?
In addition, I think on some Unices overwriting argv[0] does not have the
effect Dscho is aiming for.
I thought we do discourage [:pass] part in our documentation.
^ permalink raw reply
* Re: Bug: /usr/local/bin/git-daemon starts /usr/sbin/git-upload-pack?
From: Brian Foster @ 2008-07-10 7:32 UTC (permalink / raw)
To: Git Mailing List; +Cc: Jan Wielemaker
In-Reply-To: <200807092220.10655.J.Wielemaker@uva.nl>
On Wednesday 09 July 2008 22:20:10 Jan Wielemaker wrote:
>[ ... ]
> P.s. This [ git-daemon using a weird path to exec git-upload-pack
> was ] quite nasty to debug. I was forced to to run git-daemon
> stand-alone (not xinetd) another port and run strace -f -p <pid>
> to discover the cause of this problem. Even with --verbose, the
> only error response was the client complaining on unexpected EOF.
Just a hint: I had some odd exec problems in the past
(as it happens, also from xinetd and also path-related,
but not(?) your problem), and found that exporting
GIT_TRACE=/tmp/LOG-git-daemon
gave some valuable clews. I'm unaware of GIT_TRACE
being documented. I also concur it's annoying neither
--syslog nor --verbose seems to cause these sort of
errors to be logged.
cheers!
-blf-
--
“How many surrealists does it take to | Brian Foster
change a lightbulb? Three. One calms | somewhere in south of France
the warthog, and two fill the bathtub | Stop E$$o (ExxonMobil)!
with brightly-coloured machine tools.” | http://www.stopesso.com
^ permalink raw reply
* Re: [PATCH] Fix problem with authentification on http repository.
From: Jeff King @ 2008-07-10 7:24 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Mike Hommey, Junio C Hamano, jean.guyader, git, Jean Guyader
In-Reply-To: <alpine.DEB.1.00.0807091341230.5277@eeepc-johanness>
On Wed, Jul 09, 2008 at 01:43:07PM +0200, Johannes Schindelin wrote:
> > Note that http://user:pass@server/path/ /should/ work (but that would
> > need validation), though not a good idea on command line.
>
> Well, now that the programs using URLs are all builtins, we can actually
> do something about it. We can edit out the "user[:pass]@" part out of
> argv, which usually means that "ps" will not see it anymore.
Wouldn't there still be a race condition for publicly broadcasting your
password via ps?
-Peff
^ permalink raw reply
* Re: [PATCH] revisions: refactor handle_revision_opt into parse_revision_opt.
From: Jeff King @ 2008-07-10 7:14 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, gitster
In-Reply-To: <1215639514-1612-2-git-send-email-madcoder@debian.org>
On Wed, Jul 09, 2008 at 11:38:34PM +0200, Pierre Habouzit wrote:
> It seems we're using handle_revision_opt the same way each time, have a
> wrapper around it that does the 9-liner we copy each time instead.
>
> handle_revision_opt can be static in the module for now, it's always
> possible to make it public again if needed.
I have been on the road, and I finally got the chance to read through
your whole parseopt/blame refactoring. I think it looks good overall, as
do these patches.
I have one comment, though I am not sure it is worth implementing.
I was happy to see this refactoring, which I think improves readability:
> - n = handle_revision_opt(&revs, ctx.argc, ctx.argv,
> - &ctx.cpidx, ctx.out);
> - if (n <= 0) {
> - error("unknown option `%s'", ctx.argv[0]);
> - usage_with_options(blame_opt_usage, options);
> - }
> - ctx.argv += n;
> - ctx.argc -= n;
> + parse_revision_opt(&revs, &ctx, options, blame_opt_usage);
but it also seems like the top bit of that for loop is boilerplate, too:
> for (;;) {
> switch (parse_options_step(&ctx, options, shortlog_usage)) {
> case PARSE_OPT_HELP:
> exit(129);
> case PARSE_OPT_DONE:
> goto parse_done;
> }
AFAICT, the main reason for not folding this into your refactored
function is that after the parse_options_step, but before we handle the
revision arg to parse_revision_opt, there needs to be an opportunity for
the caller to intercept and do something based on revision opts (like
blame does with --reverse):
if (!strcmp(ctx.argv[0], "--reverse")) {
ctx.argv[0] = "--children";
reverse = 1;
}
But I wonder if it would be a suitable alternative to just add
"--reverse" in this case to the blame options, but with an option flag
for "parse me, but also pass me along to the next parser" (which would
be added). Then we could do our thing in a callback.
Of course, in this case, we do something a bit tricky by actually
_rewriting_ the argument to "--children". So we would have to have
support for callbacks rewriting arguments, or it would have to manually
do what "--children" should do. So perhaps it isn't worth the trouble.
This particular boilerplate is at least not very error-prone.
So food for thought, mainly, I suppose. Apologies if you already thought
of this and I missed the discussion. I think I am up to date on my
back-reading of the git list, but it is easy to lose some threads. :)
-Peff
^ permalink raw reply
* Re: [PATCH 2/4] git-imap-send: Add support for SSL.
From: Jeff King @ 2008-07-10 6:39 UTC (permalink / raw)
To: Rob Shearman; +Cc: Josh Triplett, git
In-Reply-To: <1096648c0807091424g1e10d0ccrae0be929ec428b89@mail.gmail.com>
On Wed, Jul 09, 2008 at 10:24:27PM +0100, Rob Shearman wrote:
> >> [imap]
> >> + sslverify = false
> > [...]
> >> + else if (!strcmp( "ssl_verify", key ))
> >
> > The example and the code disagree on the name of the
> > sslverify/ssl_verify option.
>
> I wouldn't exactly call it "disagree". The config variable is limited
> by not allowing underscores, whereas the C language does allow them.
I think his point is that the example says "sslverify" but the code is
looking for the config variable "ssl_verify". So that config won't work.
However as you mention, underscore isn't allowed, so your strcmp line is
totally bogus anyway, but you silently fix it in your 3/4 "style" fix.
-Peff
^ permalink raw reply
* [JGIT PATCH 5/5] Explicitly capture the stderr from a failed SSH fetch or push
From: Shawn O. Pearce @ 2008-07-10 6:13 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1215670403-19191-5-git-send-email-spearce@spearce.org>
If the remote command name is not found on the remote system we are
likely to get a shell error sent to the channel's stderr stream,
and yet the channel is connected. So we don't see the problem
until we try to read the advertised refs, which is somewhat late.
If we get EOF before the first advertised ref it is a very good
indication that the remote side did not start the process we wanted
it to. Tossing a specialized exception allows the SSH transport
to offer up the contents of the stderr channel (if any) as possible
indication of why the repository does not exist.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../spearce/egit/ui/EclipseSshSessionFactory.java | 5 +-
.../jgit/errors/NoRemoteRepositoryException.java | 59 ++++++++++++++++++++
.../spearce/jgit/transport/BasePackConnection.java | 3 +-
.../jgit/transport/DefaultSshSessionFactory.java | 30 ++++++++++-
.../spearce/jgit/transport/TransportGitSsh.java | 34 +++++++++++-
5 files changed, 126 insertions(+), 5 deletions(-)
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/NoRemoteRepositoryException.java
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
index 144d47d..8f80373 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java
@@ -55,7 +55,10 @@ class EclipseSshSessionFactory extends SshSessionFactory {
StringBuilder sb = new StringBuilder();
public String toString() {
- return all.toString();
+ String r = all.toString();
+ while (r.endsWith("\n"))
+ r = r.substring(0, r.length() - 1);
+ return r;
}
@Override
diff --git a/org.spearce.jgit/src/org/spearce/jgit/errors/NoRemoteRepositoryException.java b/org.spearce.jgit/src/org/spearce/jgit/errors/NoRemoteRepositoryException.java
new file mode 100644
index 0000000..604ec4d
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/errors/NoRemoteRepositoryException.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.errors;
+
+import org.spearce.jgit.transport.URIish;
+
+/**
+ * Indicates a remote repository does not exist.
+ */
+public class NoRemoteRepositoryException extends TransportException {
+ private static final long serialVersionUID = 1L;
+
+ /**
+ * Constructs an exception indicating a repository does not exist.
+ *
+ * @param uri
+ * URI used for transport
+ * @param s
+ * message
+ */
+ public NoRemoteRepositoryException(final URIish uri, final String s) {
+ super(uri, s);
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
index 7dc4620..52f3f48 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
@@ -49,6 +49,7 @@ import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Set;
+import org.spearce.jgit.errors.NoRemoteRepositoryException;
import org.spearce.jgit.errors.PackProtocolException;
import org.spearce.jgit.errors.TransportException;
import org.spearce.jgit.lib.ObjectId;
@@ -129,7 +130,7 @@ abstract class BasePackConnection extends BaseConnection {
line = pckIn.readString();
} catch (EOFException eof) {
if (avail.isEmpty())
- throw new TransportException(uri, "not found.");
+ throw new NoRemoteRepositoryException(uri, "not found.");
throw eof;
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java b/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
index 5924a04..b4578d4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java
@@ -253,6 +253,34 @@ class DefaultSshSessionFactory extends SshSessionFactory {
@Override
public OutputStream getErrorStream() {
- return System.err;
+ return new OutputStream() {
+ private StringBuilder all = new StringBuilder();
+
+ private StringBuilder sb = new StringBuilder();
+
+ public String toString() {
+ String r = all.toString();
+ while (r.endsWith("\n"))
+ r = r.substring(0, r.length() - 1);
+ return r;
+ }
+
+ @Override
+ public void write(final int b) throws IOException {
+ if (b == '\r') {
+ System.err.print('\r');
+ return;
+ }
+
+ sb.append((char) b);
+
+ if (b == '\n') {
+ final String line = sb.toString();
+ System.err.print(line);
+ all.append(line);
+ sb = new StringBuilder();
+ }
+ }
+ };
}
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
index 9a6c719..3f2cd37 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
@@ -44,6 +44,7 @@ import java.io.OutputStream;
import java.net.ConnectException;
import java.net.UnknownHostException;
+import org.spearce.jgit.errors.NoRemoteRepositoryException;
import org.spearce.jgit.errors.TransportException;
import org.spearce.jgit.lib.Repository;
@@ -217,6 +218,25 @@ class TransportGitSsh extends PackTransport {
}
}
+ NoRemoteRepositoryException cleanNotFound(NoRemoteRepositoryException nf) {
+ String why = errStream.toString();
+ if (why == null || why.length() == 0)
+ return nf;
+
+ String path = uri.getPath();
+ if (uri.getScheme() != null && uri.getPath().startsWith("/~"))
+ path = uri.getPath().substring(1);
+
+ final StringBuilder pfx = new StringBuilder();
+ pfx.append("fatal: ");
+ sqAlways(pfx, path);
+ pfx.append(": ");
+ if (why.startsWith(pfx.toString()))
+ why = why.substring(pfx.length());
+
+ return new NoRemoteRepositoryException(uri, why);
+ }
+
class SshFetchConnection extends BasePackFetchConnection {
private ChannelExec channel;
@@ -238,7 +258,12 @@ class TransportGitSsh extends PackTransport {
throw new TransportException(uri,
"remote hung up unexpectedly", err);
}
- readAdvertisedRefs();
+
+ try {
+ readAdvertisedRefs();
+ } catch (NoRemoteRepositoryException notFound) {
+ throw cleanNotFound(notFound);
+ }
}
@Override
@@ -277,7 +302,12 @@ class TransportGitSsh extends PackTransport {
throw new TransportException(uri,
"remote hung up unexpectedly", err);
}
- readAdvertisedRefs();
+
+ try {
+ readAdvertisedRefs();
+ } catch (NoRemoteRepositoryException notFound) {
+ throw cleanNotFound(notFound);
+ }
}
@Override
--
1.5.6.2.393.g45096
^ permalink raw reply related
* [JGIT PATCH 3/5] Reuse the same SSH connection when automatically fetching tags
From: Shawn O. Pearce @ 2008-07-10 6:13 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1215670403-19191-3-git-send-email-spearce@spearce.org>
If we have to open a second connection to git-upload-pack in order
to automatically follow and fetch annotated tags we may be able to
reuse the same JSch Session object and simply run the second stream
over the already established, encrypted TCP stream.
Reusing the Session avoids the overheads associated with performing
public/private key authentication and reduces the overall latency
of the fetch process.
To make reuse work we cache the Session at the Transport instance
level and ask Transport API users to close the Transport once they
are completely done talking to this remote repository. While the
Transport instance is unclosed however multiple FetchConnection
and/or PushConnections can be established, with each subsequent
connection being cheaper to setup.
In the future we may also support reusing the same Session onto
different Transport instances, permitting access to multiple
repositories hosted on the same remote SSH server.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../org/spearce/egit/core/op/CloneOperation.java | 10 +++-
.../egit/ui/internal/clone/SourceBranchPage.java | 1 +
.../spearce/jgit/transport/PushProcessTest.java | 5 ++
.../org/spearce/jgit/transport/TransportTest.java | 10 +++
.../src/org/spearce/jgit/pgm/Fetch.java | 17 ++++--
.../src/org/spearce/jgit/pgm/LsRemote.java | 1 +
.../src/org/spearce/jgit/transport/Transport.java | 10 +++
.../spearce/jgit/transport/TransportAmazonS3.java | 5 ++
.../spearce/jgit/transport/TransportBundle.java | 5 ++
.../spearce/jgit/transport/TransportGitAnon.java | 5 ++
.../spearce/jgit/transport/TransportGitSsh.java | 60 +++++++++-----------
.../org/spearce/jgit/transport/TransportHttp.java | 5 ++
.../org/spearce/jgit/transport/TransportLocal.java | 5 ++
.../org/spearce/jgit/transport/TransportSftp.java | 52 +++++++++---------
14 files changed, 124 insertions(+), 67 deletions(-)
diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/op/CloneOperation.java b/org.spearce.egit.core/src/org/spearce/egit/core/op/CloneOperation.java
index 7600e3b..656f3cb 100644
--- a/org.spearce.egit.core/src/org/spearce/egit/core/op/CloneOperation.java
+++ b/org.spearce.egit.core/src/org/spearce/egit/core/op/CloneOperation.java
@@ -85,8 +85,14 @@ public class CloneOperation implements IRunnableWithProgress {
private void doFetch(final IProgressMonitor monitor)
throws NotSupportedException, TransportException {
- fetchResult = Transport.open(local, remote).fetch(
- new EclipseGitProgressTransformer(monitor), null);
+ final Transport tn = Transport.open(local, remote);
+ try {
+ final EclipseGitProgressTransformer pm;
+ pm = new EclipseGitProgressTransformer(monitor);
+ fetchResult = tn.fetch(pm, null);
+ } finally {
+ tn.close();
+ }
}
private void doCheckout(final IProgressMonitor monitor) throws IOException {
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
index b704aaa..b0aba1e 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
@@ -241,6 +241,7 @@ class SourceBranchPage extends WizardPage {
adv = fn.getRefs();
} finally {
fn.close();
+ tn.close();
}
final Ref idHEAD = fn.getRef(Constants.HEAD);
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/PushProcessTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/PushProcessTest.java
index fae1cbb..357e6b7 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/PushProcessTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/PushProcessTest.java
@@ -379,6 +379,11 @@ public class PushProcessTest extends RepositoryTestCase {
TransportException {
return new MockPushConnection();
}
+
+ @Override
+ public void close() {
+ // nothing here
+ }
}
private class MockPushConnection extends BaseConnection implements
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
index 10f6651..dc1cb21 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/TransportTest.java
@@ -56,6 +56,16 @@ public class TransportTest extends RepositoryTestCase {
final RepositoryConfig config = db.getConfig();
remoteConfig = new RemoteConfig(config, "test");
remoteConfig.addURI(new URIish("http://everyones.loves.git/u/2"));
+ transport = null;
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ if (transport != null) {
+ transport.close();
+ transport = null;
+ }
+ super.tearDown();
}
/**
diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/Fetch.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/Fetch.java
index 36a0592..c361c26 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/Fetch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/Fetch.java
@@ -63,12 +63,17 @@ class Fetch extends TextBuiltin {
args = new String[] { "origin" };
final Transport tn = Transport.open(db, args[argi++]);
- final List<RefSpec> toget = new ArrayList<RefSpec>();
- for (; argi < args.length; argi++)
- toget.add(new RefSpec(args[argi]));
- final FetchResult r = tn.fetch(new TextProgressMonitor(), toget);
- if (r.getTrackingRefUpdates().isEmpty())
- return;
+ final FetchResult r;
+ try {
+ final List<RefSpec> toget = new ArrayList<RefSpec>();
+ for (; argi < args.length; argi++)
+ toget.add(new RefSpec(args[argi]));
+ r = tn.fetch(new TextProgressMonitor(), toget);
+ if (r.getTrackingRefUpdates().isEmpty())
+ return;
+ } finally {
+ tn.close();
+ }
out.print("From ");
out.print(tn.getURI().setPass(null));
diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/LsRemote.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/LsRemote.java
index dbdfeb3..21e02ec 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/LsRemote.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/LsRemote.java
@@ -70,6 +70,7 @@ class LsRemote extends TextBuiltin {
}
} finally {
c.close();
+ tn.close();
}
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java
index b962162..5bec4d2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java
@@ -527,6 +527,16 @@ public abstract class Transport {
public abstract PushConnection openPush() throws NotSupportedException,
TransportException;
+ /**
+ * Close any resources used by this transport.
+ * <p>
+ * If the remote repository is contacted by a network socket this method
+ * must close that network socket, disconnecting the two peers. If the
+ * remote repository is actually local (same system) this method must close
+ * any open file handles used to read the "remote" repository.
+ */
+ public abstract void close();
+
private Collection<RefSpec> expandPushWildcardsFor(
final Collection<RefSpec> specs) {
final Map<String, Ref> localRefs = local.getAllRefs();
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
index cd62c5b..9aa2567 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
@@ -159,6 +159,11 @@ class TransportAmazonS3 extends WalkTransport {
return r;
}
+ @Override
+ public void close() {
+ // No explicit connections are maintained.
+ }
+
class DatabaseS3 extends WalkRemoteObjectDatabase {
private final String bucketName;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
index 6169179..24d49eb 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
@@ -101,6 +101,11 @@ class TransportBundle extends PackTransport {
"Push is not supported for bundle transport");
}
+ @Override
+ public void close() {
+ // Resources must be established per-connection.
+ }
+
class BundleFetchConnection extends BaseFetchConnection {
FileInputStream in;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitAnon.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitAnon.java
index 8a78099..a80c335 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitAnon.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitAnon.java
@@ -77,6 +77,11 @@ class TransportGitAnon extends PackTransport {
return new TcpPushConnection();
}
+ @Override
+ public void close() {
+ // Resources must be established per-connection.
+ }
+
Socket openConnection() throws TransportException {
final int port = uri.getPort() > 0 ? uri.getPort() : GIT_PORT;
try {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
index 1bbdf04..b169f4c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
@@ -78,7 +78,10 @@ class TransportGitSsh extends PackTransport {
return false;
}
- final SshSessionFactory sch;
+ private final SshSessionFactory sch;
+
+ private Session sock;
+
OutputStream errStream;
TransportGitSsh(final Repository local, final URIish uri) {
@@ -96,6 +99,17 @@ class TransportGitSsh extends PackTransport {
return new SshPushConnection();
}
+ @Override
+ public void close() {
+ if (sock != null) {
+ try {
+ sch.releaseSession(sock);
+ } finally {
+ sock = null;
+ }
+ }
+ }
+
private static void sqMinimal(final StringBuilder cmd, final String val) {
if (val.matches("^[a-zA-Z0-9._/-]*$")) {
// If the string matches only generally safe characters
@@ -152,17 +166,18 @@ class TransportGitSsh extends PackTransport {
cmd.append('\'');
}
- Session openSession() throws TransportException {
+ private void initSession() throws TransportException {
+ if (sock != null)
+ return;
+
final String user = uri.getUser();
final String pass = uri.getPass();
final String host = uri.getHost();
final int port = uri.getPort();
try {
- final Session session;
- session = sch.getSession(user, pass, host, port);
- if (!session.isConnected())
- session.connect();
- return session;
+ sock = sch.getSession(user, pass, host, port);
+ if (!sock.isConnected())
+ sock.connect();
} catch (JSchException je) {
final Throwable c = je.getCause();
if (c instanceof UnknownHostException)
@@ -173,8 +188,9 @@ class TransportGitSsh extends PackTransport {
}
}
- ChannelExec exec(final Session sock, final String exe)
- throws TransportException {
+ ChannelExec exec(final String exe) throws TransportException {
+ initSession();
+
try {
final ChannelExec channel = (ChannelExec) sock.openChannel("exec");
String path = uri.getPath();
@@ -202,15 +218,12 @@ class TransportGitSsh extends PackTransport {
}
class SshFetchConnection extends BasePackFetchConnection {
- private Session session;
-
private ChannelExec channel;
SshFetchConnection() throws TransportException {
super(TransportGitSsh.this);
try {
- session = openSession();
- channel = exec(session, getOptionUploadPack());
+ channel = exec(getOptionUploadPack());
if (channel.isConnected())
init(channel.getInputStream(), channel.getOutputStream());
@@ -240,27 +253,16 @@ class TransportGitSsh extends PackTransport {
channel = null;
}
}
-
- if (session != null) {
- try {
- sch.releaseSession(session);
- } finally {
- session = null;
- }
- }
}
}
class SshPushConnection extends BasePackPushConnection {
- private Session session;
-
private ChannelExec channel;
SshPushConnection() throws TransportException {
super(TransportGitSsh.this);
try {
- session = openSession();
- channel = exec(session, getOptionReceivePack());
+ channel = exec(getOptionReceivePack());
init(channel.getInputStream(), channel.getOutputStream());
} catch (TransportException err) {
close();
@@ -285,14 +287,6 @@ class TransportGitSsh extends PackTransport {
channel = null;
}
}
-
- if (session != null) {
- try {
- sch.releaseSession(session);
- } finally {
- session = null;
- }
- }
}
}
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportHttp.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportHttp.java
index 9351a12..1357e58 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportHttp.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportHttp.java
@@ -106,6 +106,11 @@ class TransportHttp extends WalkTransport {
return r;
}
+ @Override
+ public void close() {
+ // No explicit connections are maintained.
+ }
+
class HttpObjectDB extends WalkRemoteObjectDatabase {
private final URL objectsUrl;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
index 155d59f..d74f1b3 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
@@ -93,6 +93,11 @@ class TransportLocal extends PackTransport {
return new LocalPushConnection();
}
+ @Override
+ public void close() {
+ // Resources must be established per-connection.
+ }
+
protected Process startProcessWithErrStream(final String cmd)
throws TransportException {
try {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
index c2cbe6a..6a5df07 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
@@ -91,7 +91,9 @@ class TransportSftp extends WalkTransport {
return uri.isRemote() && "sftp".equals(uri.getScheme());
}
- final SshSessionFactory sch;
+ private final SshSessionFactory sch;
+
+ private Session sock;
TransportSftp(final Repository local, final URIish uri) {
super(local, uri);
@@ -114,17 +116,29 @@ class TransportSftp extends WalkTransport {
return r;
}
- Session openSession() throws TransportException {
+ @Override
+ public void close() {
+ if (sock != null) {
+ try {
+ sch.releaseSession(sock);
+ } finally {
+ sock = null;
+ }
+ }
+ }
+
+ private void initSession() throws TransportException {
+ if (sock != null)
+ return;
+
final String user = uri.getUser();
final String pass = uri.getPass();
final String host = uri.getHost();
final int port = uri.getPort();
try {
- final Session session;
- session = sch.getSession(user, pass, host, port);
- if (!session.isConnected())
- session.connect();
- return session;
+ sock = sch.getSession(user, pass, host, port);
+ if (!sock.isConnected())
+ sock.connect();
} catch (JSchException je) {
final Throwable c = je.getCause();
if (c instanceof UnknownHostException)
@@ -135,7 +149,9 @@ class TransportSftp extends WalkTransport {
}
}
- ChannelSftp open(final Session sock) throws TransportException {
+ ChannelSftp newSftp() throws TransportException {
+ initSession();
+
try {
final Channel channel = sock.openChannel("sftp");
channel.connect();
@@ -148,10 +164,6 @@ class TransportSftp extends WalkTransport {
class SftpObjectDB extends WalkRemoteObjectDatabase {
private final String objectsPath;
- private final boolean sessionOwner;
-
- private Session session;
-
private ChannelSftp ftp;
SftpObjectDB(String path) throws TransportException {
@@ -160,9 +172,7 @@ class TransportSftp extends WalkTransport {
if (path.startsWith("~/"))
path = path.substring(2);
try {
- session = openSession();
- sessionOwner = true;
- ftp = TransportSftp.this.open(session);
+ ftp = newSftp();
ftp.cd(path);
ftp.cd("objects");
objectsPath = ftp.pwd();
@@ -177,10 +187,8 @@ class TransportSftp extends WalkTransport {
SftpObjectDB(final SftpObjectDB parent, final String p)
throws TransportException {
- sessionOwner = false;
- session = parent.session;
try {
- ftp = TransportSftp.this.open(session);
+ ftp = newSftp();
ftp.cd(parent.objectsPath);
ftp.cd(p);
objectsPath = ftp.pwd();
@@ -452,14 +460,6 @@ class TransportSftp extends WalkTransport {
ftp = null;
}
}
-
- if (sessionOwner && session != null) {
- try {
- sch.releaseSession(session);
- } finally {
- session = null;
- }
- }
}
}
}
--
1.5.6.2.393.g45096
^ permalink raw reply related
* [JGIT PATCH 2/5] Don't display passwords on the console in fetch/push output
From: Shawn O. Pearce @ 2008-07-10 6:13 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1215670403-19191-2-git-send-email-spearce@spearce.org>
When we show the URI we just fetched or pushed against there may
be a user password embedded in that URI, as saved in the user's
.git/config file. We shouldn't display that in public to prying
eyes so nulling it out will give us a copy of the URI without that
field in it.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/pgm/Fetch.java | 2 +-
.../src/org/spearce/jgit/pgm/Push.java | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/Fetch.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/Fetch.java
index c9c997e..36a0592 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/Fetch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/Fetch.java
@@ -71,7 +71,7 @@ class Fetch extends TextBuiltin {
return;
out.print("From ");
- out.print(tn.getURI());
+ out.print(tn.getURI().setPass(null));
out.println();
for (final TrackingRefUpdate u : r.getTrackingRefUpdates()) {
final char type = shortTypeOf(u.getResult());
diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/Push.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/Push.java
index cbdf465..8411a11 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/Push.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/Push.java
@@ -149,7 +149,7 @@ class Push extends TextBuiltin {
final RemoteRefUpdate rru) {
if (first) {
first = false;
- out.format("To %s\n", transport.getURI());
+ out.format("To %s\n", transport.getURI().setPass(null));
}
final String remoteName = rru.getRemoteName();
--
1.5.6.2.393.g45096
^ permalink raw reply related
* [JGIT PATCH 4/5] Report remote SSH execution errors during push via TransportException
From: Shawn O. Pearce @ 2008-07-10 6:13 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1215670403-19191-4-git-send-email-spearce@spearce.org>
If the remote side failed to execute git-receive-pack we may
have the reason why on the stderr stream of the channel, as
the remote shell may have failed execution.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../spearce/jgit/transport/TransportGitSsh.java | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
index b169f4c..9a6c719 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
@@ -228,7 +228,7 @@ class TransportGitSsh extends PackTransport {
if (channel.isConnected())
init(channel.getInputStream(), channel.getOutputStream());
else
- throw new TransportException(errStream.toString());
+ throw new TransportException(uri, errStream.toString());
} catch (TransportException err) {
close();
@@ -263,7 +263,12 @@ class TransportGitSsh extends PackTransport {
super(TransportGitSsh.this);
try {
channel = exec(getOptionReceivePack());
- init(channel.getInputStream(), channel.getOutputStream());
+
+ if (channel.isConnected())
+ init(channel.getInputStream(), channel.getOutputStream());
+ else
+ throw new TransportException(uri, errStream.toString());
+
} catch (TransportException err) {
close();
throw err;
--
1.5.6.2.393.g45096
^ permalink raw reply related
* [JGIT PATCH 1/5] Include a progress meter for large uploads to Amazon S3
From: Shawn O. Pearce @ 2008-07-10 6:13 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1215670403-19191-1-git-send-email-spearce@spearce.org>
If we are uploading a sizable pack file or idx file to the S3
service the upload happens after we have finished writing the pack
to a local temporary file. This causes a long pause for the user
while they wait for the data transfer to complete, with no real
indication of progress happening.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/transport/AmazonS3.java | 29 +++++++++++++++----
.../spearce/jgit/transport/TransportAmazonS3.java | 7 +++-
.../org/spearce/jgit/transport/TransportSftp.java | 5 +++-
.../spearce/jgit/transport/WalkPushConnection.java | 5 ++-
.../jgit/transport/WalkRemoteObjectDatabase.java | 11 ++++++-
5 files changed, 44 insertions(+), 13 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java b/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
index e8575aa..59337f8 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
@@ -75,6 +75,8 @@ import javax.crypto.spec.SecretKeySpec;
import org.spearce.jgit.awtui.AwtAuthenticator;
import org.spearce.jgit.lib.Constants;
+import org.spearce.jgit.lib.NullProgressMonitor;
+import org.spearce.jgit.lib.ProgressMonitor;
import org.spearce.jgit.util.Base64;
import org.spearce.jgit.util.HttpSupport;
import org.spearce.jgit.util.TemporaryBuffer;
@@ -376,7 +378,7 @@ public class AmazonS3 {
// We have to copy to produce the cipher text anyway so use
// the large object code path as it supports that behavior.
//
- final OutputStream os = beginPut(bucket, key);
+ final OutputStream os = beginPut(bucket, key, null, null);
os.write(data);
os.close();
return;
@@ -430,11 +432,17 @@ public class AmazonS3 {
* name of the bucket storing the object.
* @param key
* key of the object within its bucket.
+ * @param monitor
+ * (optional) progress monitor to post upload completion to
+ * during the stream's close method.
+ * @param monitorTask
+ * (optional) task name to display during the close method.
* @return a stream which accepts the new data, and transmits once closed.
* @throws IOException
* if encryption was enabled it could not be configured.
*/
- public OutputStream beginPut(final String bucket, final String key)
+ public OutputStream beginPut(final String bucket, final String key,
+ final ProgressMonitor monitor, final String monitorTask)
throws IOException {
final MessageDigest md5 = newMD5();
final TemporaryBuffer buffer = new TemporaryBuffer() {
@@ -442,7 +450,8 @@ public class AmazonS3 {
public void close() throws IOException {
super.close();
try {
- putImpl(bucket, key, md5.digest(), this);
+ putImpl(bucket, key, md5.digest(), this, monitor,
+ monitorTask);
} finally {
destroy();
}
@@ -452,7 +461,13 @@ public class AmazonS3 {
}
private void putImpl(final String bucket, final String key,
- final byte[] csum, final TemporaryBuffer buf) throws IOException {
+ final byte[] csum, final TemporaryBuffer buf,
+ ProgressMonitor monitor, String monitorTask) throws IOException {
+ if (monitor == null)
+ monitor = NullProgressMonitor.INSTANCE;
+ if (monitorTask == null)
+ monitorTask = "Uploading " + key;
+
final String md5str = Base64.encodeBytes(csum);
final long len = buf.length();
final String lenstr = String.valueOf(len);
@@ -465,10 +480,12 @@ public class AmazonS3 {
authorize(c);
c.setDoOutput(true);
c.setFixedLengthStreamingMode((int) len);
+ monitor.beginTask(monitorTask, (int) (len / 1024));
final OutputStream os = c.getOutputStream();
try {
- buf.writeTo(os, null);
+ buf.writeTo(os, monitor);
} finally {
+ monitor.endTask();
os.close();
}
@@ -641,7 +658,7 @@ public class AmazonS3 {
} else if ("rm".equals(op) || "delete".equals(op)) {
s3.delete(bucket, key);
} else if ("put".equals(op)) {
- final OutputStream os = s3.beginPut(bucket, key);
+ final OutputStream os = s3.beginPut(bucket, key, null, null);
final byte[] tmp = new byte[2048];
int n;
while ((n = System.in.read(tmp)) > 0)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
index d2f4c83..cd62c5b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
@@ -54,6 +54,7 @@ import java.util.TreeMap;
import org.spearce.jgit.errors.NotSupportedException;
import org.spearce.jgit.errors.TransportException;
import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ProgressMonitor;
import org.spearce.jgit.lib.Ref;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.lib.Ref.Storage;
@@ -236,8 +237,10 @@ class TransportAmazonS3 extends WalkTransport {
}
@Override
- OutputStream writeFile(final String path) throws IOException {
- return s3.beginPut(bucket, resolveKey(path));
+ OutputStream writeFile(final String path,
+ final ProgressMonitor monitor, final String monitorTask)
+ throws IOException {
+ return s3.beginPut(bucket, resolveKey(path), monitor, monitorTask);
}
@Override
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
index a33406b..c2cbe6a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportSftp.java
@@ -54,6 +54,7 @@ import java.util.TreeMap;
import org.spearce.jgit.errors.TransportException;
import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ProgressMonitor;
import org.spearce.jgit.lib.Ref;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.lib.Ref.Storage;
@@ -294,7 +295,9 @@ class TransportSftp extends WalkTransport {
}
@Override
- OutputStream writeFile(final String path) throws IOException {
+ OutputStream writeFile(final String path,
+ final ProgressMonitor monitor, final String monitorTask)
+ throws IOException {
try {
return ftp.put(path);
} catch (SftpException je) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
index bb5a653..904a699 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
@@ -230,14 +230,15 @@ class WalkPushConnection extends BaseConnection implements PushConnection {
// Write the pack file, then the index, as readers look the
// other direction (index, then pack file).
//
- OutputStream os = dest.writeFile(pathPack);
+ final String wt = "Put " + base.substring(0, 12);
+ OutputStream os = dest.writeFile(pathPack, monitor, wt + "..pack");
try {
pw.writePack(os);
} finally {
os.close();
}
- os = dest.writeFile(pathIdx);
+ os = dest.writeFile(pathIdx, monitor, wt + "..idx");
try {
pw.writeIndex(os);
} finally {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
index 915faac..c5a5199 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
@@ -52,6 +52,7 @@ import java.util.Map;
import org.spearce.jgit.errors.TransportException;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.ObjectId;
+import org.spearce.jgit.lib.ProgressMonitor;
import org.spearce.jgit.lib.Ref;
import org.spearce.jgit.util.NB;
@@ -213,11 +214,17 @@ abstract class WalkRemoteObjectDatabase {
* complete the write request. The stream is not buffered and each
* write may cause a network request/response so callers should
* buffer to smooth out small writes.
+ * @param monitor
+ * (optional) progress monitor to post write completion to during
+ * the stream's close method.
+ * @param monitorTask
+ * (optional) task name to display during the close method.
* @throws IOException
* writing is not supported, or attempting to write the file
* failed, possibly due to permissions or remote disk full, etc.
*/
- OutputStream writeFile(final String path) throws IOException {
+ OutputStream writeFile(final String path, final ProgressMonitor monitor,
+ final String monitorTask) throws IOException {
throw new IOException("Writing of '" + path + "' not supported.");
}
@@ -247,7 +254,7 @@ abstract class WalkRemoteObjectDatabase {
* failed, possibly due to permissions or remote disk full, etc.
*/
void writeFile(final String path, final byte[] data) throws IOException {
- final OutputStream os = writeFile(path);
+ final OutputStream os = writeFile(path, null, null);
try {
os.write(data);
} finally {
--
1.5.6.2.393.g45096
^ permalink raw reply related
* [JGIT PATCH 0/5] Yet another round of transport fixes
From: Shawn O. Pearce @ 2008-07-10 6:13 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
Apparently tweaking minor details is never done. This short
series improves minor issues I have with the transport code
in relation to error and progress reporting. We also get a
small performance boost for tag fetching over SSH.
Shawn O. Pearce (5):
Include a progress meter for large uploads to Amazon S3
Don't display passwords on the console in fetch/push output
Reuse the same SSH connection when automatically fetching tags
Report remote SSH execution errors during push via TransportException
Explicitly capture the stderr from a failed SSH fetch or push
.../org/spearce/egit/core/op/CloneOperation.java | 10 ++-
.../spearce/egit/ui/EclipseSshSessionFactory.java | 5 +-
.../egit/ui/internal/clone/SourceBranchPage.java | 1 +
.../spearce/jgit/transport/PushProcessTest.java | 5 +
.../org/spearce/jgit/transport/TransportTest.java | 10 ++
.../jgit/errors/NoRemoteRepositoryException.java | 59 +++++++++++
.../src/org/spearce/jgit/pgm/Fetch.java | 19 +++--
.../src/org/spearce/jgit/pgm/LsRemote.java | 1 +
.../src/org/spearce/jgit/pgm/Push.java | 2 +-
.../src/org/spearce/jgit/transport/AmazonS3.java | 29 +++++-
.../spearce/jgit/transport/BasePackConnection.java | 3 +-
.../jgit/transport/DefaultSshSessionFactory.java | 30 ++++++-
.../src/org/spearce/jgit/transport/Transport.java | 10 ++
.../spearce/jgit/transport/TransportAmazonS3.java | 12 ++-
.../spearce/jgit/transport/TransportBundle.java | 5 +
.../spearce/jgit/transport/TransportGitAnon.java | 5 +
.../spearce/jgit/transport/TransportGitSsh.java | 103 +++++++++++++-------
.../org/spearce/jgit/transport/TransportHttp.java | 5 +
.../org/spearce/jgit/transport/TransportLocal.java | 5 +
.../org/spearce/jgit/transport/TransportSftp.java | 57 ++++++-----
.../spearce/jgit/transport/WalkPushConnection.java | 5 +-
.../jgit/transport/WalkRemoteObjectDatabase.java | 11 ++-
22 files changed, 303 insertions(+), 89 deletions(-)
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/errors/NoRemoteRepositoryException.java
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox