Re: [PATCH] git-svn: Fix termination issues for remote svn connections
"Uli Heller" writes:
> On Fri, September 6, 2013 2:44 pm, Kyle J. McKay wrote:
> ...
>> In any case, I can now reproduce the problem (serf 1.3.1 still breaks
>> since it does not yet contain the fix and it is the most recent serf
>> release available).
>>
>> And the Git/SVN/Ra.pm fix does eliminate the problem for me (both with
>> bulk updates and with skelta updates -- the crash occurs with either).
>
> Great. So I don't have to run any more tests ;)
>
> What shall I do next? Add some inline comments to the patch?
Something like this?
-- >8 --
From: Uli Heller
Subject: [PATCH] git-svn: fix termination issues for remote svn connections
git-svn used in combination with serf to talk to svn repository
served over HTTPS dumps core on termination.
This is caused by a bug in serf, and the most recent serf release
1.3.1 still exhibits the problem; a fix for the bug exists (see
https://code.google.com/p/serf/source/detail?r=2146).
Until the bug is fixed, work around the issue within the git perl
module Ra.pm by freeing the private copy of the remote access object
on termination, which seems to be sufficient to prevent the error
from happening.
Note: Since subversion-1.8.0 and later do require serf-1.2.1 or
later, this issue typically shows up when upgrading to a recent
version of subversion.
Credits go to Jonathan Lambrechts for proposing a fix to Ra.pm,
Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and
pointing me to that fix.
Signed-off-by: Uli Heller
Tested-by: Kyle J. McKay
---
perl/Git/SVN/Ra.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 75ecc42..78dd346 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -32,6 +32,14 @@ BEGIN {
}
}
+# serf has a bug that leads to a coredump upon termination if the
+# remote access object is left around (not fixed yet in serf 1.3.1).
+# Explicitly free it to work around the issue.
+END {
+ $RA = undef;
+ $ra_invalid = 1;
+}
+
sub _auth_providers () {
my @rv = (
SVN::Client::get_simple_provider(),
--
1.8.4-431-g94f3a6b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Fix termination issues for remote svn connections
On Fri, September 6, 2013 2:44 pm, Kyle J. McKay wrote: > On Sep 6, 2013, at 05:06, Uli Heller wrote: >>> I'm using Git built from master (57e4c1783). I see the same behavior >>> both with and without the SVN/Ra.pm patch (and using both bulk >>> updates >>> and skelta mode). Does the problem not happen on a git svn clone? I >>> can force serf back to version 1.2.1 and try that version just to >>> see, >>> but I would like to have an example of a known failing git svn >>> command >>> for testing purposes. Thanks. >> >> I think this command should produce the error: >> >> git svn clone --stdlayout https://github.com/uli-heller/uli-javawrapper >> >> You can use any other svn repo as well, you only have to specify an >> HTTPS >> url. > > Yes, that does it. Interesting that cloning from > "https://github.com/uli-heller/uli-javawrapper > " core dumps while cloning from > "http://github.com/uli-heller/uli-javawrapper > " does not even though the latter redirects to > "https://github.com/uli-heller/uli-javawrapper > ". > > In any case, I can now reproduce the problem (serf 1.3.1 still breaks > since it does not yet contain the fix and it is the most recent serf > release available). > > And the Git/SVN/Ra.pm fix does eliminate the problem for me (both with > bulk updates and with skelta updates -- the crash occurs with either). Great. So I don't have to run any more tests ;) What shall I do next? Add some inline comments to the patch? --- Best regards, Uli. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Fix termination issues for remote svn connections
On Sep 5, 2013, at 11:48, Junio C Hamano wrote:
I am Cc'ing Kyle McKay who apparently had some experience working
with git-svn with newer svn that can only use serf, hoping that we
can get an independent opinion/test just to be sure.
On Sep 3, 2013, at 00:35, Uli Heller wrote:
When using git-svn in combination with serf-1.2.1 core dumps are
created on termination. This is caused by a bug in serf, a fix for
the bug exists (see https://code.google.com/p/serf/source/detail?r=2146)
.
Nevertheless, I think it makes sense to fix the issue within the
git perl module Ra.pm, too. The change frees the private copy of
the remote access object on termination which prevents the error
from happening.
Note: Since subversion-1.8.0 and later do require serf-1.2.1 or later,
the core dumps typically do show up when upgrading to a recent version
of subversion.
Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.
Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and
pointing me to that fix.
---
In addition to the needed 'Signed-off-by:' a mention should be made
here of the need to use 'https:' to reproduce the problem.
perl/Git/SVN/Ra.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 75ecc42..78dd346 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -32,6 +32,11 @@ BEGIN {
}
}
+END {
+ $RA = undef;
+ $ra_invalid = 1;
+}
+
sub _auth_providers () {
my @rv = (
SVN::Client::get_simple_provider(),
--
1.8.4
Tested-by: Kyle J. McKay
I was able to reproduce the core dumps using subversion 1.8.3 (the
most recent subversion release) with serf 1.3.1 (the most recent serf
release) and verify that this patch eliminates the core dump at git-
svn termination time.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Fix termination issues for remote svn connections
On Sep 6, 2013, at 05:06, Uli Heller wrote: I'm using Git built from master (57e4c1783). I see the same behavior both with and without the SVN/Ra.pm patch (and using both bulk updates and skelta mode). Does the problem not happen on a git svn clone? I can force serf back to version 1.2.1 and try that version just to see, but I would like to have an example of a known failing git svn command for testing purposes. Thanks. I think this command should produce the error: git svn clone --stdlayout https://github.com/uli-heller/uli-javawrapper You can use any other svn repo as well, you only have to specify an HTTPS url. Yes, that does it. Interesting that cloning from "https://github.com/uli-heller/uli-javawrapper " core dumps while cloning from "http://github.com/uli-heller/uli-javawrapper " does not even though the latter redirects to "https://github.com/uli-heller/uli-javawrapper ". In any case, I can now reproduce the problem (serf 1.3.1 still breaks since it does not yet contain the fix and it is the most recent serf release available). And the Git/SVN/Ra.pm fix does eliminate the problem for me (both with bulk updates and with skelta updates -- the crash occurs with either). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Fix termination issues for remote svn connections
On Fri, September 6, 2013 1:46 pm, Kyle J. McKay wrote:
> On Sep 5, 2013, at 11:48, Junio C Hamano wrote:
>> "Uli Heller" writes:
>>
>>> When using git-svn in combination with serf-1.2.1 core dumps are
>>> created on termination. This is caused by a bug in serf, a fix for
>>> the bug exists (see
>>> https://code.google.com/p/serf/source/detail?r=2146)
>>> .
>>> Nevertheless, I think it makes sense to fix the issue within the
>>> git perl module Ra.pm, too. The change frees the private copy of
>>> the remote access object on termination which prevents the error
>>> from happening.
>>>
>>> Note: Since subversion-1.8.0 and later do require serf-1.2.1 or
>>> later,
>>> the core dumps typically do show up when upgrading to a recent
>>> version
>>> of subversion.
>>>
>>> Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.
>>> Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and
>>> pointing me to that fix.
>>> ---
>>
>> Thanks. Please sign-off your patch.
>>
>> I am Cc'ing Kyle McKay who apparently had some experience working
>> with git-svn with newer svn that can only use serf, hoping that we
>> can get an independent opinion/test just to be sure. Also Cc'ed is
>> Eric Wong who has been the official git-svn area expert, but I
>> understand that Eric hasn't needed to use git-svn for quite a while,
>> so it is perfectly fine if he does not have any comment on this one.
>>
>> We may want to find a volunteer to move "git svn" forward as a new
>> area expert (aka subsystem maintainer), by the way.
>>
>>> perl/Git/SVN/Ra.pm | 5 +
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
>>> index 75ecc42..78dd346 100644
>>> --- a/perl/Git/SVN/Ra.pm
>>> +++ b/perl/Git/SVN/Ra.pm
>>> @@ -32,6 +32,11 @@ BEGIN {
>>> }
>>> }
>>>
>>> +END {
>>> + $RA = undef;
>>> + $ra_invalid = 1;
>>> +}
>>> +
>>> sub _auth_providers () {
>>> my @rv = (
>>> SVN::Client::get_simple_provider(),
>
> I have not, as of yet, been able to reproduce the problem, so I cannot
> verify the solution. Maybe Uli can provide an example of a git-svn
> command that demonstrates the failure?
>
> I am running a fresh build of subversion 1.8.3 with serf version 1.3.1
> (the most recent serf release).
>
> According to the serf library history, version 1.3.1 of serf was
> tagged at revision 2139 from revision 2138, but the serf fix mentioned
> above was checked in at revision 2146, so it cannot possibly be in the
> serf 1.3.1 release.
>
> I'm using Git built from master (57e4c1783). I see the same behavior
> both with and without the SVN/Ra.pm patch (and using both bulk updates
> and skelta mode). Does the problem not happen on a git svn clone? I
> can force serf back to version 1.2.1 and try that version just to see,
> but I would like to have an example of a known failing git svn command
> for testing purposes. Thanks.
I think this command should produce the error:
git svn clone --stdlayout https://github.com/uli-heller/uli-javawrapper
You can use any other svn repo as well, you only have to specify an HTTPS
url.
[Yes, I know you typically don't clone github via git svn]
I'll do some more tests using git HEAD and serf 1.3.1 when I'm back home.
Thanks + best regards, Uli
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Fix termination issues for remote svn connections
On Sep 5, 2013, at 11:48, Junio C Hamano wrote:
"Uli Heller" writes:
When using git-svn in combination with serf-1.2.1 core dumps are
created on termination. This is caused by a bug in serf, a fix for
the bug exists (see https://code.google.com/p/serf/source/detail?r=2146)
.
Nevertheless, I think it makes sense to fix the issue within the
git perl module Ra.pm, too. The change frees the private copy of
the remote access object on termination which prevents the error
from happening.
Note: Since subversion-1.8.0 and later do require serf-1.2.1 or
later,
the core dumps typically do show up when upgrading to a recent
version
of subversion.
Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.
Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and
pointing me to that fix.
---
Thanks. Please sign-off your patch.
I am Cc'ing Kyle McKay who apparently had some experience working
with git-svn with newer svn that can only use serf, hoping that we
can get an independent opinion/test just to be sure. Also Cc'ed is
Eric Wong who has been the official git-svn area expert, but I
understand that Eric hasn't needed to use git-svn for quite a while,
so it is perfectly fine if he does not have any comment on this one.
We may want to find a volunteer to move "git svn" forward as a new
area expert (aka subsystem maintainer), by the way.
perl/Git/SVN/Ra.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 75ecc42..78dd346 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -32,6 +32,11 @@ BEGIN {
}
}
+END {
+ $RA = undef;
+ $ra_invalid = 1;
+}
+
sub _auth_providers () {
my @rv = (
SVN::Client::get_simple_provider(),
I have not, as of yet, been able to reproduce the problem, so I cannot
verify the solution. Maybe Uli can provide an example of a git-svn
command that demonstrates the failure?
I am running a fresh build of subversion 1.8.3 with serf version 1.3.1
(the most recent serf release).
According to the serf library history, version 1.3.1 of serf was
tagged at revision 2139 from revision 2138, but the serf fix mentioned
above was checked in at revision 2146, so it cannot possibly be in the
serf 1.3.1 release.
I'm using Git built from master (57e4c1783). I see the same behavior
both with and without the SVN/Ra.pm patch (and using both bulk updates
and skelta mode). Does the problem not happen on a git svn clone? I
can force serf back to version 1.2.1 and try that version just to see,
but I would like to have an example of a known failing git svn command
for testing purposes. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Fix termination issues for remote svn connections
Eric Wong writes:
> Junio C Hamano wrote:
>> "Uli Heller" writes:
>> > Nevertheless, I think it makes sense to fix the issue within the
>> > git perl module Ra.pm, too. The change frees the private copy of
>> > the remote access object on termination which prevents the error
>> > from happening.
>
>> Thanks. Please sign-off your patch.
>>
>> I am Cc'ing Kyle McKay who apparently had some experience working
>> with git-svn with newer svn that can only use serf, hoping that we
>> can get an independent opinion/test just to be sure. Also Cc'ed is
>> Eric Wong who has been the official git-svn area expert, but I
>> understand that Eric hasn't needed to use git-svn for quite a while,
>> so it is perfectly fine if he does not have any comment on this one.
>>
>> We may want to find a volunteer to move "git svn" forward as a new
>> area expert (aka subsystem maintainer), by the way.
>
> Correct, git-svn has the effect of being self-obsoleting.
>
> I agree with adding a workaround for broken things, however
> I suggest a code comment explaining why it is necessary.
> The commit message is important, too, but might get harder to track
> down if there's code movement/refactoring in the future.
Thanks for a good suggestion. I agree that this addition is a good
example where in-code comment would really help the future readers.
>
>> > +END {
>> > + $RA = undef;
>> > + $ra_invalid = 1;
>> > +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Fix termination issues for remote svn connections
Junio C Hamano wrote:
> "Uli Heller" writes:
> > Nevertheless, I think it makes sense to fix the issue within the
> > git perl module Ra.pm, too. The change frees the private copy of
> > the remote access object on termination which prevents the error
> > from happening.
> Thanks. Please sign-off your patch.
>
> I am Cc'ing Kyle McKay who apparently had some experience working
> with git-svn with newer svn that can only use serf, hoping that we
> can get an independent opinion/test just to be sure. Also Cc'ed is
> Eric Wong who has been the official git-svn area expert, but I
> understand that Eric hasn't needed to use git-svn for quite a while,
> so it is perfectly fine if he does not have any comment on this one.
>
> We may want to find a volunteer to move "git svn" forward as a new
> area expert (aka subsystem maintainer), by the way.
Correct, git-svn has the effect of being self-obsoleting.
I agree with adding a workaround for broken things, however
I suggest a code comment explaining why it is necessary.
The commit message is important, too, but might get harder to track
down if there's code movement/refactoring in the future.
> > +END {
> > + $RA = undef;
> > + $ra_invalid = 1;
> > +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-svn: Fix termination issues for remote svn connections
"Uli Heller" writes:
> When using git-svn in combination with serf-1.2.1 core dumps are
> created on termination. This is caused by a bug in serf, a fix for
> the bug exists (see https://code.google.com/p/serf/source/detail?r=2146).
> Nevertheless, I think it makes sense to fix the issue within the
> git perl module Ra.pm, too. The change frees the private copy of
> the remote access object on termination which prevents the error
> from happening.
>
> Note: Since subversion-1.8.0 and later do require serf-1.2.1 or later,
> the core dumps typically do show up when upgrading to a recent version
> of subversion.
>
> Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.
> Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and
> pointing me to that fix.
> ---
Thanks. Please sign-off your patch.
I am Cc'ing Kyle McKay who apparently had some experience working
with git-svn with newer svn that can only use serf, hoping that we
can get an independent opinion/test just to be sure. Also Cc'ed is
Eric Wong who has been the official git-svn area expert, but I
understand that Eric hasn't needed to use git-svn for quite a while,
so it is perfectly fine if he does not have any comment on this one.
We may want to find a volunteer to move "git svn" forward as a new
area expert (aka subsystem maintainer), by the way.
> perl/Git/SVN/Ra.pm | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
> index 75ecc42..78dd346 100644
> --- a/perl/Git/SVN/Ra.pm
> +++ b/perl/Git/SVN/Ra.pm
> @@ -32,6 +32,11 @@ BEGIN {
> }
> }
>
> +END {
> + $RA = undef;
> + $ra_invalid = 1;
> +}
> +
> sub _auth_providers () {
> my @rv = (
> SVN::Client::get_simple_provider(),
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-svn: Fix termination issues for remote svn connections
When using git-svn in combination with serf-1.2.1 core dumps are
created on termination. This is caused by a bug in serf, a fix for
the bug exists (see https://code.google.com/p/serf/source/detail?r=2146).
Nevertheless, I think it makes sense to fix the issue within the
git perl module Ra.pm, too. The change frees the private copy of
the remote access object on termination which prevents the error
from happening.
Note: Since subversion-1.8.0 and later do require serf-1.2.1 or later,
the core dumps typically do show up when upgrading to a recent version
of subversion.
Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.
Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and
pointing me to that fix.
---
perl/Git/SVN/Ra.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 75ecc42..78dd346 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -32,6 +32,11 @@ BEGIN {
}
}
+END {
+ $RA = undef;
+ $ra_invalid = 1;
+}
+
sub _auth_providers () {
my @rv = (
SVN::Client::get_simple_provider(),
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
