All,
I applied Kyle's latest patch
> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
> index 622535e2..96888228 100644
> --- a/perl/Git/SVN/Ra.pm
> +++ b/perl/Git/SVN/Ra.pm
> @@ -391,6 +391,7 @@ sub longest_common_path {
> sub gs_fetch_loop_common {
> my ($self, $base, $head, $gsv, $globs) = @_;
> return if ($base > $head);
> + $::_repository->_open_cat_blob_if_needed;
> my $gpool = SVN::Pool->new_default;
> my $ra_url = $self->url;
> my $reload_ra = sub {
alone and this fixes the bug for me. Thanks a lot, Kyle!
Cheers,
Nico
On Thu, Feb 26, 2015 at 12:37 AM, Nico Schlömer
<nico.schloemer@xxxxxxxxx> wrote:
> Kyle,
>
> you are absolutely correct, I used the wrong Git installation in my last
> email. With both your and Eric's patches applied, I'm getting
> ```
> [...]
> r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn)
> Unexpected result returned from git cat-file at
> /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 344.
> Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at
> /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 345, <GEN16> line
> 757.
>
> error closing pipe: Bad file descriptor at
> /home/nschloe/libexec/git-core/git-svn line 0.
> error closing pipe: Bad file descriptor at
> /home/nschloe/libexec/git-core/git-svn line 0.
> ```
> where line 344 is
> ```
> $size = $::_repository->cat_blob($fb->{blob}, $base);
> ```
> Adding the line in `perl/Git/SVN/Ra.pm` as per your suggestion does not
> change this.
>
> Cheers,
> Nico
>
>
> On Wed, Feb 25, 2015 at 9:34 PM Kyle J. McKay <mackyle@xxxxxxxxx> wrote:
>>
>> On Feb 25, 2015, at 07:07, Nico Schlömer wrote:
>>
>> > Thanks Kyle for the patch! I applied it and ran
>> > ```
>> > git svn clone https://geuz.org/svn/gmsh/trunk
>> > ```
>> > Unfortunately, I'm still getting
>> > ```
>> > [...]
>> > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn)
>> > error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/
>> > Fetcher.pm line 335.
>> > error closing pipe: Bad file descriptor at /usr/share/perl5/Git/SVN/
>> > Fetcher.pm line 335.
>> > out pipe went bad at /usr/share/perl5/Git.pm line 955.
>> > ```
>>
>> Are you certain you're running with the patch? I ask because earlier
>> you sent this:
>>
>> On Jan 31, 2015, at 04:51, Nico Schlömer wrote:
>>
>> > I tried the patch and I still get
>> > ```
>> > [...]
>> > r100 = e2a9b5baa2cebb18591ecb04ff350410d52f36de (refs/remotes/git-svn)
>> > Unexpected result returned from git cat-file at
>> > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 335.
>> > Failed to read object 619f9d1d857fb287d06a70c9dac6b8b534d0de6a at
>> > /home/nschloe/share/perl/5.18.2/Git/SVN/Fetcher.pm line 336, <GEN16>
>> > line 757.
>> >
>> > error closing pipe: Bad file descriptor at
>> > /home/nschloe/libexec/git-core/git-svn line 0.
>> > error closing pipe: Bad file descriptor at
>> > /home/nschloe/libexec/git-core/git-svn line 0.
>> > ```
>> > when
>> > ```
>> > git svn clone https://geuz.org/svn/gmsh/trunk
>> > ```
>>
>> And as the patch below applies to Fetcher.pm at line 322 and inserts 8
>> lines, I do not see how you could be getting the same error message at
>> line 335 if the patch was present. Even if you manually applied it by
>> only inserting the two lines of code, the line numbers would be at
>> least 337, not 335 as reported above. I also notice the path to
>> Fetcher.pm is different from your earlier report.
>>
>> That said, the fact that it happens right after r100 (which is when
>> SVN::Pool::clear is getting called) suggests there's another file
>> handle getting caught up in the SVN::Pool somehow. (When I try to
>> clone gmsh, I got to r4885 before a server disconnection.)
>>
>> It could be as simple as the open2 call FileHandle result in later
>> perl versions ends up in the SVN::Pool whereas in earlier Perl and/or
>> svn versions it does not.
>>
>> What happens if you add this change on top of the Fetcher.pm change:
>>
>> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
>> index 622535e2..96888228 100644
>> --- a/perl/Git/SVN/Ra.pm
>> +++ b/perl/Git/SVN/Ra.pm
>> @@ -391,6 +391,7 @@ sub longest_common_path {
>> sub gs_fetch_loop_common {
>> my ($self, $base, $head, $gsv, $globs) = @_;
>> return if ($base > $head);
>> + $::_repository->_open_cat_blob_if_needed;
>> my $gpool = SVN::Pool->new_default;
>> my $ra_url = $self->url;
>> my $reload_ra = sub {
>>
>> -Kyle
>>
>> > Cheers,
>> > Nico
>> >
>> > On Wed, Feb 25, 2015 at 11:20 AM Kyle J. McKay <mackyle@xxxxxxxxx>
>> > wrote:
>> > I believe I have been able to track down this problem and implement a
>> > fix. Please report back if this patch fixes the problem for you.
>> >
>> > -Kyle
>> >
>> > -- 8< --
>> > Subject: [PATCH] Git::SVN::Fetcher: avoid premature 'svn_hash' temp
>> > file closure
>> >
>> > Since b19138b (git-svn: Make it incrementally faster by minimizing
>> > temp
>> > files, v1.6.0), git-svn has been using the Git.pm temp_acquire and
>> > temp_release mechanism to avoid unnecessary temp file churn and
>> > provide
>> > a speed boost.
>> >
>> > However, that change introduced a call to temp_acquire inside the
>> > Git::SVN::Fetcher::close_file function for an 'svn_hash' temp file.
>> > Because an SVN::Pool is active at the time this function is called, if
>> > the Git::temp_acquire function ends up actually creating a new
>> > FileHandle for the temp file (which it will the first time it's called
>> > with the name 'svn_hash') that FileHandle will end up in the SVN::Pool
>> > and should that pool have SVN::Pool::clear called on it that
>> > FileHandle
>> > will be closed out from under Git::temp_acquire.
>> >
>> > Since the only call site to Git::temp_acquire with the name 'svn_hash'
>> > is inside the close_file function, if an 'svn_hash' temp file is ever
>> > created its FileHandle is guaranteed to be created in the active
>> > SVN::Pool.
>> >
>> > This has not been a problem in the past because the SVN::Pool was not
>> > being cleared. However, since dfa72fdb (git-svn: reload RA every
>> > log-window-size, v2.2.0) the pool has been getting cleared
>> > periodically
>> > at which point the FileHandle for the 'svn_hash' temp file gets
>> > closed.
>> > Any subsequent calls to Git::temp_acquire for 'svn_hash', however,
>> > succeed without creating/opening a new temporary file since it still
>> > has
>> > the now invalid FileHandle in its cache. Callers that then attempt to
>> > use that FileHandle fail with an error.
>> >
>> > We avoid this problem by making sure the 'svn_hash' temp file is
>> > created
>> > in the same place the 'svn_delta_...' and 'git_blob_...' temp files
>> > are
>> > (and then temp_release'd) so that it can be safely used inside the
>> > close_file function without having its FileHandle end up in an
>> > SVN::Pool
>> > that gets cleared.
>> >
>> > Signed-off-by: Kyle J. McKay <mackyle@xxxxxxxxx>
>> > ---
>> > perl/Git/SVN/Fetcher.pm | 8 ++++++++
>> > 1 file changed, 8 insertions(+)
>> >
>> > diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
>> > index 10edb277..613055a3 100644
>> > --- a/perl/Git/SVN/Fetcher.pm
>> > +++ b/perl/Git/SVN/Fetcher.pm
>> > @@ -322,6 +322,14 @@ sub apply_textdelta {
>> > # (but $base does not,) so dup() it for reading in close_file
>> > open my $dup, '<&', $fh or croak $!;
>> > my $base = $::_repository->temp_acquire("git_blob_${$}_
>> > $suffix");
>> > + # close_file may call temp_acquire on 'svn_hash', but
>> > because of the
>> > + # call chain, if the temp_acquire call from close_file ends
>> > up being the
>> > + # call that first creates the 'svn_hash' temp file, then the
>> > FileHandle
>> > + # that's created as a result will end up in an SVN::Pool
>> > that we clear
>> > + # in SVN::Ra::gs_fetch_loop_common. Avoid that by making
>> > sure the
>> > + # 'svn_hash' FileHandle is already created before close_file
>> > is called.
>> > + my $tmp_fh = $::_repository->temp_acquire('svn_hash');
>> > + $::_repository->temp_release($tmp_fh, 1);
>> >
>> > if ($fb->{blob}) {
>> > my ($base_is_link, $size);
>> > --
>>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html