threads / patch / 14014

patchRemove dependency on IO::String from Git.pm test

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

## tl;dr

10 messages between Jun 18, 2008 and Jun 19, 2008. Diffs are folded; open one to read it.

replies: 9people: 6as markdown or json

Michael Hendricks· Jun 18, 2008, 13:37 UTC · lore

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(-)
Show changes to t/t9700/test.pl +6 −6
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
Jakub Narebski· Jun 18, 2008, 14:46 UTC · re: Michael Hendricks · lore

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

Michael Hendricks <michael@ndrix.org> writes:
> 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.

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
Rafael Garcia-Suarez· Jun 18, 2008, 15:10 UTC · re: Jakub Narebski · lore

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

2008/6/18 Jakub Narebski <jnareb@gmail.com>:
Show 12 quoted lines
> Michael Hendricks <michael@ndrix.org> writes:
>
>> 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.
>
> 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.

"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.

Johannes Schindelin· Jun 18, 2008, 17:00 UTC · re: Jakub Narebski · lore

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

Hi,
On Wed, 18 Jun 2008, Jakub Narebski wrote:
Show 9 quoted lines
> Michael Hendricks <michael@ndrix.org> writes:
> 
> > 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.
> 
> Besides if I understand correctly gitweb very much requires Perl >= 5.8 
> because of required Unicode support.
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

Jakub Narebski· Jun 18, 2008, 17:52 UTC · re: Johannes Schindelin · lore

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

On Wed, 18 Jun 2008, Johannes Schindelin wrote:
Show 12 quoted lines
> On Wed, 18 Jun 2008, Jakub Narebski wrote:
> > Michael Hendricks <michael@ndrix.org> writes:
> > 
> > > 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.
> > 
> > Besides if I understand correctly gitweb very much requires Perl >= 5.8 
> > because of required Unicode support.
> 
> Did I miss something?  Was this patch not more about Git.pm?
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
(who should have been CC-ed, by the way).
 
> BTW I think it is not nice at all how the dependency hell with Git.pm is 
> made worse recently.
It is not dependency for Git.pm, but for Git.pm TEST.
> 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...

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

-- 
Jakub Narebski
Poland
Johannes Schindelin· Jun 18, 2008, 19:35 UTC · re: Jakub Narebski · lore

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

Hi,
On Wed, 18 Jun 2008, Jakub Narebski wrote:
Show 6 quoted lines
> On Wed, 18 Jun 2008, Johannes Schindelin wrote:
>
> > BTW I think it is not nice at all how the dependency hell with Git.pm 
> > is made worse recently.
> 
> It is not dependency for Git.pm, but for Git.pm TEST.

So? Why do you want to break the _test_ on those machines where you need them most? We _know_ that a release from Junio works fine on Linux.

Show 7 quoted lines
> > 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...
> 
> And I pointed out how it could be resolved (use 5.8 specific feature, or 
> IO::String, or skip tests).

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

Lea Wiemann· Jun 18, 2008, 19:42 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin wrote:
> So?  Why do you want to break the _test_ on those machines where you need 
> them most?  We _know_ that a release from Junio works fine on Linux.

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
Junio C Hamano· Jun 18, 2008, 19:13 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 23 quoted lines
> Hi,
>
> On Wed, 18 Jun 2008, Jakub Narebski wrote:
>
>> Michael Hendricks <michael@ndrix.org> writes:
>> 
>> > 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.
>> 
>> Besides if I understand correctly gitweb very much requires Perl >= 5.8 
>> because of required Unicode support.
>
> 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...

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'.

Johannes Schindelin· Jun 18, 2008, 19:37 UTC · re: Junio C Hamano · lore

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

Hi,
On Wed, 18 Jun 2008, Junio C Hamano wrote:
Show 10 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 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...
> 
> 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'.

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

Lea Wiemann· Jun 19, 2008, 18:25 UTC · re: Michael Hendricks · lore

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

Michael Hendricks wrote:
> Instead of using IO::String to create an in-memory filehandle, use
> open() with a scalar reference as the filename.

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

← back to recent threads