* [PATCH] fix hang in git fetch if pointed at a 0 length bundle
From: Brian Harring @ 2012-01-03 1:13 UTC (permalink / raw)
To: git; +Cc: spearce
git-repo if interupted at the exact wrong time will generate zero
length bundles- literal empty files. git-repo is wrong here, but
git fetch shouldn't effectively spin loop if pointed at a zero
length bundle.
Signed-off-by: Brian Harring <ferringb@chromium.org>
---
bundle.c | 2 +-
t/t5704-bundle.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/bundle.c b/bundle.c
index 4742f27..ac9cc20 100644
--- a/bundle.c
+++ b/bundle.c
@@ -31,7 +31,7 @@ static int strbuf_readline_fd(struct strbuf *sb, int fd)
while (1) {
char ch;
ssize_t len = xread(fd, &ch, 1);
- if (len < 0)
+ if (len <= 0)
return -1;
strbuf_addch(sb, ch);
if (ch == '\n')
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 728ccd8..75ce5b8 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -53,4 +53,14 @@ test_expect_failure 'bundle --stdin <rev-list options>' '
'
+test_expect_success 'die if bundle file is empty' '
+
+ echo -n > empty-bundle
+ timeout 5 git fetch empty-bundle
+ ret=$?
+ [ $ret == 128 ] && return 0
+ return $ret
+
+'
+
test_done
--
1.7.8.2
^ permalink raw reply related
* Re: Git ghost references
From: Johannes Sixt @ 2012-01-03 8:23 UTC (permalink / raw)
To: Chris Leong; +Cc: git
In-Reply-To: <CAJ6vYjfpx-hfDsd+urp5_iS99p0RhmxohOc+TNv7SUWFnYe5wQ@mail.gmail.com>
Am 03.01.2012 00:42, schrieb Chris Leong:
> I seem to have a "ghost reference" - ie. I can check out a reference
> that doesn't appear to exist. Does anyone know what might cause this?
>
> ~/gaf-cvs (project-membership)$ g show-ref | grep production
> ~/gaf-cvs (project-membership)$ g co production
> Note: checking out 'production'.
> ...
The most likely reason is that you have a ref 'production' directly in
the .git directory. Perhaps you or one of your scripts created it
accidentally using 'git update-ref production ae5b621', i.e., without
giving the full ref path name.
-- Hannes
^ permalink raw reply
* Re: Git ghost references
From: Chris Leong @ 2012-01-03 8:32 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4F02BAF9.1000206@kdbg.org>
I did have a production file in my .git and also an empty folder left
in my remotes. Thanks heaps!
On Tue, Jan 3, 2012 at 7:23 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 03.01.2012 00:42, schrieb Chris Leong:
>> I seem to have a "ghost reference" - ie. I can check out a reference
>> that doesn't appear to exist. Does anyone know what might cause this?
>>
>> ~/gaf-cvs (project-membership)$ g show-ref | grep production
>> ~/gaf-cvs (project-membership)$ g co production
>> Note: checking out 'production'.
>> ...
>
> The most likely reason is that you have a ref 'production' directly in
> the .git directory. Perhaps you or one of your scripts created it
> accidentally using 'git update-ref production ae5b621', i.e., without
> giving the full ref path name.
>
> -- Hannes
^ permalink raw reply
* Re: [PATCH] fix hang in git fetch if pointed at a 0 length bundle
From: Johannes Sixt @ 2012-01-03 8:35 UTC (permalink / raw)
To: Brian Harring; +Cc: git, spearce
In-Reply-To: <20120103011352.GA13825@localhost>
Am 03.01.2012 02:13, schrieb Brian Harring:
> git-repo if interupted at the exact wrong time will generate zero
> length bundles- literal empty files. git-repo is wrong here, but
> git fetch shouldn't effectively spin loop if pointed at a zero
> length bundle.
Adding a test case is very much appreciated.
> +test_expect_success 'die if bundle file is empty' '
How about 'empty bundle file is rejected'?
> +
> + echo -n > empty-bundle
'echo -n' is not portable; use simply
>empty-bundle &&
(note the style: no blank after >). Also chain commands using &&.
> + timeout 5 git fetch empty-bundle
Yes, there was an infinite loop. But we do not specifically protect our
git invocations in the test suite against this sort of failure. Just write
test_must_fail git fetch empty-bundle
and end the test case here.
> + ret=$?
> + [ $ret == 128 ] && return 0
> + return $ret
> +
> +'
Furthermore, indentation should be one tabstop, not blanks.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS
From: Ævar Arnfjörð Bjarmason @ 2012-01-03 10:17 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Junio C Hamano, Jakub Narebski, Jeff King
In-Reply-To: <4EFA5EB3.4000802@tu-clausthal.de>
On Wed, Dec 28, 2011 at 01:11, Sven Strickroth
<sven.strickroth@tu-clausthal.de> wrote:
Nom nom, some Perl. Thanks for tackling this. Reviewing it as
requested by Junio.
> diff --git a/git-svn.perl b/git-svn.perl
> index eeb83d3..bcee8aa 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -4415,25 +4415,7 @@ sub username {
>
> sub _read_password {
> my ($prompt, $realm) = @_;
> - my $password = '';
> - if (exists $ENV{GIT_ASKPASS}) {
> - open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
> - $password = <PH>;
> - $password =~ s/[\012\015]//; # \n\r
> - close(PH);
> - } else {
> - print STDERR $prompt;
> - STDERR->flush;
> - require Term::ReadKey;
> - Term::ReadKey::ReadMode('noecho');
> - while (defined(my $key = Term::ReadKey::ReadKey(0))) {
> - last if $key =~ /[\012\015]/; # \n\r
> - $password .= $key;
> - }
> - Term::ReadKey::ReadMode('restore');
> - print STDERR "\n";
> - STDERR->flush;
> - }
> + my $password = Git->prompt($prompt);
> $password;
> }
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index f7ce511..b1c7c50 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -58,7 +58,7 @@ require Exporter;
> command_output_pipe command_input_pipe command_close_pipe
> command_bidi_pipe command_close_bidi_pipe
> version exec_path html_path hash_object git_cmd_try
> - remote_refs
> + remote_refs prompt
> temp_acquire temp_release temp_reset temp_path);
>
>
> @@ -512,6 +512,55 @@ C<git --html-path>). Useful mostly only internally.
> sub html_path { command_oneline('--html-path') }
>
>
> +=item prompt ( PROMPT )
> +
> +Query user C<PROMPT> and return answer from user.
> +
> +Check if GIT_ASKPASS or SSH_ASKPASS is set, use first matching for querying
> +user and return answer. If no *_ASKPASS variable is set, the variable is
> +empty or an error occoured, the terminal is tried as a fallback.
> +
> +=cut
> +
> +sub prompt {
> + my ($self, $prompt) = _maybe_self(@_);
> + my $ret;
> + if (exists $ENV{'GIT_ASKPASS'}) {
> + $ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);
> + }
> + if (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {
> + $ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);
> + }
> + if (!defined $ret) {
> + print STDERR $prompt;
> + STDERR->flush;
> + require Term::ReadKey;
> + Term::ReadKey::ReadMode('noecho');
> + while (defined(my $key = Term::ReadKey::ReadKey(0))) {
> + last if $key =~ /[\012\015]/; # \n\r
> + $ret .= $key;
> + }
> + Term::ReadKey::ReadMode('restore');
> + print STDERR "\n";
> + STDERR->flush;
> + }
> + return $ret;
> +}
Personally I prefer not to write code I don't have to write. Here
you're doing:
my ($self, $prompt) = _maybe_self(@_);
And then never using $self, so there's actually no reason for this to
be an object/class function. It could just be called as:
my $password = Git::prompt($prompt);
Instead of:
my $password = Git->prompt($prompt);
Which means you could change the first line to just:
my ($prompt) = @_;
Which wouldn't leave the reader wondering why this needs to maybe
construct an object just to throw it away.
> +sub _prompt {
> + my ($askpass, $prompt) = @_;
> + unless ($askpass) {
> + return undef;
> + }
The empty list is false in Perl, so explicitly returning undef is
usually the wrong thing. It means that in list context you'll return a
true list (consisting of one undef element), instead of an empty false
one.
$ perl -MData::Dump=dump -wle 'sub a { return } sub b { return
undef }; my @lists = ([a()], [b()]); for (@lists) { print @$_ ? "true"
: "false" }'
false
true
You're not running into that error here since you always use scalar
context. But it's better just to do:
if (xyz) {
return;
}
Unless you want this behavior.
But aside from that I find this code a bit bizarre. The caller is
doing:
if (exists $ENV{'GIT_ASKPASS'}) {
$ret = _prompt($ENV{'GIT_ASKPASS'}, $prompt);
}
if (!defined $ret && exists $ENV{'SSH_ASKPASS'}) {
$ret = _prompt($ENV{'SSH_ASKPASS'}, $prompt);
}
Which means we check whether *_ASKPASS *exists* in the env hash, and
then the function checks whether that value is true or not.
Which means the code implicitly depends on Perl's idea of true/false
values for the names of those commands. E.g. you can't create a
command called "0" and put it in your $PATH.
I suppose the case you actually care about is someone being able to do:
export GIT_ASKPASS=
Instead of the better:
unset GIT_ASKPASS
> + my $ret;
> + open my $fh, "-|", $askpass, $prompt || return undef;
As pointed out already this is a logic error due to the ||.
But even if that worked I think this behavior is a bit
questionable. Why don't we just throw an error at this point? The way
I'd write this would be something like:
my $pass = _prompt('GIT_ASKPASS', $prompt);
And then:
sub _prompt {
my ($env_var, $prompt) = @_;
# or exists(), depending on whether we insist on "unset"
return unless length $ENV{$env_var};
my $command = $ENV{$env_var};
open my $fh, "-|", $command or die "We can't get your password
from `$command' given in $env_var: $!";
chomp(my $password = <$fh>);
close $fh or die "We can't close() `$command' given in $env_var: $!";
return $password;
}
Which would give the user an error if his GIT_ASKPASS command
failed. Check the return value of close() too, and re-structure the
passing around of the env variable so we could give a sensible error
message.
> + $ret = <$fh>;
> + $ret =~ s/[\012\015]//g; # strip \n\r, chomp does not work on all systems (i.e. windows) as expected
Urm yes it does. \n in Perl is magical and doesn't mean \012 like it
does in some languages. It means "The Platform's Native
Newline".
Which is \012 on Unix, \015\012 on Windows, and was \015 on Mac OS
until support for it was removed. This is covered in the second
section of "perldoc perlport".
Can you show me a case where it fails, and under what environment
exactly? Maybe it's e.g.s some Cygwin-specific peculiarity, in which
case we could check for that platform specifically.
^ permalink raw reply
* Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS
From: Sven Strickroth @ 2012-01-03 10:25 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Jakub Narebski, Jeff King
In-Reply-To: <CACBZZX7P9PEq0wZp0d3dSwDjF6J6Z3cO4VtWc9_frBengtqPLw@mail.gmail.com>
Am 03.01.2012 11:17 schrieb Ævar Arnfjörð Bjarmason:
>> + $ret = <$fh>;
>> + $ret =~ s/[\012\015]//g; # strip \n\r, chomp does not work on all systems (i.e. windows) as expected
>
> Urm yes it does. \n in Perl is magical and doesn't mean \012 like it
> does in some languages. It means "The Platform's Native
> Newline".
>
> Which is \012 on Unix, \015\012 on Windows, and was \015 on Mac OS
> until support for it was removed. This is covered in the second
> section of "perldoc perlport".
>
> Can you show me a case where it fails, and under what environment
> exactly? Maybe it's e.g.s some Cygwin-specific peculiarity, in which
> case we could check for that platform specifically.
I'm using msys perl (shipped with msysgit) and there just using chomp()
did not work.
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply
* Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS
From: Ævar Arnfjörð Bjarmason @ 2012-01-03 12:03 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Junio C Hamano, Jakub Narebski, Jeff King
In-Reply-To: <4F02D79B.1020002@tu-clausthal.de>
On Tue, Jan 3, 2012 at 11:25, Sven Strickroth
<sven.strickroth@tu-clausthal.de> wrote:
> Am 03.01.2012 11:17 schrieb Ævar Arnfjörð Bjarmason:
>>> + $ret = <$fh>;
>>> + $ret =~ s/[\012\015]//g; # strip \n\r, chomp does not work on all systems (i.e. windows) as expected
>>
>> Urm yes it does. \n in Perl is magical and doesn't mean \012 like it
>> does in some languages. It means "The Platform's Native
>> Newline".
>>
>> Which is \012 on Unix, \015\012 on Windows, and was \015 on Mac OS
>> until support for it was removed. This is covered in the second
>> section of "perldoc perlport".
>>
>> Can you show me a case where it fails, and under what environment
>> exactly? Maybe it's e.g.s some Cygwin-specific peculiarity, in which
>> case we could check for that platform specifically.
>
> I'm using msys perl (shipped with msysgit) and there just using chomp()
> did not work.
That's odd, what does this print:
perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =
"moo\015\012"; my ($fh, $name) = tempfile(); print $fh $str; close
$fh; open my $in, "<", $name or die $!; my $in_str = <$in>; chomp(my
$cin_str = $in_str); print "in_str:<$in_str> cin_str:<$cin_str>
END\n"'
And how about this:
perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =
"moo\015\012"; my ($fh, $name) = tempfile(); print $fh $str; close
$fh; open my $in, "<:crlf", $name or die $!; my $in_str = <$in>;
chomp(my $cin_str = $in_str); print "in_str:<$in_str>
cin_str:<$cin_str> END\n"'
It could be that there's some bug in either perl or mingw's build of
perl where it won't turn on the :crlf IO layer by default.
^ permalink raw reply
* Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS
From: Ævar Arnfjörð Bjarmason @ 2012-01-03 12:06 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Junio C Hamano, Jakub Narebski, Jeff King
In-Reply-To: <CACBZZX6iMobuU90skpbNPaGQFxYNOAjmZ6ceO4PGqfZSMkgePQ@mail.gmail.com>
On Tue, Jan 3, 2012 at 13:03, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Tue, Jan 3, 2012 at 11:25, Sven Strickroth
> <sven.strickroth@tu-clausthal.de> wrote:
>> Am 03.01.2012 11:17 schrieb Ævar Arnfjörð Bjarmason:
>>>> + $ret = <$fh>;
>>>> + $ret =~ s/[\012\015]//g; # strip \n\r, chomp does not work on all systems (i.e. windows) as expected
>>>
>>> Urm yes it does. \n in Perl is magical and doesn't mean \012 like it
>>> does in some languages. It means "The Platform's Native
>>> Newline".
>>>
>>> Which is \012 on Unix, \015\012 on Windows, and was \015 on Mac OS
>>> until support for it was removed. This is covered in the second
>>> section of "perldoc perlport".
>>>
>>> Can you show me a case where it fails, and under what environment
>>> exactly? Maybe it's e.g.s some Cygwin-specific peculiarity, in which
>>> case we could check for that platform specifically.
>>
>> I'm using msys perl (shipped with msysgit) and there just using chomp()
>> did not work.
>
> That's odd, what does this print:
>
> perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =
> "moo\015\012"; my ($fh, $name) = tempfile(); print $fh $str; close
> $fh; open my $in, "<", $name or die $!; my $in_str = <$in>; chomp(my
> $cin_str = $in_str); print "in_str:<$in_str> cin_str:<$cin_str>
> END\n"'
>
> And how about this:
>
> perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =
> "moo\015\012"; my ($fh, $name) = tempfile(); print $fh $str; close
> $fh; open my $in, "<:crlf", $name or die $!; my $in_str = <$in>;
> chomp(my $cin_str = $in_str); print "in_str:<$in_str>
> cin_str:<$cin_str> END\n"'
>
> It could be that there's some bug in either perl or mingw's build of
> perl where it won't turn on the :crlf IO layer by default.
Or actually you could do this before running your patch, except you
have to change the code to use chomp() instead of your regex hack:
export PERLIO=crlf
If that makes it work we should just do that somewhere else (e.g. at
the top of Git.pm) if we detect Windows, which'll make chomp() work as
intended everywhere.
^ permalink raw reply
* Re: [PATCH] fix hang in git fetch if pointed at a 0 length bundle
From: Nguyen Thai Ngoc Duy @ 2012-01-03 12:07 UTC (permalink / raw)
To: Brian Harring; +Cc: git, spearce
In-Reply-To: <20120103011352.GA13825@localhost>
On Tue, Jan 3, 2012 at 8:13 AM, Brian Harring <ferringb@gmail.com> wrote:
> @@ -31,7 +31,7 @@ static int strbuf_readline_fd(struct strbuf *sb, int fd)
> while (1) {
> char ch;
> ssize_t len = xread(fd, &ch, 1);
> - if (len < 0)
> + if (len <= 0)
> return -1;
> strbuf_addch(sb, ch);
> if (ch == '\n')
I think it should return 0 when len == 0 because strictly speaking eof
is not a fault. It's not really important though because the two
callers in this file work fine either way.
FWIW I went through all xread call sites. All seem to handle return
value <= 0 correctly.
--
Duy
^ permalink raw reply
* Re: [PATCH 1/2] git-svn, perl/Git.pm: add central method for prompting passwords honoring GIT_ASKPASS and SSH_ASKPASS
From: Sven Strickroth @ 2012-01-03 13:18 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Jakub Narebski, Jeff King
In-Reply-To: <CACBZZX6iMobuU90skpbNPaGQFxYNOAjmZ6ceO4PGqfZSMkgePQ@mail.gmail.com>
Am 03.01.2012 13:03 schrieb Ævar Arnfjörð Bjarmason:
>> I'm using msys perl (shipped with msysgit) and there just using chomp()
>> did not work.
But why not drop all \n and \r, since we only accept and wait for a
single line?
chomp($str = <STDIN>) works as expected, but for some reason the line I
got from the ASKPASS tool isn't. Btw. the askpass tool provides a string
followed with \n (c++).
> That's odd, what does this print:
>
> perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =
> "moo\015\012"; my ($fh, $name) = tempfile(); print $fh $str; close
> $fh; open my $in, "<", $name or die $!; my $in_str = <$in>; chomp(my
> $cin_str = $in_str); print "in_str:<$in_str> cin_str:<$cin_str>
> END\n"'
>
> And how about this:
>
> perl -MData::Dumper -MFile::Temp=tempfile -we 'my $str =
> "moo\015\012"; my ($fh, $name) = tempfile(); print $fh $str; close
> $fh; open my $in, "<:crlf", $name or die $!; my $in_str = <$in>;
> chomp(my $cin_str = $in_str); print "in_str:<$in_str>
> cin_str:<$cin_str> END\n"'
>
> It could be that there's some bug in either perl or mingw's build of
> perl where it won't turn on the :crlf IO layer by default.
I get an error in both cases "Das System kann die angegebene Datei nicht
finden".
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply
* [PATCH] fix hang in git fetch if pointed at a 0 length bundle
From: Brian Harring @ 2012-01-03 13:46 UTC (permalink / raw)
To: git
In-Reply-To: <20120103011352.GA13825@localhost>
git-repo if interupted at the exact wrong time will generate zero
length bundles- literal empty files. git-repo is wrong here, but
git fetch shouldn't effectively spin loop if pointed at a zero
length bundle.
Signed-off-by: Brian Harring <ferringb@chromium.org>
---
bundle.c | 4 ++--
t/t5704-bundle.sh | 6 ++++++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/bundle.c b/bundle.c
index 4742f27..b8acf3c 100644
--- a/bundle.c
+++ b/bundle.c
@@ -31,8 +31,8 @@ static int strbuf_readline_fd(struct strbuf *sb, int fd)
while (1) {
char ch;
ssize_t len = xread(fd, &ch, 1);
- if (len < 0)
- return -1;
+ if (len <= 0)
+ return len;
strbuf_addch(sb, ch);
if (ch == '\n')
break;
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 728ccd8..4ae127d 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -53,4 +53,10 @@ test_expect_failure 'bundle --stdin <rev-list options>' '
'
+test_expect_success 'empty bundle file is rejected' '
+
+ >empty-bundle && test_must_fail git fetch empty-bundle
+
+'
+
test_done
--
1.7.8.2
^ permalink raw reply related
* rebase -i does not use a commit it should
From: Kevin @ 2012-01-03 14:00 UTC (permalink / raw)
To: git
I tried to rebase some commits, when I found out it didn't pick all
commits it should.
A distiled situation is this:
A (issues/links_footer)
B (issues/links-footer)
C
| D (master)
| |
| |
| |
| |
| /
*
Here is the output of git log --graph --oneline --format="%h %d" master HEAD
http://paste.pocoo.org/show/529305/
And this is the output of git log --oneline --format="%h" master..
7fb46fd
6efa164
50b5950
When I do git rebase -i master, it only shows the top 2 commits, but
if I do git rebase -i 50b5950^, it does show all three committs.
Anyone knows whats going wrong?
^ permalink raw reply
* Re: rebase -i does not use a commit it should
From: Kevin @ 2012-01-03 14:04 UTC (permalink / raw)
To: git
In-Reply-To: <CAO54GHB1uwJdjj0=dCVp_M4sAFtqooM9hnBSNurje2n+-dGDEA@mail.gmail.com>
Using git version 1.7.8
^ permalink raw reply
* Re: rebase -i does not use a commit it should
From: Kevin @ 2012-01-03 14:18 UTC (permalink / raw)
To: git
In-Reply-To: <CAO54GHAwgbdpdqZw5Pc+jvEyq12DAq4Bio_MbTnmXHsvm-5p8Q@mail.gmail.com>
Ok, problem solved.
The issue was already present in the history of master, and thus it
would not be applied again.
On Tue, Jan 3, 2012 at 3:04 PM, Kevin <ikke@ikke.info> wrote:
> Using git version 1.7.8
^ permalink raw reply
* git-p4 under cygwin
From: Paul Chown @ 2012-01-03 15:32 UTC (permalink / raw)
To: git
I'm trying to use git-p4 to push changes from git into a Perforce repository under Cygwin, using the cygwin git installation. I followed the instructions at http://kb.perforce.com/article/1417/git-p4 and everything works fine until I do the final 'git p4 submit'.
During the submit command I get the following error:
Path '/cygdrive/c/work/perforce/config_test\...' is not under client's root 'c:\work\perforce\config_test'.
Traceback (most recent call last):
File "/usr/local/bin/git-p4", line 2371, in <module>
main()
File "/usr/local/bin/git-p4", line 2366, in main
if not cmd.run(args):
File "/usr/local/bin/git-p4", line 1130, in run
p4_sync("...")
File "/usr/local/bin/git-p4", line 137, in p4_sync
p4_system(["sync", path])
File "/usr/local/bin/git-p4", line 131, in p4_system
subprocess.check_call(real_cmd, shell=expand)
File "/usr/lib/python2.6/subprocess.py", line 498, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['p4', 'sync', '...']' returned non-zero exit status 1
Is there are recommended approach for using git-p4 on Windows? I also tried using the non-Cygwin installations of git, but these don't seem to have any Python support in them, so the git-p4 code won't run at all.
Many thanks in advance
Paul Chown
^ permalink raw reply
* Re: git-p4 under cygwin
From: Thomas Berg @ 2012-01-03 15:54 UTC (permalink / raw)
To: Paul Chown; +Cc: git
In-Reply-To: <1325604726.49597.YahooMailClassic@web132101.mail.ird.yahoo.com>
On Tue, Jan 3, 2012 at 4:32 PM, Paul Chown <pmchown@yahoo.co.uk> wrote:
> Is there are recommended approach for using git-p4 on Windows? I also tried using the non-Cygwin installations of git, but these don't seem to have any Python support in them, so the git-p4 code won't run at all.
I'm not sure, but the error may not be a cygwin related problem at
all, it could be a pure git-p4 usage issue. Do you have a perforce
workspace set up, and do you have the affected files mapped into your
workspace so git-p4 can run p4 sync on them?
Other than that, git-p4 works excellently with the regular msysgit
too. Just install python the regular way on Windows (under C:\Python27
for example) and add it to your path. This python runs just fine in
the MinGW shell too (I think it picks up any PATH changes you make in
Windows). Then just download the git-p4 python script and make sure it
is in your path. This is how I use it.
- Thomas Berg
^ permalink raw reply
* Re: git-p4 under cygwin
From: Thomas Berg @ 2012-01-03 16:07 UTC (permalink / raw)
To: Paul Chown; +Cc: git
In-Reply-To: <1325604726.49597.YahooMailClassic@web132101.mail.ird.yahoo.com>
On Tue, Jan 3, 2012 at 4:32 PM, Paul Chown <pmchown@yahoo.co.uk> wrote:
> Path '/cygdrive/c/work/perforce/config_test\...' is not under client's root 'c:\work\perforce\config_test'.
Ah, sorry, I did not see this part of the error message when I first
replied. This does seem like a cygwin related problem. Not sure how
easy it is to solve. But as mentioned, msysgit works fine with git-p4.
- Thomas
^ permalink raw reply
* Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users
From: Junio C Hamano @ 2012-01-03 18:19 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, Jeff King, Jakub Narebski
In-Reply-To: <4F00B7F3.1060105@tu-clausthal.de>
Sven Strickroth <sven.strickroth@tu-clausthal.de> writes:
> I'm still for the second patch to be applied (maybe w/o the certificate
> filename prompt), too, because this makes git-svn behave the save way as
> git-core does (especially asking for username).
I do not have much issues against the patch if the filename thing is
excluded as a short-term workaround to avoid regression.
For people who have been used to interact with git-svn from the terminal
but has *_ASKPASS for reasons other than their use of git-svn in their
environment, the change to the username codepath is technically a
regression, as they used to be able to see and correct typo while giving
their username but with the patch *_ASKPASS will kick in and they have to
type in bline, but I am not particularly worried about it. It is something
you type very often and committed to your muscle memory anyway.
^ permalink raw reply
* Re: [PATCH] Submodules always use a relative path to gitdir
From: Junio C Hamano @ 2012-01-03 18:27 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Antony Male, git, iveqy
In-Reply-To: <4F007492.8010909@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Am 29.12.2011 23:40, schrieb Junio C Hamano:
>> Antony Male <antony.male@gmail.com> writes:
>> I further wonder if we can get away without using separate-git-dir option
>> in this codepath, though. IOW using
>>
>> git clone $quiet -bare ${reference:+"$reference"} "$url" "$gitdir"
>>
>> might be a better solution.
>
> A quick test shows that using a bare repo won't fly because without the
> core.worktree setting commands that operate on the work tree can't be
> run anymore inside submodules (starting with the initial checkout).
Probably the right thing to do would be to restructure the flow as I
suggested, i.e.
if we do not have it yet
then
git clone --bare ...
fi
# now we have it, make sure they are correct
git config core.bare false
git config core.worktree $there
echo "gitdir: $here" >$there/.git
> Yes, and the core.worktree setting also contains an absolute path. So
> we must either make that relative too and rewrite it on every "git
> submodule add" to record the possibly changed path there or make the
> bare clone work with a work tree (which sounds a bit strange ;-).
Update of core.worktree has to be done regardless of the absolute/relative
differences anyway, no?
The first version of the superproject you trigger module_clone for
submodule $name may happen to have it at $path, module_clone notices that
you do not have it, and the initial "clone --separate-git-dir" will set
the core.worktree to $superproject/$path. Nobody will update it after
that, even when we check out different version of superproject that has
the same submodule $name at a different location in the superproject.
^ permalink raw reply
* Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users
From: Jeff King @ 2012-01-03 18:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sven Strickroth, git, Jakub Narebski
In-Reply-To: <7vzke4vebl.fsf@alter.siamese.dyndns.org>
On Tue, Jan 03, 2012 at 10:19:42AM -0800, Junio C Hamano wrote:
> For people who have been used to interact with git-svn from the terminal
> but has *_ASKPASS for reasons other than their use of git-svn in their
> environment, the change to the username codepath is technically a
> regression, as they used to be able to see and correct typo while giving
> their username but with the patch *_ASKPASS will kick in and they have to
> type in bline, but I am not particularly worried about it. It is something
> you type very often and committed to your muscle memory anyway.
There is one difference between how git and ssh use the ASKPASS
variable. In git, we try it _first_, and fall back to asking on the
terminal. For ssh, they first try the terminal, and fall back to
askpass only when the terminal cannot be opened.
If we tried the terminal first, then it wouldn't be a big deal to use
*_ASKPASS more frequently, since it's a fallback. Of course, that in
itself might be a regression for some people.
I wonder if we should make the order:
1. GIT_ASKPASS
2. terminal
3. SSH_ASKPASS
to help make our use SSH_ASKPASS better match that of ssh. I dunno. I am
not an askpass user these days, so I don't know what people expect or
want.
-Peff
^ permalink raw reply
* Re: [PATCH] Submodules always use a relative path to gitdir
From: Junio C Hamano @ 2012-01-03 18:45 UTC (permalink / raw)
To: Phil Hord; +Cc: Antony Male, git, iveqy
In-Reply-To: <CABURp0pdvf9Eo_pM2UCYUBANOJOGON6pQS-SXuCWQE=s2XNOfQ@mail.gmail.com>
Phil Hord <phil.hord@gmail.com> writes:
>> Again, who said that you are allowed to move the superproject directory in
>> the first place? I would understand that it might be nicer if it could be
>> moved but I haven't thought this through thoroughly yet---there may be
>> other side effects from doing so, other than the relativeness of "gitdir".
>
> Previously it was accepted practice to clone a local repo with rsync.
> This method continues to work well even with submodules before
> git-files became the norm. But now it breaks because of the absolute
> paths.
You are utterly mistaken.
There are 47 million things you can do to your repository outside of the
control of git, and obviously we do not exhaustively enumerate everything
that ought to work (or not work). Anything that is not explicitly allowed
in the documentation is, ehh, not allowed.
Many such things may happen to work, either by accident or as a natural
consequence of the design. Some things needs adjustments after you do them
without telling git. There is a difference between what is not allowed and
what is explicitly forbidden.
Copying with rsync (or cp for that matter) is one good example. Doing so
will cause the cached stat information in the index and the working tree
files go out of sync, and diff-files will give you false differences after
that. You would adjust to that by running "update-index --refresh". So we
do not say "you are allowed to cp and git will guarantee everything will
work as-is", but it is not explicitly forbidden. As long as you make
necessary adjustments, you can keep using the copied repository.
> So, who said you were NOT allowed to move the superproject directory
> directory in the first place?
See above.
And the extent of the design of
echo "gitdir: $there" >.git && git config core.worktree "$(pwd)"
is to work with the locations of these two places as they are set up.
Moving one or the other or both may or may not work without adjusting to
what you did. If you "mv $there $newlocation" (the repository) behind
Git's back, you may need to update .git to point at the new location of
the repository. If you move your working tree woth "mv", you may need to
update core.worktree to point at the new location of the working tree.
And until you do so things may not work. That is why we do not explicitly
say "you can move them to arbitrary places without telling git and things
will work"---because that is not the case.
> This doesn't explain why one path is absolute and one is relative.
Exactly. Because absolute/relative does not come into play as the scope of
the design did not include supporting "moving" one, the other, or both to
arbitrary places without telling git.
^ permalink raw reply
* Re: [PATCHv2] stash: Don't fail if work dir contains file named 'HEAD'
From: Junio C Hamano @ 2012-01-03 18:45 UTC (permalink / raw)
To: Jonathon Mah; +Cc: git, Thomas Rast
In-Reply-To: <A244F415-73A1-4A77-BC4A-AC7F85946F02@JonathonMah.com>
Thanks, both.
^ permalink raw reply
* Re: [PATCH] Work around sed portability issue in t8006-blame-textconv
From: Junio C Hamano @ 2012-01-03 19:05 UTC (permalink / raw)
To: Ben Walton; +Cc: git
In-Reply-To: <1325339068-6063-1-git-send-email-bwalton@artsci.utoronto.ca>
Ben Walton <bwalton@artsci.utoronto.ca> writes:
> In test 'blame --textconv with local changes' of t8006-blame-textconv,
> using /usr/xpg4/bin/sed on Solaris as set by SANE_TOOL_PATH, an
> additional newline was added to the output from the 'helper' script
> driven by git attributes.
>
> This was noted by sed with a message such as:
> sed: Missing newline at end of file zero.bin.
>
> In turn, this was triggering a fatal error from git blame:
> fatal: unable to read files to diff
Interesting. A file with incomplete line technically is not a text file
and sed is supposed to work on text files, so it is allowed to be picky.
> Use perl -p -e instead of sed -e to work around this portability issue
> as it will not insert the newline.
I am not sure if additional newline is the problem, or the exit status
from sed is, from your description. Your first paragraph says you will get
output from sed but with an extra newline, and then later you said blame
noticed an error in its attempt to read the contents. I am suspecting that
it checked the exit status from the textconv subprocess and noticed the
error and that is the cause of the issue, but could you clarify? IOW, I
am suspecting that replacing "as it will not insert the newline" with "as
it does not error out on an incomplete line" is necessary in this
sentence.
Thanks.
^ permalink raw reply
* Re: Stashing individual files
From: Jeff King @ 2012-01-03 19:06 UTC (permalink / raw)
To: Chris Leong; +Cc: git
In-Reply-To: <CAJ6vYjduoBNrRcvcvQbX_yY-3-Qm5ZbXOM0WQpWRwC1H1OCqaA@mail.gmail.com>
On Tue, Jan 03, 2012 at 10:32:02AM +1100, Chris Leong wrote:
> Thanks for making such a wonderful product. I find the stash command
> really useful, but it doesn't work very well when I just need to
> temporarily revert one or two files. I know that there is the
> interactive command, but if you have modified a large number of files,
> then it takes quite a bit of effort. Is there any way I can define an
> alias, stashfiles, so that I can just type git stashfiles file1 file2?
> Also, please consider adding such a feature into a future version.
I have sometimes wanted this, too. One problem is that the arguments in
a "stash save" get sucked into the message. I really wish it were:
git stash save [-m <msg>] [[--] <pathspec...>]
which would match other git commands. And related, it would be nice to
have:
git stash foo.c bar.c
but that conflicts with our safety-valve to avoid accidentally stashing
when no command is given.
For now, we could probably do it like this:
git stash save [<message>] [-- <pathspec...>]
IOW, make the "--" a requirement for specifying filenames. The only
regression is that "--" as a single argument can no longer be used in
stash messages. So this works now:
git stash save working on foo -- needs bar
but would be interpreted under my proposal as stashing "needs" and "bar"
with the message "working on foo". You would instead have to spell it:
git stash save "working on foo -- needs bar"
I think that would be OK compromise, though. I'd rather not introduce a
whole new "stashfiles" command (or even a new subcommand of stash) if we
can avoid it.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] daemon: add tests
From: Jeff King @ 2012-01-03 19:18 UTC (permalink / raw)
To: Clemens Buchacher
Cc: Jonathan Nieder, git, Junio C Hamano, Erik Faye-Lund,
Ilari Liusvaara, Nguyễn Thái Ngọc Duy
In-Reply-To: <20120102194711.GA25296@ecki.lan>
On Mon, Jan 02, 2012 at 08:47:11PM +0100, Clemens Buchacher wrote:
> On Mon, Jan 02, 2012 at 03:25:08AM -0600, Jonathan Nieder wrote:
> >
> > > [Subject: daemon: add tests]
> >
> > Can't believe I missed this. That seems like a worthy cause ---
> > can someone remind me why this is dropped, or if there are any
> > tweaks I can help with to get it picked up again?
>
> We were discussing some open issues with patch 2/2, which was based
> on the tests. I later abandoned the idea for that patch. But the
> tests should be ok by themselves.
Yes, I'd like to see them included, even without the second patch. We
currently have zero tests for git-daemon, so even just verifying that
it starts and can let people fetch is an improvement.
-Peff
^ 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