Re: [PATCH] Remove dependency on IO::String from Git.pm test

Previous thread: Getting the path right for git over SSH by Ben Schmidt on Wednesday, June 18, 2008 - 4:22 am. (1 message)

Next thread: sharing object packs by marc.zonzon+git on Wednesday, June 18, 2008 - 12:57 pm. (3 messages)
From: Michael Hendricks
Date: Wednesday, June 18, 2008 - 6:37 am

Instead of using IO::String to create an in-memory filehandle, use
open() with a scalar reference as the filename.  This feature has been
available since Perl 5.8.0 (which was released in 2002), so it should
be available pretty much everywhere by now.

Signed-off-by: Michael Hendricks <michael@ndrix.org>
---

This patch should apply on top of Junio's lw/perlish branch.

 t/t9700/test.pl |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 8318fec..e34c01e 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -9,7 +9,6 @@ use Test::More qw(no_plan);
 use Cwd;
 use File::Basename;
 use File::Temp;
-use IO::String;
 
 BEGIN { use_ok('Git') }
 
@@ -69,20 +68,21 @@ is($r->ident_person("Name", "email", "123 +0000"), "Name <email>",
 
 # objects and hashes
 ok(our $file1hash = $r->command_oneline('rev-parse', "HEAD:file1"), "(get file hash)");
-our $iostring = IO::String->new;
+my $output;
+open our $iostring, '>', \$output;
 is($r->cat_blob($file1hash, $iostring), 15, "cat_blob: size");
-is(${$iostring->string_ref}, "changed file 1\n", "cat_blob: data");
+is($output, "changed file 1\n", "cat_blob: data");
 our $tmpfile = File::Temp->new();
-print $tmpfile ${$iostring->string_ref};
+print $tmpfile $output;
 is(Git::hash_object("blob", $tmpfile), $file1hash, "hash_object: roundtrip");
 $tmpfile = File::Temp->new();
 print $tmpfile my $test_text = "test blob, to be inserted\n";
 $tmpfile->close;
 like(our $newhash = $r->hash_and_insert_object($tmpfile), qr/[0-9a-fA-F]{40}/,
      "hash_and_insert_object: returns hash");
-$iostring = IO::String->new;
+open $iostring, '>', \$output;
 is($r->cat_blob($newhash, $iostring), length $test_text, "cat_blob: roundtrip size");
-is(${$iostring->string_ref}, $test_text, "cat_blob: roundtrip data");
+is($output, $test_text, "cat_blob: roundtrip data");
 
 # paths
 is($r->repo_path, "./.git", "repo_path");
-- 
1.5.5.23.g2a5fe

--

From: Jakub Narebski
Date: Wednesday, June 18, 2008 - 7:46 am

Besides if I understand correctly gitweb very much requires Perl >= 5.8
because of required Unicode support.

Nevertheless adding "use v5.8.0;" or "use 5.008_000;" would be I guess
good idea.


And best solution, although perhaps unnecessary, would be to check
for version >= 5.8, if older check for IO::String, and even if that
fails, simply skip those tests that require in-memory filehandle
(or use tempfile).

-- 
Jakub Narebski
Poland
ShadeHawk on #git
--

From: Rafael Garcia-Suarez
Date: Wednesday, June 18, 2008 - 8:10 am

"use 5.008;" is preferred form; "use v5.8.0" might yield obscure error
messages on perls < 5.6, which is not the desired result.
--

From: Johannes Schindelin
Date: Wednesday, June 18, 2008 - 10:00 am

Hi,


Did I miss something?  Was this patch not more about Git.pm?

BTW I think it is not nice at all how the dependency hell with Git.pm is 
made worse recently.

It is fascinating through how much _pain_ we go with the shell scripts to 
maintain portability, even with _very_ old or obscure systems (see the SCO 
server patches that came in not long ago!), and just walk over that 
portability when it comes to Perl...

Ciao,
Dscho

--

From: Jakub Narebski
Date: Wednesday, June 18, 2008 - 10:52 am

Oops... You are right, my mistake.

For my defense I'd like to point out that the patch this patch is
response to was made by gitweb caching project GSoC student, Lea Wiemann


And I pointed out how it could be resolved (use 5.8 specific feature,
or IO::String, or skip tests).

-- 
Jakub Narebski
Poland
--

From: Johannes Schindelin
Date: Wednesday, June 18, 2008 - 12:35 pm

Hi,


So?  Why do you want to break the _test_ on those machines where you need 

I have to point out that the platforms I was speaking of are not know to 
make upgrading as easy as Linux.  And some of them _do_ come with pretty 
old perl.  And yes, I had this exact issue (remember when I worked on 
removing Git's dependency on Python?  That was it.  Not enough quota.  
Uncooperative admin.  Desperate need for a sensible SCM).

In any case, I have to reiterate my point: breaking 
backwards-compatibility for _no_ good reason is wrong.  I have not looked 
at the patch in question, but I seriously doubt that this is the easiest, 
most elegant (and yes, backwards-compatible) solution.

Typically, it is not a good sign when you _require_ newer and newer 
features and versions of components you use.

Whatever,
Dscho

--

From: Lea Wiemann
Date: Wednesday, June 18, 2008 - 12:42 pm

Goodness.  Everyone in this thread, please relax a notch.  I thought we 
had pretty clear agreement that we want to keep Perl 5.6 compatibility 
for Git.pm.  The solution is to use a temporary file in the current 
directory (and unlink it afterwards).  If nobody has done that by 
tomorrow I'll do it.  It'll take that less time than to even read this 
thread.

Now lets get back to work.

-- Lea
--

From: Junio C Hamano
Date: Wednesday, June 18, 2008 - 12:13 pm

Hey, calm down a bit and give me a bit more credit.  I am not that stupid.
I've looked at the patch, and it is parked in 'pu', not near 'next'.
--

From: Johannes Schindelin
Date: Wednesday, June 18, 2008 - 12:37 pm

Hi,


Heh.  The part about the shell scripts was actually meant as a genuine 
praise.  And I did not mean to criticize _you_ for introducing IO::Stream.

Ciao,
Dscho

--

From: Lea Wiemann
Date: Thursday, June 19, 2008 - 11:25 am

I've now sent a new version of my script that uses File::Temp and only 
depends on Perl 5.6.2.  Thanks for making the start!

Jakub Narebski wrote:
 > [...] Lea Wiemann (who should have been CC-ed, by the way).

For the record, CC'ing me or not doesn't make a difference if it's 
posted to the list (I don't actually notice whether I'm CC'ed).

-- Lea
--

Previous thread: Getting the path right for git over SSH by Ben Schmidt on Wednesday, June 18, 2008 - 4:22 am. (1 message)

Next thread: sharing object packs by marc.zonzon+git on Wednesday, June 18, 2008 - 12:57 pm. (3 messages)