Storable format breaks pmc2c when Perl is upgraded

[allison]

Downstream in Debian, the upgrade from Perl 5.10 to 5.12 broke Rakudo compiles, because the installed PMC .dump files were written using the 5.10 version of Storable, and could not be read using the 5.12 version of Storable. The upstream bugs (closed) related to this failure are:

 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634105

 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634970

This is solved in the short-term, but in the longer-term, Storable is not a good format for installed, persistent metadata. (It was never intended to be installed, only to be a temporary build-process cache.)

We need to re-examine what metadata pmc2c actually needs for building PMCs that inherit from core Parrot PMCs. I suspect that it's actually much smaller than the current .dump files, and that installing some simple JSON metadata files for the core PMCs would actually be sufficient.

The work on M0 will eventually make this irrelevant, but that work would likely also benefit from some thought about abstracting the metadata needed to inherit from a PMC.

[jkeenan]

For reference, here are the places in the codebase where we will have to consider replacing use of Storable.

lib/Parrot/Pmc2c/PMC.pm:26:use Storable ();
lib/Parrot/Pmc2c/PMC.pm:74:    Storable::store( $self, $self->filename('.dump') );
lib/Parrot/Pmc2c/Pmc2cMain.pm:10:use Storable;
lib/Parrot/Pmc2c/Pmc2cMain.pm:142:
  consists of a binary dump of a hash reference, Storable-style.
lib/Parrot/Pmc2c/Pmc2cMain.pm:165:    return Storable::retrieve($filename);
lib/Parrot/Pmc2c/VTable.pm:6:use Storable ();
lib/Parrot/Pmc2c/VTable.pm:56:    Storable::store( $self, $dump_filename );

[jkeenan]

94 .dump files in Storable format which get installed

[nick@…]

On Sat, Jul 30, 2011 at 08:46:55AM -0000, Parrot wrote:

>  Downstream in Debian, the upgrade from Perl 5.10 to 5.12 broke Rakudo
>  compiles, because the installed PMC .dump files were written using the
>  5.10 version of Storable, and could not be read using the 5.12 version of
>  Storable. The upstream bugs (closed) related to this failure are:

I suspect that the problem is that they were written with Storable::store,
which is platform and configuration specific, and that when upgrading from
5.10 to 5.12 Debian changed their perl configuration from 32 to 64 bit
integers.

Storable itself is quite capable of reading files made all the way back to
0.60-something, for the same configuration.

Had parrot used Storable::nstore (the slower but portable format) there
would not have been this problem.

>  This is solved in the short-term, but in the longer-term, Storable is not
>  a good format for installed, persistent metadata. (It was never intended
>  to be installed, only to be a temporary build-process cache.)

However, that's key. As the intent was not to install the files, then
the faster "native" format was a sane choice.

And I agree with that (any) Storable format isn't an appropriate choice
for something installed. It's not a format that parrot itself can read.

Nicholas Clark

[jkeenan]

Replying to nick@…:

> I suspect that the problem is that they were written with Storable::store,
> which is platform and configuration specific, and that when upgrading from
> 5.10 to 5.12 Debian changed their perl configuration from 32 to 64 bit
> integers.
> 
> Storable itself is quite capable of reading files made all the way back to
> 0.60-something, for the same configuration.
> 
> Had parrot used Storable::nstore (the slower but portable format) there
> would not have been this problem.

I agree. For this reason we use Storable::nstore() in three other places in Parrot -- albeit all less mission-critical places in the configuration system:

lib/Parrot/Configure/Trace.pm
lib/Parrot/Configure.pm
lib/Parrot/Test/Pod.pm

I have created a tt2167/nstore branch in git which replaces store with nstore in lib/Parrot/Pmc2c/PMC.pm and lib/Parrot/Pmc2c/VTable.pm. Under an ordinary, non-optimized build on linux/i386, it passes make test  Smolder 19496. Although I agree that we should replace Storable as soon as possible, please give the branch a spin on the systems where we think we are having a problem.

Thank you very much.

kid51

[jkeenan]

I have merged the tt2167/nstore branch into master in c70c89c, so we're now using nstore instead of store throughout. This improves things but does not address the long-term problem.

Thank you very much.

kid51