Git.pm with recent File::Temp fail

Git.pm with recent File::Temp fail

[H.Merijn Brand]

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

git-1.8.2, perl-5.16.3, File::Temp-0.23

Without patch:

$ git svn fetch
'tempfile' can't be called as a method at /pro/lib/perl5/site_perl/5.16.3/Git.pm line 1117.

After patch:

$ git svn fetch
        M       t/06virtual.t
r15506 = 6c65be7ff36ffc6fd9b960a4b470ca297103004e (refs/remotes/git-svn)
⋮

patch attached

-- 
H.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/
using perl5.00307 .. 5.17   porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/        http://www.test-smoke.org/
http://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/

[-- Attachment #2: 0001-Syntax-error-in-Git.pm-for-File-Temp-0.23.patch --]
[-- Type: text/x-patch, Size: 1355 bytes --]

>From e78bf3e99deb26050f8515076db63075f6d0d171 Mon Sep 17 00:00:00 2001
From: "H.Merijn Brand - Tux" <h.m.brand@xs4all.nl>
Date: Fri, 22 Mar 2013 20:56:53 +0100
Subject: [PATCH] Syntax error in Git.pm for File::Temp-0.23
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.2"

This is a multi-part message in MIME format.
--------------1.8.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


Testing with perl-5.16.3 and most recent File::Temp-0.23 revealed:

$ git svn fetch
'tempfile' can't be called as a method at /pro/lib/perl5/site_perl/5.16.3/Git.pm line 1117.
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


--------------1.8.2
Content-Type: text/x-patch; name="0001-Syntax-error-in-Git.pm-for-File-Temp-0.23.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Syntax-error-in-Git.pm-for-File-Temp-0.23.patch"

diff --git a/perl/Git.pm b/perl/Git.pm
index 96cac39..cf4f54a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1265,7 +1265,7 @@ sub _temp_cache {
 			$tmpdir = $self->repo_path();
 		}
 
-		($$temp_fd, $fname) = File::Temp->tempfile(
+		($$temp_fd, $fname) = File::Temp::tempfile(
 			'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
 			) or throw Error::Simple("couldn't open new temp file");
 

--------------1.8.2--

Re: Git.pm with recent File::Temp fail

[H.Merijn Brand]

On Sat, 23 Mar 2013 16:32:47 +0000, Ben Walton <bdwalton@gmail.com>
wrote:

> Hello,

Still failing in 1.8.2.1

> On Fri, Mar 22, 2013 at 7:57 PM, H.Merijn Brand <h.m.brand@xs4all.nl> wrote:
> > git-1.8.2, perl-5.16.3, File::Temp-0.23
> >
> > Without patch:
> >
> > $ git svn fetch
> > 'tempfile' can't be called as a method at /pro/lib/perl5/site_perl/5.16.3/Git.pm line 1117.
> >
> > After patch:
> >
> > $ git svn fetch
> >         M       t/06virtual.t
> > r15506 = 6c65be7ff36ffc6fd9b960a4b470ca297103004e (refs/remotes/git-svn)
> > ⋮
> >
> > patch attached
> 
> This list prefers patches to be inline so that they can easily be
> commented on, etc.  I took a quick look at your patch and while it
> looks fine to me, fwiw.
> 
> You may want to resubmit using git send-email though so that others
> will look at it and possibly apply it.

-- 
H.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/
using perl5.00307 .. 5.17   porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/        http://www.test-smoke.org/
http://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Git.pm with recent File::Temp fail

[H.Merijn Brand]

On Sat, 23 Mar 2013 16:32:47 +0000, Ben Walton <bdwalton@gmail.com>
wrote:

> Hello,

Still failing in 1.8.2.2

Short fix:
--8<---
diff --git a/perl/Git.pm b/perl/Git.pm
index dc48159..7a252ef 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1265,7 +1265,7 @@ sub _temp_cache {
                        $tmpdir = $self->repo_path();
                }

-               ($$temp_fd, $fname) = File::Temp->tempfile(
+               ($$temp_fd, $fname) = File::Temp::tempfile(
                        'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
                        ) or throw Error::Simple("couldn't open new temp file");

-->8---

Long fix inlined

> On Fri, Mar 22, 2013 at 7:57 PM, H.Merijn Brand <h.m.brand@xs4all.nl> wrote:
> > git-1.8.2, perl-5.16.3, File::Temp-0.23
> >
> > Without patch:
> >
> > $ git svn fetch
> > 'tempfile' can't be called as a method at /pro/lib/perl5/site_perl/5.16.3/Git.pm line 1117.
> >
> > After patch:
> >
> > $ git svn fetch
> >         M       t/06virtual.t
> > r15506 = 6c65be7ff36ffc6fd9b960a4b470ca297103004e (refs/remotes/git-svn)
> > ⋮
> >
> > patch attached
> 
> This list prefers patches to be inline so that they can easily be
> commented on, etc.  I took a quick look at your patch and while it
> looks fine to me, fwiw.
> 
> You may want to resubmit using git send-email though so that others
> will look at it and possibly apply it.

From b3be713101469f8bea14b854eb7840132ffdca8a Mon Sep 17 00:00:00 2001
From: "H.Merijn Brand - Tux" <h.m.brand@xs4all.nl>
Date: Sun, 28 Apr 2013 11:03:15 +0200
Subject: [PATCH] Git.pm with recent File::Temp fail
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.8.2.1"

This is a multi-part message in MIME format.
--------------1.8.2.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


git-1.8.2{,.1,.2}, perl-5.16.3, File::Temp-0.23

Without patch:

$ git svn fetch
'tempfile' can't be called as a method at /pro/lib/perl5/site_perl/5.16.3/Git.pm line 1117.

After patch:

$ git svn fetch
        M       t/06virtual.t
r15506 = 6c65be7ff36ffc6fd9b960a4b470ca297103004e (refs/remotes/git-svn)
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


--------------1.8.2.1
Content-Type: text/x-patch; name="0001-Git.pm-with-recent-File-Temp-fail.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Git.pm-with-recent-File-Temp-fail.patch"

diff --git a/perl/Git.pm b/perl/Git.pm
index dc48159..7a252ef 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1265,7 +1265,7 @@ sub _temp_cache {
                        $tmpdir = $self->repo_path();
                }

-               ($$temp_fd, $fname) = File::Temp->tempfile(
+               ($$temp_fd, $fname) = File::Temp::tempfile(
                        'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
                        ) or throw Error::Simple("couldn't open new temp file");


--------------1.8.2.1--


-- 
H.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/
using perl5.00307 .. 5.17   porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/        http://www.test-smoke.org/
http://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Git.pm with recent File::Temp fail

[Jeff King]

On Sun, Apr 28, 2013 at 11:09:33AM +0200, H.Merijn Brand wrote:

> Still failing in 1.8.2.2
> 
> Short fix:
> --8<---
> diff --git a/perl/Git.pm b/perl/Git.pm
> index dc48159..7a252ef 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -1265,7 +1265,7 @@ sub _temp_cache {
>                         $tmpdir = $self->repo_path();
>                 }
> 
> -               ($$temp_fd, $fname) = File::Temp->tempfile(
> +               ($$temp_fd, $fname) = File::Temp::tempfile(
>                         'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
>                         ) or throw Error::Simple("couldn't open new temp file");

I think this fix is the right thing. Your patch probably didn't get
applied because it did not follow the guidelines in SubmittingPatches.
Please make sure the subject-line of your email says "[PATCH]".

I.e., this part:

> From b3be713101469f8bea14b854eb7840132ffdca8a Mon Sep 17 00:00:00 2001
> From: "H.Merijn Brand - Tux" <h.m.brand@xs4all.nl>
> Date: Sun, 28 Apr 2013 11:03:15 +0200
> Subject: [PATCH] Git.pm with recent File::Temp fail

Should be your actual email header, not just in the body of the message.

> git-1.8.2{,.1,.2}, perl-5.16.3, File::Temp-0.23
> 
> Without patch:
> 
> $ git svn fetch
> 'tempfile' can't be called as a method at /pro/lib/perl5/site_perl/5.16.3/Git.pm line 1117.
> 
> After patch:
> 
> $ git svn fetch
>         M       t/06virtual.t
> r15506 = 6c65be7ff36ffc6fd9b960a4b470ca297103004e (refs/remotes/git-svn)

I think the relevant thing is that File::Temp 0.23 started complaining
about this. From their Changes file:

  2013-03-14 Tim Jenness <tjenness@cpan.org>

          ---- Release V0.23 CPAN ----

  [...]
        * Temp.pm: Calling tempfile or tempdir as a class method now
          produce a more useful fatal error message

So I think the right commit message is something like:

  We call File::Temp's "tempfile" function as a class method, but it was
  never designed to be called this way. Older versions seemed to
  tolerate it, but as of File::Temp 0.23, it blows up like this:

    $ git svn fetch
    'tempfile' can't be called as a method at .../Git.pm line 1117.

  Fix it by calling it as a regular function, just inside the File::Temp
  namespace.

-Peff

Re: Git.pm with recent File::Temp fail

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sun, Apr 28, 2013 at 11:09:33AM +0200, H.Merijn Brand wrote:
>
>> Still failing in 1.8.2.2
>> 
>> Short fix:
>> --8<---
>> diff --git a/perl/Git.pm b/perl/Git.pm
>> index dc48159..7a252ef 100644
>> --- a/perl/Git.pm
>> +++ b/perl/Git.pm
>> @@ -1265,7 +1265,7 @@ sub _temp_cache {
>>                         $tmpdir = $self->repo_path();
>>                 }
>> 
>> -               ($$temp_fd, $fname) = File::Temp->tempfile(
>> +               ($$temp_fd, $fname) = File::Temp::tempfile(
>>                         'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
>>                         ) or throw Error::Simple("couldn't open new temp file");
>
> I think this fix is the right thing. Your patch probably didn't get
> applied because it did not follow the guidelines in SubmittingPatches.

It was more like "It managed to miss people's attention because the
subject didn't say [PATCH]", but yes you are right that the change
the patch text itself shows looks reasonable and that I cannot apply
a patch that is not signed off (and worse yet in multipart/mixed).

> ...
> So I think the right commit message is something like:
>
>   We call File::Temp's "tempfile" function as a class method, but it was
>   never designed to be called this way. Older versions seemed to
>   tolerate it, but as of File::Temp 0.23, it blows up like this:
>
>     $ git svn fetch
>     'tempfile' can't be called as a method at .../Git.pm line 1117.
>
>   Fix it by calling it as a regular function, just inside the File::Temp
>   namespace.
>
> -Peff

Thanks.

Re: Git.pm with recent File::Temp fail

[H.Merijn Brand]

On Sun, 28 Apr 2013 20:59:16 -0700, Junio C Hamano <gitster@pobox.com>
wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Sun, Apr 28, 2013 at 11:09:33AM +0200, H.Merijn Brand wrote:
> >
> >> Still failing in 1.8.2.2
> >> 
> >> Short fix:
> >> --8<---
> >> diff --git a/perl/Git.pm b/perl/Git.pm
> >> index dc48159..7a252ef 100644
> >> --- a/perl/Git.pm
> >> +++ b/perl/Git.pm
> >> @@ -1265,7 +1265,7 @@ sub _temp_cache {
> >>                         $tmpdir = $self->repo_path();
> >>                 }
> >> 
> >> -               ($$temp_fd, $fname) = File::Temp->tempfile(
> >> +               ($$temp_fd, $fname) = File::Temp::tempfile(
> >>                         'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
> >>                         ) or throw Error::Simple("couldn't open new temp file");
> >
> > I think this fix is the right thing. Your patch probably didn't get
> > applied because it did not follow the guidelines in SubmittingPatches.
> 
> It was more like "It managed to miss people's attention because the
> subject didn't say [PATCH]", but yes you are right that the change
> the patch text itself shows looks reasonable and that I cannot apply
> a patch that is not signed off (and worse yet in multipart/mixed).

My first attempt (when git-1.8.2 was released) had the patch attached
and the reply was

On Sat, 23 Mar 2013 16:32:47 +0000, Ben Walton <bdwalton@gmail.com>
wrote:

> This list prefers patches to be inline so that they can easily be
> commented on, etc.  I took a quick look at your patch and while it
> looks fine to me, fwiw.
> 
> You may want to resubmit using git send-email though so that others
> will look at it and possibly apply it.

So I retried inlining. No mention about *how* to inline or that the
subject should have the word "PATCH".

I don't need any credits. I just want git to work from scratch :)

> > ...
> > So I think the right commit message is something like:
> >
> >   We call File::Temp's "tempfile" function as a class method, but it was
> >   never designed to be called this way. Older versions seemed to
> >   tolerate it, but as of File::Temp 0.23, it blows up like this:
> >
> >     $ git svn fetch
> >     'tempfile' can't be called as a method at .../Git.pm line 1117.
> >
> >   Fix it by calling it as a regular function, just inside the File::Temp
> >   namespace.

Sounds about right.

> > -Peff
> 
> Thanks.

-- 
H.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/
using perl5.00307 .. 5.17   porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/        http://www.test-smoke.org/
http://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Git.pm with recent File::Temp fail

[Junio C Hamano]

"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:

> I don't need any credits. I just want git to work from scratch :)

The sign-off procedure is not about giving credits to you.  It is to
protect us from others (e.g. your employer) by having a record that
you claimed that you had the authority to give us the change.

For a single-liner change like this, this often does not matter in
practice, but we try to be consistent as it makes everybody's life
simpler to have a single procedure for all the patches.

Thanks

>
>> > ...
>> > So I think the right commit message is something like:
>> >
>> >   We call File::Temp's "tempfile" function as a class method, but it was
>> >   never designed to be called this way. Older versions seemed to
>> >   tolerate it, but as of File::Temp 0.23, it blows up like this:
>> >
>> >     $ git svn fetch
>> >     'tempfile' can't be called as a method at .../Git.pm line 1117.
>> >
>> >   Fix it by calling it as a regular function, just inside the File::Temp
>> >   namespace.
>
> Sounds about right.
>
>> > -Peff
>> 
>> Thanks.

Re: Git.pm with recent File::Temp fail

[Junio C Hamano]

Here is what I'll queue.

Thanks for the help, all of you.

-- >8 --
From: H. Merijn Brand <h.m.brand@xs4all.nl>
Subject: Git.pm: call tempfile from File::Temp as a regular function

We call File::Temp's "tempfile" function as a class method, but it was
never designed to be called this way. Older versions seemed to
tolerate it, but as of File::Temp 0.23, it blows up like this:

  $ git svn fetch
  'tempfile' can't be called as a method at .../Git.pm line 1117.

Fix it by calling it as a regular function, just inside the File::Temp
namespace.

Signed-off-by: H. Merijn Brand <h.m.brand@xs4all.nl>
Helped-by: Jeff King <peff@peff.net>
---
 perl/Git.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 497f420..76383b9 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1039,7 +1039,7 @@ sub _temp_cache {
 			$tmpdir = $self->repo_path();
 		}
 
-		($$temp_fd, $fname) = File::Temp->tempfile(
+		($$temp_fd, $fname) = File::Temp::tempfile(
 			'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
 			) or throw Error::Simple("couldn't open new temp file");
 

Re: Git.pm with recent File::Temp fail

[H.Merijn Brand]

On Mon, 29 Apr 2013 01:18:18 -0700, Junio C Hamano <gitster@pobox.com>
wrote:

> Here is what I'll queue.
> 
> Thanks for the help, all of you.

Thank you. I will keep this conversation as a guide for possible next
patches

> -- >8 --
> From: H. Merijn Brand <h.m.brand@xs4all.nl>
> Subject: Git.pm: call tempfile from File::Temp as a regular function
> 
> We call File::Temp's "tempfile" function as a class method, but it was
> never designed to be called this way. Older versions seemed to
> tolerate it, but as of File::Temp 0.23, it blows up like this:
> 
>   $ git svn fetch
>   'tempfile' can't be called as a method at .../Git.pm line 1117.
> 
> Fix it by calling it as a regular function, just inside the File::Temp
> namespace.
> 
> Signed-off-by: H. Merijn Brand <h.m.brand@xs4all.nl>
> Helped-by: Jeff King <peff@peff.net>
> ---
>  perl/Git.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/perl/Git.pm b/perl/Git.pm
> index 497f420..76383b9 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -1039,7 +1039,7 @@ sub _temp_cache {
>  			$tmpdir = $self->repo_path();
>  		}
>  
> -		($$temp_fd, $fname) = File::Temp->tempfile(
> +		($$temp_fd, $fname) = File::Temp::tempfile(
>  			'Git_XXXXXX', UNLINK => 1, DIR => $tmpdir,
>  			) or throw Error::Simple("couldn't open new temp file");
>  


-- 
H.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/
using perl5.00307 .. 5.17   porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/        http://www.test-smoke.org/
http://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/