Patrick Williams | c124f4f | 2015-09-15 14:41:29 -0500 | [diff] [blame] | 1 | From 55d430eb02fc116581847304ca20321687978269 Mon Sep 17 00:00:00 2001 |
| 2 | From: Jonathan Nieder <jrnieder@gmail.com> |
| 3 | Date: Fri, 27 Jul 2012 10:35:07 -0500 |
| 4 | Subject: Memoize::Storable: respect 'nstore' option not respected |
| 5 | MIME-Version: 1.0 |
| 6 | Content-Type: text/plain; charset=UTF-8 |
| 7 | Content-Transfer-Encoding: 8bit |
| 8 | |
| 9 | Memoize(3perl) says: |
| 10 | |
| 11 | tie my %cache => 'Memoize::Storable', $filename, 'nstore'; |
| 12 | memoize 'function', SCALAR_CACHE => [HASH => \%cache]; |
| 13 | |
| 14 | Include the ‘nstore’ option to have the "Storable" database |
| 15 | written in ‘network order’. (See Storable for more details |
| 16 | about this.) |
| 17 | |
| 18 | In fact the "nstore" option does no such thing. Option parsing looks |
| 19 | like this: |
| 20 | |
| 21 | @options{@_} = (); |
| 22 | |
| 23 | $self->{OPTIONS}{'nstore'} is accordingly set to undef. Later |
| 24 | Memoize::Storable checks if the option is true, and since undef is |
| 25 | not true, the "else" branch is always taken. |
| 26 | |
| 27 | if ($self->{OPTIONS}{'nstore'}) { |
| 28 | Storable::nstore($self->{H}, $self->{FILENAME}); |
| 29 | } else { |
| 30 | Storable::store($self->{H}, $self->{FILENAME}); |
| 31 | } |
| 32 | |
| 33 | Correcting the condition to (exists $self->{OPTIONS}{'nstore'}) fixes |
| 34 | it. |
| 35 | |
| 36 | Noticed because git-svn, which uses the 'nstore' option for its |
| 37 | on-disk caches, was producing |
| 38 | |
| 39 | Byte order is not compatible at ../../lib/Storable.pm |
| 40 | |
| 41 | when run using a perl with a different integer size (and hence |
| 42 | byteorder). |
| 43 | |
| 44 | Reported by Tim Retout (RT#77790) |
| 45 | |
| 46 | Bug-Debian: http://bugs.debian.org/587650 |
| 47 | Bug: https://rt.cpan.org/Public/Bug/Display.html?id=77790 |
| 48 | Forwarded: https://rt.cpan.org/Public/Bug/Display.html?id=77790 |
| 49 | Patch-Name: fixes/memoize_storable_nstore.diff |
| 50 | --- |
| 51 | cpan/Memoize/Memoize/Storable.pm | 2 +- |
| 52 | cpan/Memoize/t/tie_storable.t | 24 ++++++++++++++++++++---- |
| 53 | 2 files changed, 21 insertions(+), 5 deletions(-) |
| 54 | |
| 55 | diff --git a/cpan/Memoize/Memoize/Storable.pm b/cpan/Memoize/Memoize/Storable.pm |
| 56 | index 1314797..87876f2 100644 |
| 57 | --- a/cpan/Memoize/Memoize/Storable.pm |
| 58 | +++ b/cpan/Memoize/Memoize/Storable.pm |
| 59 | @@ -55,7 +55,7 @@ sub DESTROY { |
| 60 | require Carp if $Verbose; |
| 61 | my $self= shift; |
| 62 | print STDERR "Memoize::Storable::DESTROY(@_)\n" if $Verbose; |
| 63 | - if ($self->{OPTIONS}{'nstore'}) { |
| 64 | + if (exists $self->{OPTIONS}{'nstore'}) { |
| 65 | Storable::nstore($self->{H}, $self->{FILENAME}); |
| 66 | } else { |
| 67 | Storable::store($self->{H}, $self->{FILENAME}); |
| 68 | diff --git a/cpan/Memoize/t/tie_storable.t b/cpan/Memoize/t/tie_storable.t |
| 69 | index de3b8dc..a624238 100644 |
| 70 | --- a/cpan/Memoize/t/tie_storable.t |
| 71 | +++ b/cpan/Memoize/t/tie_storable.t |
| 72 | @@ -31,18 +31,34 @@ if ($@) { |
| 73 | exit 0; |
| 74 | } |
| 75 | |
| 76 | -print "1..4\n"; |
| 77 | +print "1..9\n"; |
| 78 | |
| 79 | $file = "storable$$"; |
| 80 | 1 while unlink $file; |
| 81 | tryout('Memoize::Storable', $file, 1); # Test 1..4 |
| 82 | 1 while unlink $file; |
| 83 | +tryout('Memoize::Storable', $file, 5, 'nstore'); # Test 5..8 |
| 84 | +assert_netorder($file, 9); # Test 9 |
| 85 | +1 while unlink $file; |
| 86 | + |
| 87 | + |
| 88 | +sub assert_netorder { |
| 89 | + my ($file, $testno) = @_; |
| 90 | + |
| 91 | + my $netorder = Storable::file_magic($file)->{'netorder'}; |
| 92 | + print ($netorder ? "ok $testno\n" : "not ok $testno\n"); |
| 93 | +} |
| 94 | |
| 95 | sub tryout { |
| 96 | - my ($tiepack, $file, $testno) = @_; |
| 97 | + my ($tiepack, $file, $testno, $option) = @_; |
| 98 | |
| 99 | - tie my %cache => $tiepack, $file |
| 100 | - or die $!; |
| 101 | + if (defined $option) { |
| 102 | + tie my %cache => $tiepack, $file, $option |
| 103 | + or die $!; |
| 104 | + } else { |
| 105 | + tie my %cache => $tiepack, $file |
| 106 | + or die $!; |
| 107 | + } |
| 108 | |
| 109 | memoize 'c5', |
| 110 | SCALAR_CACHE => [HASH => \%cache], |