* Re: [PATCH] send-email: Fix %config_path_settings handling
From: Junio C Hamano @ 2011-10-14 19:26 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Michael J Gruber, Git Mailing List, Cord Seele, Cord Seele
In-Reply-To: <201110142049.32734.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> From: Cord Seele <cowose@gmail.com>
> value... which admittedly is a bit cryptic. More readable if more
> verbose option would be to use hash reference, e.g.:
>
> my %config_bool_settings = (
> "thread" => { variable => \$thread, default => 1},
> [...]
>
> Or something like that.
Do you really want to leave this "Or something like that" here?
> 3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
> didn't add test for alias expansion to t9001-send-email.sh
I was hoping that an updated patch to have a new test or two here...
> Signed-off-by: Cord Seele <cowose@gmail.com>
> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Is this the version tested by Michael?
> + my $target = $config_path_settings{$setting};
> + if (ref($target) eq "ARRAY" && !@$target) {
> + # multi-valued and not set
> + my @values = Git::config_path(@repo, "$prefix.$setting");
> + @$target = @values if (@values && defined $values[0]);
> + } elsif (!defined $$target) {
> + # multi-valued and not set
> + $$target = Git::config_path(@repo, "$prefix.$setting");
> + }
If the target is an array ref and for whatever reason the array is already
populated, wouldn't you check "if (!defined $$target)" with this change?
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jeff King @ 2011-10-14 19:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyen Thai Ngoc Duy, git, Ilari Liusvaara, Johannes Sixt,
Jonathan Nieder
In-Reply-To: <20111014192326.GA7713@sigill.intra.peff.net>
On Fri, Oct 14, 2011 at 03:23:26PM -0400, Jeff King wrote:
> Subject: [PATCH] daemon: give friendlier error messages to clients
>
> When the git-daemon is asked about an inaccessible
> repository, it simply hangs up the connection without saying
> anything further. This makes it hard to distinguish between
> a repository we cannot access (e.g., due to typo), and a
> service or network outage.
>
> Instead, let's print an "ERR" line, which git clients
> understand since v1.6.1 (2008-12-24).
>
> Because there is a risk of leaking information about
> non-exported repositories, by default all errors simply say
> "access denied". Open sites can pass a flag to turn on more
> specific messages.
I'm tempted to suggest this on top:
-- >8 --
Subject: [PATCH] daemon: turn on informative errors by default
These are only a problem if you have a bunch of inaccessible
repositories served from the same root as your regular
exported repositories, and you are sensitive about people
learning about the existence of those repositories.
Git is foremost an open system, and our defaults should
reflect that.
Signed-off-by: Jeff King <peff@peff.net>
---
But since it is a potential security issue, it does seem kind of mean to
closed sites to just flip the switch on them.
Documentation/git-daemon.txt | 6 +++---
daemon.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index ac57c6d..2b17175 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -161,12 +161,12 @@ the facility of inet daemon to achieve the same before spawning
repository configuration. By default, all the services
are overridable.
---informative-errors::
- Return more verbose errors to the client, differentiating
+--no-informative-errors::
+ By default, we return verbose errors to the client, differentiating
conditions like "no such repository" from "repository not
exported". This is more convenient for clients, but may leak
information about the existence of unexported repositories.
- Without this option, all errors report "access denied" to the
+ With this option, all errors report "access denied" to the
client.
<directory>::
diff --git a/daemon.c b/daemon.c
index e5869ec..ba41a40 100644
--- a/daemon.c
+++ b/daemon.c
@@ -20,7 +20,7 @@
static int log_syslog;
static int verbose;
static int reuseaddr;
-static int informative_errors;
+static int informative_errors = 1;
static const char daemon_usage[] =
"git daemon [--verbose] [--syslog] [--export-all]\n"
--
1.7.6.4.37.g43b58b
^ permalink raw reply related
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Junio C Hamano @ 2011-10-14 20:24 UTC (permalink / raw)
To: Jeff King
Cc: Nguyen Thai Ngoc Duy, git, Ilari Liusvaara, Johannes Sixt,
Jonathan Nieder
In-Reply-To: <20111014192741.GA13029@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Subject: [PATCH] daemon: turn on informative errors by default
>
> These are only a problem if you have a bunch of inaccessible
> repositories served from the same root as your regular
> exported repositories, and you are sensitive about people
> learning about the existence of those repositories.
>
> Git is foremost an open system, and our defaults should
> reflect that.
>
> Signed-off-by: Jeff King <peff@peff.net>
I think the logic in the last paragraph is flawed.
There is a difference between Git being an open system, and installations
and users of Git being primarily people who work on open projects.
Even though personally I wish there weren't.
> But since it is a potential security issue, it does seem kind of mean to
> closed sites to just flip the switch on them.
It would have been a better split to have the 1/2 patch to support both
informative and uninformative errors, with the default to say "access
denied", and 2/2 to flip the default to be more open.
Will queue as-is, though.
> Documentation/git-daemon.txt | 6 +++---
> daemon.c | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> index ac57c6d..2b17175 100644
> --- a/Documentation/git-daemon.txt
> +++ b/Documentation/git-daemon.txt
> @@ -161,12 +161,12 @@ the facility of inet daemon to achieve the same before spawning
> repository configuration. By default, all the services
> are overridable.
>
> ---informative-errors::
> - Return more verbose errors to the client, differentiating
> +--no-informative-errors::
> + By default, we return verbose errors to the client, differentiating
> conditions like "no such repository" from "repository not
> exported". This is more convenient for clients, but may leak
> information about the existence of unexported repositories.
> - Without this option, all errors report "access denied" to the
> + With this option, all errors report "access denied" to the
> client.
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jeff King @ 2011-10-14 20:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyen Thai Ngoc Duy, git, Ilari Liusvaara, Johannes Sixt,
Jonathan Nieder
In-Reply-To: <7v7h47z5i0.fsf@alter.siamese.dyndns.org>
On Fri, Oct 14, 2011 at 01:24:07PM -0700, Junio C Hamano wrote:
> > Git is foremost an open system, and our defaults should
> > reflect that.
> [...]
>
> I think the logic in the last paragraph is flawed.
>
> There is a difference between Git being an open system, and installations
> and users of Git being primarily people who work on open projects.
>
> Even though personally I wish there weren't.
I think it is not the logic that is flawed, but the communication. What
I meant was that git was originally designed to support open projects
(like the kernel), and they are our primary target.
Ingo said something similar here:
http://article.gmane.org/gmane.linux.kernel/1202320
Still, primary target and primary user are not necessarily the same
thing. And a minor convenience for one audience that introduces a
security problem for another audience may not be a good tradeoff, no
matter who the audiences are.
I didn't really expect you to take my second patch. We tend to be a bit
more conservative than that around here.
> > But since it is a potential security issue, it does seem kind of mean to
> > closed sites to just flip the switch on them.
>
> It would have been a better split to have the 1/2 patch to support both
> informative and uninformative errors, with the default to say "access
> denied", and 2/2 to flip the default to be more open.
Isn't that what I did? It was what I meant to do, anyway...
Or did you mean the options would have been better worded as:
--errors={terse,informative}
or something similar?
-Peff
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Junio C Hamano @ 2011-10-14 20:48 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git, Ilari Liusvaara,
Johannes Sixt, Jonathan Nieder
In-Reply-To: <20111014203438.GA15643@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> It would have been a better split to have the 1/2 patch to support both
>> informative and uninformative errors, with the default to say "access
>> denied", and 2/2 to flip the default to be more open.
>
> Isn't that what I did? It was what I meant to do, anyway...
>
> Or did you mean the options would have been better worded as:
>
> --errors={terse,informative}
>
> or something similar?
Nothing that elaborate.
Supporting --no-* variant even when the default is already no will allow
people to prepare their daemon invocation command line beforehand to ensure
that they won't be affected to a more lenient default that may or may not
come in the future. That's all.
^ permalink raw reply
* Re: [PATCH] send-email: Fix %config_path_settings handling
From: Jakub Narebski @ 2011-10-14 20:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Git Mailing List, Cord Seele, Cord Seele
In-Reply-To: <7vbotjz85o.fsf@alter.siamese.dyndns.org>
From: Cord Seele <cowose@gmail.com>
cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30) broke
the expansion of aliases.
This was caused by treating %config_path_settings, newly introduced in
said patch, like %config_bool_settings instead of like %config_settings.
Copy from %config_settings, making it more readable.
While at it add basic test for expansion of aliases, and for path
expansion, which would catch this error.
Nb. there were a few issues that were responsible for this error:
1. %config_bool_settings and %config_settings despite similar name have
different semantic.
%config_bool_settings values are arrays where the first element is
(reference to) the variable to set, and second element is default
value... which admittedly is a bit cryptic. More readable if more
verbose option would be to use hash reference, e.g.:
my %config_bool_settings = (
"thread" => { variable => \$thread, default => 1},
[...]
%config_settings values are either either reference to scalar variable
or reference to array. In second case it means that option (or config
option) is multi-valued. BTW. this is similar to what Getopt::Long does.
2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
the setting "aliasesfile" was moved from %config_settings to newly
introduced %config_path_settings. But the loop that parses settings
from %config_path_settings was copy'n'pasted *wrongly* from
%config_bool_settings instead of from %config_settings.
It looks like cec5dae author cargo-culted this change...
3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
didn't add test for alias expansion to t9001-send-email.sh
Signed-off-by: Cord Seele <cowose@gmail.com>
Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > From: Cord Seele <cowose@gmail.com>
> > value... which admittedly is a bit cryptic. More readable if more
> > verbose option would be to use hash reference, e.g.:
> >
> > my %config_bool_settings = (
> > "thread" => { variable => \$thread, default => 1},
> > [...]
> >
> > Or something like that.
>
> Do you really want to leave this "Or something like that" here?
Removed.
"e.g." should be enough.
> > 3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
> > didn't add test for alias expansion to t9001-send-email.sh
>
> I was hoping that an updated patch to have a new test or two here...
Done. I thought to add it in separate patch...
> > Signed-off-by: Cord Seele <cowose@gmail.com>
> > Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
> > Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>
> Is this the version tested by Michael?
This one is.
[cut previous version]
git-send-email.perl | 12 ++++++++++--
t/t9001-send-email.sh | 28 ++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 91607c5..6885dfa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -337,8 +337,16 @@ sub read_config {
}
foreach my $setting (keys %config_path_settings) {
- my $target = $config_path_settings{$setting}->[0];
- $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+ my $target = $config_path_settings{$setting};
+ if (ref($target) eq "ARRAY") {
+ unless (@$target) {
+ my @values = Git::config_path(@repo, "$prefix.$setting");
+ @$target = @values if (@values && defined $values[0]);
+ }
+ }
+ else {
+ $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+ }
}
foreach my $setting (keys %config_settings) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 579ddb7..87b4acc 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1168,4 +1168,32 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
test -n "$(ls msgtxt*)"
'
+test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
+ clean_fake_sendmail &&
+ echo "alias sbd somebody@example.org" >.mailrc &&
+ git config --replace-all sendemail.aliasesfile "$(pwd)/.mailrc" &&
+ git config sendemail.aliasfiletype mailrc &&
+ git send-email \
+ --from="Example <nobody@example.com>" \
+ --to=sbd \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ outdir/0001-*.patch \
+ 2>errors >out &&
+ grep "^!somebody@example\.org!$" commandline1
+'
+
+test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
+ clean_fake_sendmail &&
+ echo "alias sbd someone@example.org" >~/.mailrc &&
+ git config --replace-all sendemail.aliasesfile "~/.mailrc" &&
+ git config sendemail.aliasfiletype mailrc &&
+ git send-email \
+ --from="Example <nobody@example.com>" \
+ --to=sbd \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ outdir/0001-*.patch \
+ 2>errors >out &&
+ grep "^!someone@example\.org!$" commandline1
+'
+
test_done
--
1.7.6
^ permalink raw reply related
* Re: [PATCH] t1304: fall back to $USER if $LOGNAME is not defined
From: René Scharfe @ 2011-10-14 20:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Brandon Casey, Matthieu Moy
In-Reply-To: <7vobxjza9t.fsf@alter.siamese.dyndns.org>
Am 14.10.2011 20:41, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> For some reason $LOGNAME is not set anymore for me after an upgrade from
>> Ubuntu 11.04 to 11.10. Use $USER in such a case.
>>
>> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
>> ---
>> The only other use of $LOGNAME is in git-cvsimport, which does the same.
>
> While the change to assign $USER to $LOGNAME when the latter is not set is
> not wrong per-se, I have to wonder (1) why this test should use LOGNAME
> not USER in the first place, and (2) why you lost LOGNAME.
>
> LOGNAME
> The system shall initialize this variable at the time of login to be the
> user's login name. See <pwd.h> . For a value of LOGNAME to be portable
> across implementations of POSIX.1-2008, the value should be composed of
> characters from the portable filename character set.
>
> Will apply anyway. Thanks.
Common practise (on Linux at least) seems to be to set both $LOGNAME
(from Sys-V) and $USER (from BSD) to the same value, so either could be
used. The portable way is to check both.
Both were set before the upgrade on my system, and $LOGNAME is _still_
set if I log onto the console or use sudo or su, but it's _not_ set in a
plain Gnome Terminal with bash. I also wonder why that may be.
René
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jonathan Nieder @ 2011-10-14 21:02 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git, Ilari Liusvaara,
Johannes Sixt
In-Reply-To: <20111014192326.GA7713@sigill.intra.peff.net>
Jeff King wrote:
> When the git-daemon is asked about an inaccessible
> repository, it simply hangs up the connection without saying
> anything further. This makes it hard to distinguish between
> a repository we cannot access (e.g., due to typo), and a
> service or network outage.
*nod*
> Instead, let's print an "ERR" line, which git clients
> understand since v1.6.1 (2008-12-24).
Just to be clear, "git archive --remote" does not understand ERR lines
in the 'master' branch (though 908aaceb makes it understand them in
'next'). But I consider even distinguishing
a. fatal: git archive: protocol error
b. fatal: git archive: expected ACK/NAK, got EOF
[(a) is how an ERR response is reported, and (b) a remote hangup] to
be progress, so it's not so important. :)
> Because there is a risk of leaking information about
> non-exported repositories, by default all errors simply say
> "access denied". Open sites can pass a flag to turn on more
> specific messages.
I'm not sure what an "open site" is. :) But having this flag for
sites to declare whether they consider whether a repository exists to
be privileged information seems reasonable to me.
Note that this really would be privileged information in some
not-too-weird cases. For example, if many users have a repository at
~/.git, ~/.config/.git, or ~/src/linux/.git, then someone might try to
access
/home/alice/.git
/home/alice/.config/.git
/home/alice/src/linux/.git
/home/bob/.git
...
in turn to find a valid username, as reconnaisance for a later
attack not involving git.
Luckily, this can be avoided with
git daemon --user-path=public_git --base-path=/pub/git
which only allows access to subdirectories of /pub/git and
public_git in home directories. With
git daemon --base-path=/pub/git
there is still no problem, since access to home directories is not
allowed at all in that case. I suppose the documentation should
mention that --informative-errors is best not used unless --base-path
is also in use. (Almost everyone is already using --base-path, so
this isn't a very serious problem.)
> --- a/daemon.c
> +++ b/daemon.c
> @@ -20,6 +20,7 @@
[...]
> @@ -1167,6 +1176,14 @@ int main(int argc, char **argv)
> make_service_overridable(arg + 18, 0);
> continue;
> }
> + if (!prefixcmp(arg, "--informative-errors")) {
> + informative_errors = 1;
> + continue;
> + }
> + else if (!prefixcmp(arg, "--no-informative-errors")) {
> + informative_errors = 0;
> + continue;
> + }
> if (!strcmp(arg, "--")) {
Micronit: uncuddled "else". The style of the surrounding code is to
just not include the "else" at all and rely on "continue" to
short-circuit things.
Anyway, except for the documentation nits mentioned above (and Junio's
nit, too),
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks a lot for this.
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jeff King @ 2011-10-14 21:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyen Thai Ngoc Duy, git, Ilari Liusvaara, Johannes Sixt,
Jonathan Nieder
In-Reply-To: <7vpqhzxpsc.fsf@alter.siamese.dyndns.org>
On Fri, Oct 14, 2011 at 01:48:51PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> >> It would have been a better split to have the 1/2 patch to support both
> >> informative and uninformative errors, with the default to say "access
> >> denied", and 2/2 to flip the default to be more open.
> >
> > Isn't that what I did? It was what I meant to do, anyway...
> >
> > Or did you mean the options would have been better worded as:
> >
> > --errors={terse,informative}
> >
> > or something similar?
>
> Nothing that elaborate.
>
> Supporting --no-* variant even when the default is already no will allow
> people to prepare their daemon invocation command line beforehand to ensure
> that they won't be affected to a more lenient default that may or may not
> come in the future. That's all.
Oh. Then look again at 1/2. It supports both forms; I just didn't bother
advertising the --no form in the manpage, since it was the default.
-Peff
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jonathan Nieder @ 2011-10-14 21:06 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git, Ilari Liusvaara,
Johannes Sixt
In-Reply-To: <20111014210506.GA16226@sigill.intra.peff.net>
Jeff King wrote:
> Oh. Then look again at 1/2. It supports both forms; I just didn't bother
> advertising the --no form in the manpage, since it was the default.
Yes, and that was the bug Junio mentioned. If we are considering 2/2
then admins will need to know about the --no form before we roll out
the change in default. :)
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jeff King @ 2011-10-14 21:12 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git, Ilari Liusvaara,
Johannes Sixt
In-Reply-To: <20111014210251.GD16371@elie.hsd1.il.comcast.net>
On Fri, Oct 14, 2011 at 04:02:51PM -0500, Jonathan Nieder wrote:
> > Instead, let's print an "ERR" line, which git clients
> > understand since v1.6.1 (2008-12-24).
>
> Just to be clear, "git archive --remote" does not understand ERR lines
> in the 'master' branch (though 908aaceb makes it understand them in
> 'next'). But I consider even distinguishing
>
> a. fatal: git archive: protocol error
> b. fatal: git archive: expected ACK/NAK, got EOF
>
> [(a) is how an ERR response is reported, and (b) a remote hangup] to
> be progress, so it's not so important. :)
Thanks, I forgot to mention that. It's not as nice as the push/fetch
case, but I agree it's a step forward.
> > Because there is a risk of leaking information about
> > non-exported repositories, by default all errors simply say
> > "access denied". Open sites can pass a flag to turn on more
> > specific messages.
>
> I'm not sure what an "open site" is. :) But having this flag for
> sites to declare whether they consider whether a repository exists to
> be privileged information seems reasonable to me.
I meant sites which are just serving a bunch of public repos, like
kernel.org.
> Note that this really would be privileged information in some
> not-too-weird cases. For example, if many users have a repository at
> ~/.git, ~/.config/.git, or ~/src/linux/.git, then someone might try to
> access
>
> /home/alice/.git
> /home/alice/.config/.git
> /home/alice/src/linux/.git
> /home/bob/.git
> ...
>
> in turn to find a valid username, as reconnaisance for a later
> attack not involving git.
I sort of assume everybody serves a specific directory hierarchy, but
maybe that is not the case. I don't run git-daemon myself, so I am
probably guilty of generalizing how other people use it.
Anyway, I think the issue is sufficiently nuanced that we should keep
the default to the conservative "access denied" (i.e., throw away my
second patch for now).
> > --- a/daemon.c
> > +++ b/daemon.c
> > @@ -20,6 +20,7 @@
> [...]
> > @@ -1167,6 +1176,14 @@ int main(int argc, char **argv)
> > make_service_overridable(arg + 18, 0);
> > continue;
> > }
> > + if (!prefixcmp(arg, "--informative-errors")) {
> > + informative_errors = 1;
> > + continue;
> > + }
> > + else if (!prefixcmp(arg, "--no-informative-errors")) {
> > + informative_errors = 0;
> > + continue;
> > + }
> > if (!strcmp(arg, "--")) {
>
> Micronit: uncuddled "else". The style of the surrounding code is to
> just not include the "else" at all and rely on "continue" to
> short-circuit things.
Oops. Yes, it should just drop the else to match the surrounding code.
-Peff
^ permalink raw reply
* [PATCHv3] daemon: give friendlier error messages to clients
From: Jeff King @ 2011-10-14 21:19 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git, Ilari Liusvaara,
Johannes Sixt
In-Reply-To: <20111014211244.GA16429@sigill.intra.peff.net>
When the git-daemon is asked about an inaccessible
repository, it simply hangs up the connection without saying
anything further. This makes it hard to distinguish between
a repository we cannot access (e.g., due to typo), and a
service or network outage.
Instead, let's print an "ERR" line, which git clients
understand since v1.6.1 (2008-12-24).
Because there is a risk of leaking information about
non-exported repositories, by default all errors simply say
"access denied". Sites which don't have hidden repositories,
or don't care, can pass a flag to turn on more specific
messages.
Signed-off-by: Jeff King <peff@peff.net>
---
Minor tweaks to the documentation and code style to make Jonathan happy.
:)
Note: I labeled this "v3", as it is the third one posted, but the prior
ones were not labeled with versions at all.
Documentation/git-daemon.txt | 10 ++++++++++
daemon.c | 25 +++++++++++++++++++++----
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 69a1e4a..31b28fc 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -161,6 +161,16 @@ the facility of inet daemon to achieve the same before spawning
repository configuration. By default, all the services
are overridable.
+--informative-errors::
+--no-informative-errors::
+ When informative errors are turned on, git-daemon will report
+ more verbose errors to the client, differentiating conditions
+ like "no such repository" from "repository not exported". This
+ is more convenient for clients, but may leak information about
+ the existence of unexported repositories. When informative
+ errors are not enabled, all errors report "access denied" to the
+ client. The default is --no-informative-errors.
+
<directory>::
A directory to add to the whitelist of allowed directories. Unless
--strict-paths is specified this will also include subdirectories
diff --git a/daemon.c b/daemon.c
index 4c8346d..6f111af 100644
--- a/daemon.c
+++ b/daemon.c
@@ -20,6 +20,7 @@
static int log_syslog;
static int verbose;
static int reuseaddr;
+static int informative_errors;
static const char daemon_usage[] =
"git daemon [--verbose] [--syslog] [--export-all]\n"
@@ -247,6 +248,14 @@ static int git_daemon_config(const char *var, const char *value, void *cb)
return 0;
}
+static int daemon_error(const char *dir, const char *msg)
+{
+ if (!informative_errors)
+ msg = "access denied";
+ packet_write(1, "ERR %s: %s", msg, dir);
+ return -1;
+}
+
static int run_service(char *dir, struct daemon_service *service)
{
const char *path;
@@ -257,11 +266,11 @@ static int run_service(char *dir, struct daemon_service *service)
if (!enabled && !service->overridable) {
logerror("'%s': service not enabled.", service->name);
errno = EACCES;
- return -1;
+ return daemon_error(dir, "service not enabled");
}
if (!(path = path_ok(dir)))
- return -1;
+ return daemon_error(dir, "no such repository");
/*
* Security on the cheap.
@@ -277,7 +286,7 @@ static int run_service(char *dir, struct daemon_service *service)
if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
logerror("'%s': repository not exported.", path);
errno = EACCES;
- return -1;
+ return daemon_error(dir, "repository not exported");
}
if (service->overridable) {
@@ -291,7 +300,7 @@ static int run_service(char *dir, struct daemon_service *service)
logerror("'%s': service not enabled for '%s'",
service->name, path);
errno = EACCES;
- return -1;
+ return daemon_error(dir, "service not enabled");
}
/*
@@ -1167,6 +1176,14 @@ int main(int argc, char **argv)
make_service_overridable(arg + 18, 0);
continue;
}
+ if (!prefixcmp(arg, "--informative-errors")) {
+ informative_errors = 1;
+ continue;
+ }
+ if (!prefixcmp(arg, "--no-informative-errors")) {
+ informative_errors = 0;
+ continue;
+ }
if (!strcmp(arg, "--")) {
ok_paths = &argv[i+1];
break;
--
1.7.6.4.37.g43b58b
^ permalink raw reply related
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jonathan Nieder @ 2011-10-14 21:20 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git, Ilari Liusvaara,
Johannes Sixt
In-Reply-To: <20111014192741.GA13029@sigill.intra.peff.net>
Jeff King wrote:
> I'm tempted to suggest this on top:
>
> -- >8 --
> Subject: [PATCH] daemon: turn on informative errors by default
Very good idea, as long as we are cautious about making sure admins
know about the change before it comes (for example using release
notes).
[...]
> Git is foremost an open system, and our defaults should
> reflect that.
I think this is a lousy justification. :)
Sure, certain prominent users of git (like kernel.org) would probably
not want to set --no-informative-errors. And you and I might prefer
that _nobody_ set --no-informative-errors. But this does not mean the
design of Git has anything to do with that, and I am afraid of the
direction it would take us in if we start pretending it does.
The git daemon is primarily a functional, secure, admin-friendly and
client-friendly program whose defaults should make admins happy where
possible. Luckily all that is consistent with --informative-errors.
In most use cases (i.e., not weird military security kinds of things),
although --informative-errors without --base-path could have negative
security impact as Andreas has explained before, --informative-errors
with --base-path is harmless as far as I can tell.
I am tempted to propose making --base-path mandatory when there is not
at least one path_ok argument, so in the unusual case, people would
have to explicitly say they want to serve repositories rooted at /.
Just my two cents,
Jonathan
^ permalink raw reply
* [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.
From: Andrei Warkentin @ 2011-10-14 21:51 UTC (permalink / raw)
To: git, gitster; +Cc: Andrei Warkentin
Many users of p4/sd use changelists for review, regression
tests and batch builds, thus changes are almost never directly
submitted.
This new config option lets a 'p4 change -i' run instead of
the 'p4 submit -i'.
Signed-off-by: Andrei Warkentin <andreiw@vmware.com>
---
contrib/fast-import/git-p4 | 16 ++++++++++++----
contrib/fast-import/git-p4.txt | 10 ++++++++++
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2f7b270..19c295b 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -959,7 +959,10 @@ class P4Submit(Command, P4UserMap):
submitTemplate = message[:message.index(separatorLine)]
if self.isWindows:
submitTemplate = submitTemplate.replace("\r\n", "\n")
- p4_write_pipe("submit -i", submitTemplate)
+ if gitConfig("git-p4.changeOnSubmit"):
+ p4_write_pipe("change -i", submitTemplate)
+ else:
+ p4_write_pipe("subadasdmit -i", submitTemplate)
if self.preserveUser:
if p4User:
@@ -981,9 +984,14 @@ class P4Submit(Command, P4UserMap):
file = open(fileName, "w+")
file.write(self.prepareLogMessage(template, logMessage))
file.close()
- print ("Perforce submit template written as %s. "
- + "Please review/edit and then use p4 submit -i < %s to submit directly!"
- % (fileName, fileName))
+ if gitConfig("git-p4.changeOnSubmit"):
+ print ("Perforce submit template written as %s. "
+ + "Please review/edit and then use p4 change -i < %s to create changelist!"
+ % (fileName, fileName))
+ else:
+ print ("Perforce submit template written as %s. "
+ + "Please review/edit and then use p4 submit -i < %s to submit directly!"
+ % (fileName, fileName))
def run(self, args):
if len(args) == 0:
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index 52003ae..3a3a815 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -180,6 +180,16 @@ git-p4.allowSubmit
git config [--global] git-p4.allowSubmit false
+git-p4.changeOnSubmit
+
+ git config [--global] git-p4.changeOnSubmit false
+
+Most places using p4/sourcedepot don't actually want you submit
+changes directly, and changelists are used to do regression testing,
+batch builds and review, hence, by setting this parameter to
+true you acknowledge you end up creating a changelist which you
+must then manually commit.
+
git-p4.syncFromOrigin
A useful setup may be that you have a periodically updated git repository
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCHv3] daemon: give friendlier error messages to clients
From: Junio C Hamano @ 2011-10-14 21:52 UTC (permalink / raw)
To: Jeff King
Cc: Jonathan Nieder, Junio C Hamano, Nguyen Thai Ngoc Duy, git,
Ilari Liusvaara, Johannes Sixt
In-Reply-To: <20111014211921.GB16429@sigill.intra.peff.net>
Thanks, both. I'll drop the previous one, and rebuild the integration
branches by queuign this version instead.
^ permalink raw reply
* Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.
From: Tor Arvid Lund @ 2011-10-14 22:31 UTC (permalink / raw)
To: Andrei Warkentin; +Cc: git, gitster
In-Reply-To: <1318629110-15232-1-git-send-email-andreiw@vmware.com>
Hi Andrei, and thanks for trying to help improve git-p4! :-)
2011/10/14 Andrei Warkentin <andreiw@vmware.com>:
> Many users of p4/sd use changelists for review, regression
> tests and batch builds, thus changes are almost never directly
> submitted.
Just out of curiosity... what is 'sd'?
> This new config option lets a 'p4 change -i' run instead of
> the 'p4 submit -i'.
Well... I have to say that I'm not crazy about this patch... I don't
think it is very elegant to have a config flag that says that "when
the user says 'git p4 submit', then don't submit, but do something
else instead".
I would much rather have made a patch to introduce some new command
like 'git p4 change'.
> Signed-off-by: Andrei Warkentin <andreiw@vmware.com>
> ---
> contrib/fast-import/git-p4 | 16 ++++++++++++----
> contrib/fast-import/git-p4.txt | 10 ++++++++++
> 2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 2f7b270..19c295b 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -959,7 +959,10 @@ class P4Submit(Command, P4UserMap):
> submitTemplate = message[:message.index(separatorLine)]
> if self.isWindows:
> submitTemplate = submitTemplate.replace("\r\n", "\n")
> - p4_write_pipe("submit -i", submitTemplate)
> + if gitConfig("git-p4.changeOnSubmit"):
> + p4_write_pipe("change -i", submitTemplate)
> + else:
> + p4_write_pipe("subadasdmit -i", submitTemplate)
... 'subadasdmit'? Did some debug/test code sneak in to your patch?
>
> if self.preserveUser:
> if p4User:
> @@ -981,9 +984,14 @@ class P4Submit(Command, P4UserMap):
> file = open(fileName, "w+")
> file.write(self.prepareLogMessage(template, logMessage))
> file.close()
> - print ("Perforce submit template written as %s. "
> - + "Please review/edit and then use p4 submit -i < %s to submit directly!"
> - % (fileName, fileName))
> + if gitConfig("git-p4.changeOnSubmit"):
> + print ("Perforce submit template written as %s. "
> + + "Please review/edit and then use p4 change -i < %s to create changelist!"
> + % (fileName, fileName))
> + else:
> + print ("Perforce submit template written as %s. "
> + + "Please review/edit and then use p4 submit -i < %s to submit directly!"
> + % (fileName, fileName))
>
> def run(self, args):
> if len(args) == 0:
> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
> index 52003ae..3a3a815 100644
> --- a/contrib/fast-import/git-p4.txt
> +++ b/contrib/fast-import/git-p4.txt
> @@ -180,6 +180,16 @@ git-p4.allowSubmit
>
> git config [--global] git-p4.allowSubmit false
>
> +git-p4.changeOnSubmit
> +
> + git config [--global] git-p4.changeOnSubmit false
> +
> +Most places using p4/sourcedepot don't actually want you submit
> +changes directly, and changelists are used to do regression testing,
> +batch builds and review, hence, by setting this parameter to
> +true you acknowledge you end up creating a changelist which you
> +must then manually commit.
> +
It might be just me, but I never heard of 'sourcedepot' before this
patch... Google tells me that it might be some old version of p4 that
Microsoft used many many years ago. If that's the case, maybe it
doesn't add much value to talk about it in git-p4 docs.. (??)
And you claim 'most places don't want people to submit directly'. I'm
not sure I agree with that, and anyway it seems like the git-p4 docs
should be phrased more neutral than that.
My advice would be, as I mentioned earlier, to rework this into a
patch introducing a separate command 'git p4 change' instead of this
config flag.
Have a good one!
Tor Arvid
> git-p4.syncFromOrigin
>
> A useful setup may be that you have a periodically updated git repository
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH 1/8] t1020: disable the pwd test on MinGW
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Johannes Schindelin
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
It fails both for line ending and for DOS path reasons.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t1020-subdirectory.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
index 3b1b985..e23ac0e 100755
--- a/t/t1020-subdirectory.sh
+++ b/t/t1020-subdirectory.sh
@@ -118,7 +118,7 @@ test_expect_success 'alias expansion' '
)
'
-test_expect_success '!alias expansion' '
+test_expect_success NOT_MINGW '!alias expansion' '
pwd >expect &&
(
git config alias.test !pwd &&
--
1.7.7.1.gbba15
^ permalink raw reply related
* [PATCH 3/8] t9001: do not fail only due to CR/LF issues
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Johannes Schindelin
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t9001-send-email.sh | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 579ddb7..7d75738 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -23,6 +23,7 @@ test_expect_success $PREREQ \
echo do
echo " echo \"!\$a!\""
echo "done >commandline\$output"
+ test_have_prereq MINGW && echo "dos2unix commandline\$output"
echo "cat > msgtxt\$output"
) >fake.sendmail &&
chmod +x ./fake.sendmail &&
--
1.7.7.1.gbba15
^ permalink raw reply related
* [PATCH 6/8] t9300: do not run --cat-blob-fd related tests on MinGW
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Johannes Schindelin
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
As diagnosed by Johannes Sixt, msys.dll does not hand through file
descriptors > 2 to child processes, so these test cases cannot passes when
run through an MSys bash.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t9300-fast-import.sh | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index bd32b91..438aaf6 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2237,7 +2237,7 @@ test_expect_success 'R: cat-blob-fd must be a nonnegative integer' '
test_must_fail git fast-import --cat-blob-fd=-1 </dev/null
'
-test_expect_success 'R: print old blob' '
+test_expect_success NOT_MINGW 'R: print old blob' '
blob=$(echo "yes it can" | git hash-object -w --stdin) &&
cat >expect <<-EOF &&
${blob} blob 11
@@ -2249,7 +2249,7 @@ test_expect_success 'R: print old blob' '
test_cmp expect actual
'
-test_expect_success 'R: in-stream cat-blob-fd not respected' '
+test_expect_success NOT_MINGW 'R: in-stream cat-blob-fd not respected' '
echo hello >greeting &&
blob=$(git hash-object -w greeting) &&
cat >expect <<-EOF &&
@@ -2270,7 +2270,7 @@ test_expect_success 'R: in-stream cat-blob-fd not respected' '
test_cmp expect actual.1
'
-test_expect_success 'R: print new blob' '
+test_expect_success NOT_MINGW 'R: print new blob' '
blob=$(echo "yep yep yep" | git hash-object --stdin) &&
cat >expect <<-EOF &&
${blob} blob 12
@@ -2288,7 +2288,7 @@ test_expect_success 'R: print new blob' '
test_cmp expect actual
'
-test_expect_success 'R: print new blob by sha1' '
+test_expect_success NOT_MINGW 'R: print new blob by sha1' '
blob=$(echo "a new blob named by sha1" | git hash-object --stdin) &&
cat >expect <<-EOF &&
${blob} blob 25
--
1.7.7.1.gbba15
^ permalink raw reply related
* [PATCH 7/8] mergetools: use the correct tool for Beyond Compare 3 on Windows
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Pat Thoyts
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>
On Windows the bcompare tool launches a graphical program and does
not wait for it to terminate. A separate 'bcomp' tool is provided which
will wait for the view to exit so we use this instead.
Reported-by: Werner BEROUX <werner@beroux.com>
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
mergetools/bc3 | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/mergetools/bc3 b/mergetools/bc3
index 27b3dd4..b642bf2 100644
--- a/mergetools/bc3
+++ b/mergetools/bc3
@@ -16,5 +16,12 @@ merge_cmd () {
}
translate_merge_tool_path() {
- echo bcompare
+ case $(uname -s) in
+ *MINGW*)
+ echo bcomp
+ ;;
+ *)
+ echo bcompare
+ ;;
+ esac
}
--
1.7.7.1.gbba15
^ permalink raw reply related
* [PATCH 2/8] t1402: Ignore a few cases that must fail due to DOS path expansion
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Johannes Schindelin
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t1402-check-ref-format.sh | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 710fcca..1a5e343 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -36,7 +36,7 @@ invalid_ref 'refs///heads/foo'
valid_ref 'refs///heads/foo' --normalize
invalid_ref 'heads/foo/'
invalid_ref '/heads/foo'
-valid_ref '/heads/foo' --normalize
+test_have_prereq MINGW || valid_ref '/heads/foo' --normalize
invalid_ref '///heads/foo'
valid_ref '///heads/foo' --normalize
invalid_ref './foo'
@@ -120,9 +120,12 @@ invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
invalid_ref "$ref" --normalize
-valid_ref "$ref" '--allow-onelevel --normalize'
-invalid_ref "$ref" '--refspec-pattern --normalize'
-valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
+if test_have_prereq NOT_MINGW
+then
+ valid_ref "$ref" '--allow-onelevel --normalize'
+ invalid_ref "$ref" '--refspec-pattern --normalize'
+ valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
+fi
test_expect_success "check-ref-format --branch @{-1}" '
T=$(git write-tree) &&
@@ -166,10 +169,10 @@ invalid_ref_normalized() {
valid_ref_normalized 'heads/foo' 'heads/foo'
valid_ref_normalized 'refs///heads/foo' 'refs/heads/foo'
-valid_ref_normalized '/heads/foo' 'heads/foo'
+test_have_prereq MINGW || valid_ref_normalized '/heads/foo' 'heads/foo'
valid_ref_normalized '///heads/foo' 'heads/foo'
invalid_ref_normalized 'foo'
-invalid_ref_normalized '/foo'
+test_have_prereq MINGW || invalid_ref_normalized '/foo'
invalid_ref_normalized 'heads/foo/../bar'
invalid_ref_normalized 'heads/./foo'
invalid_ref_normalized 'heads\foo'
--
1.7.7.1.gbba15
^ permalink raw reply related
* [PATCH 4/8] git-svn: On MSYS, escape and quote SVN_SSH also if set by the user
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Sebastian Schuberth
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>
From: Sebastian Schuberth <sschuberth@gmail.com>
While GIT_SSH does not require any escaping / quoting (e.g. for paths
containing spaces), SVN_SSH requires it due to its use in a Perl script.
Previously, SVN_SSH has only been escaped and quoted automatically if it
was unset and thus derived from GIT_SSH. For user convenience, do the
escaping and quoting also for a SVN_SSH set by the user. This way, the
user is able to use the same unescaped and unquoted syntax for GIT_SSH
and SVN_SSH.
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
git-svn.perl | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index a0410f0..3b33379 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -22,14 +22,13 @@ $Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
$Git::SVN::Ra::_log_window_size = 100;
$Git::SVN::_minimize_url = 'unset';
-if (! exists $ENV{SVN_SSH}) {
- if (exists $ENV{GIT_SSH}) {
- $ENV{SVN_SSH} = $ENV{GIT_SSH};
- if ($^O eq 'msys') {
- $ENV{SVN_SSH} =~ s/\\/\\\\/g;
- $ENV{SVN_SSH} =~ s/(.*)/"$1"/;
- }
- }
+if (! exists $ENV{SVN_SSH} && exists $ENV{GIT_SSH}) {
+ $ENV{SVN_SSH} = $ENV{GIT_SSH};
+}
+
+if (exists $ENV{SVN_SSH} && $^O eq 'msys') {
+ $ENV{SVN_SSH} =~ s/\\/\\\\/g;
+ $ENV{SVN_SSH} =~ s/(.*)/"$1"/;
}
$Git::SVN::Log::TZ = $ENV{TZ};
--
1.7.7.1.gbba15
^ permalink raw reply related
* [PATCH 8/8] mingw: ensure sockets are initialized before calling gethostname
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Pat Thoyts
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>
If the Windows sockets subsystem has not been initialized yet then an
attempt to get the hostname returns an error and prints a warning to the
console. This solves this issue for msysGit as seen with 'git fetch'.
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
compat/mingw.c | 7 +++++++
compat/mingw.h | 3 +++
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 8947418..efdc703 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1321,6 +1321,13 @@ static void ensure_socket_initialization(void)
initialized = 1;
}
+#undef gethostname
+int mingw_gethostname(char *name, int namelen)
+{
+ ensure_socket_initialization();
+ return gethostname(name, namelen);
+}
+
#undef gethostbyname
struct hostent *mingw_gethostbyname(const char *host)
{
diff --git a/compat/mingw.h b/compat/mingw.h
index ce9dd98..fecf0d0 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -190,6 +190,9 @@ char *mingw_getcwd(char *pointer, int len);
char *mingw_getenv(const char *name);
#define getenv mingw_getenv
+int mingw_gethostname(char *host, int namelen);
+#define gethostname mingw_gethostname
+
struct hostent *mingw_gethostbyname(const char *host);
#define gethostbyname mingw_gethostbyname
--
1.7.7.1.gbba15
^ permalink raw reply related
* [PATCH 5/8] t9901: fix line-ending dependency on windows
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Pat Thoyts
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
t/t9901-git-web--browse.sh | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
index 7906e5d..1185b42 100755
--- a/t/t9901-git-web--browse.sh
+++ b/t/t9901-git-web--browse.sh
@@ -12,7 +12,7 @@ test_expect_success \
echo http://example.com/foo\&bar >expect &&
git config browser.custom.cmd echo &&
git web--browse --browser=custom \
- http://example.com/foo\&bar >actual &&
+ http://example.com/foo\&bar | tr -d "\r" >actual &&
test_cmp expect actual
'
@@ -21,7 +21,7 @@ test_expect_success \
echo http://example.com/foo\;bar >expect &&
git config browser.custom.cmd echo &&
git web--browse --browser=custom \
- http://example.com/foo\;bar >actual &&
+ http://example.com/foo\;bar | tr -d "\r" >actual &&
test_cmp expect actual
'
@@ -30,7 +30,7 @@ test_expect_success \
echo http://example.com/foo#bar >expect &&
git config browser.custom.cmd echo &&
git web--browse --browser=custom \
- http://example.com/foo#bar >actual &&
+ http://example.com/foo#bar | tr -d "\r" >actual &&
test_cmp expect actual
'
@@ -44,7 +44,7 @@ test_expect_success \
chmod +x "fake browser" &&
git config browser.w3m.path "`pwd`/fake browser" &&
git web--browse --browser=w3m \
- http://example.com/foo >actual &&
+ http://example.com/foo | tr -d "\r" >actual &&
test_cmp expect actual
'
@@ -59,7 +59,7 @@ test_expect_success \
}
f" &&
git web--browse --browser=custom \
- http://example.com/foo >actual &&
+ http://example.com/foo | tr -d "\r" >actual &&
test_cmp expect actual
'
--
1.7.7.1.gbba15
^ permalink raw reply related
* [PATCH 0/8] Some patches from msysGit
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
To: Git; +Cc: Junio C Hamano, msysGit, Pat Thoyts
This series collects some recent patches required for msysGit
applied onto 'next' for upstream application.
Johannes Schindelin (4):
t1020: disable the pwd test on MinGW
t1402: Ignore a few cases that must fail due to DOS path expansion
t9001: do not fail only due to CR/LF issues
t9300: do not run --cat-blob-fd related tests on MinGW
Pat Thoyts (3):
t9901: fix line-ending dependency on windows
mergetools: use the correct tool for Beyond Compare 3 on Windows
mingw: ensure sockets are initialized before calling gethostname
Sebastian Schuberth (1):
git-svn: On MSYS, escape and quote SVN_SSH also if set by the user
compat/mingw.c | 7 +++++++
compat/mingw.h | 3 +++
git-svn.perl | 15 +++++++--------
mergetools/bc3 | 9 ++++++++-
t/t1020-subdirectory.sh | 2 +-
t/t1402-check-ref-format.sh | 15 +++++++++------
t/t9001-send-email.sh | 1 +
t/t9300-fast-import.sh | 8 ++++----
t/t9901-git-web--browse.sh | 10 +++++-----
9 files changed, 45 insertions(+), 25 deletions(-)
--
1.7.7.1.gbba15
^ 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