PyPycLoader can't read or write .pyc files created by the core import machinery. I'm attaching a failing test case demonstrating the issue: if you import a .py file using standard mechanisms, thus creating a .pyc, and then (in a separate process, say) attempt to make use of that cached file with PyPycLoader, the import fails with 'ValueError: bad marshal data (unknown type code)'.

It looks like that there has been a change in the binary format of .pyc files but PyPycLoader wasn't updated.

Ronan, can you confirm if 2.7 or 3.2 are affected too?

2.7 doesn't have PyPycLoader. For 3.2, it's such a pain to create a working concrete subclass of PyPycLoader that I gave up. But just by reading the code, it doesn't seem affected, as there's no discrepancy with importlib._bootstrap: they both consider that metadata are the first 8 bytes (in default, _bootstrap switched to using 12 bytes).

Could you write a patch for 3.3?

My preferred solution would be to use to use the functions I describe in issue 15031.

The format did change and importlib.abc.PyPycLoader was not subsequently updated. Problem is that the ABC has been deprecated and given backwards-compatibility instructions for Python 3.1, so I don't know if it should be considered a priority to fix this to read .pyc files created by other loaders instead of just the .pyc files it creates on its own when it implements write_bytecode().

Well, working on a deprecated ABC is certainly low-priority, but it seems rather bad to have 2 incompatible implementations of the .pyc format in the stdlib. If this is to stay like this, it should at least come with a strong warning that users of PyPycLoader must invent their own PEP 3147 magic tag, which opens the can of worms of supporting multiple .pyc formats within the same interpreter in the stdlib. 

It seems easier to just fix it to be compatible with the one blessed .pyc format, as was the case in 3.2.

Yes, a fix would be easiest, but someone has to write the fix. =) I will try to get to it before the first beta (unless Antoine, who made the .pyc change, wants to do it =).

Attaching a patch...

Using Ronan's test_PyPyc.diff, before my patch:

{{{
~/dev/hg-repos/cpython$ ./python.exe -m unittest Lib/importlib/test/source/test_abc_loader.py 
...........................................E..................
======================================================================
ERROR: test_pyc_compatibility (Lib.importlib.test.source.test_abc_loader.RegeneratedBytecodeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./Lib/importlib/test/source/util.py", line 23, in wrapper
    to_return = fxn(*args, **kwargs)
  File "./Lib/importlib/test/source/test_abc_loader.py", line 473, in test_pyc_compatibility
    mock.load_module(name)
  File "<frozen importlib._bootstrap>", line 760, in load_module
  File "<frozen importlib._bootstrap>", line 408, in module_for_loader_wrapper
  File "<frozen importlib._bootstrap>", line 636, in _load_module
  File "./Lib/importlib/test/source/test_abc_loader.py", line 201, in get_code
    code_object = super().get_code(name)
  File "/Users/marca/dev/hg-repos/cpython/Lib/importlib/abc.py", line 305, in get_code
    return marshal.loads(bytecode)
ValueError: bad marshal data (unknown type code)

----------------------------------------------------------------------
Ran 62 tests in 0.076s

FAILED (errors=1)
[114118 refs]
}}}

After my patch:

{{{
~/dev/hg-repos/cpython$ patch -p1 < ~/Desktop/cpython-issue-15030.patch 
patching file Lib/importlib/abc.py
~/dev/hg-repos/cpython$ ./python.exe -m unittest Lib/importlib/test/source/test_abc_loader.py 
..............................................................
----------------------------------------------------------------------
Ran 62 tests in 0.079s

OK
[114118 refs]
}}}

Patch looks innocent enough; would be nice to fix this for 3.3.

Similar issue in distribute: https://bitbucket.org/tarek/distribute/issue/283/bdist_egg-issues-with-python-330ax

Updated patch based on feedback from Brett (thanks!)

Third revision of my patch based on additional feedback from Brett (thanks!)...

Patch looks good, Marc. Can you sign a PSF contributor agreement and send it in (http://www.python.org/psf/contrib/)?

Brett, I just emailed the contributor agreement.

I think the loader should just unconditionally assume 12 bytes header. This is meant to work only for the exact version it ships with, so it just has to match.

> I think the loader should just unconditionally assume 12 bytes header. 
> This is meant to work only for the exact version it ships with, so it
> just has to match.

Agreed with Martin.

Hmmm if I simply do:

diff -r b66e82c9f852 Lib/importlib/abc.py
--- a/Lib/importlib/abc.py	Tue Jun 26 23:05:27 2012 +0200
+++ b/Lib/importlib/abc.py	Wed Jun 27 12:15:55 2012 -0700
@@ -282,7 +282,7 @@
                 if len(raw_timestamp) < 4:
                     raise EOFError("bad timestamp in {}".format(fullname))
                 pyc_timestamp = _bootstrap._r_long(raw_timestamp)
-                bytecode = data[8:]
+                bytecode = data[12:]
                 # Verify that the magic number is valid.
                 if imp.get_magic() != magic:
                     raise ImportError(

then I get two "ValueError: bad marshal data (unknown type code)" test errors in test_abc_loader.

I am unsure as to whether this is a bug in the test or the implementation.

The following quells the errors, but I am not all confident that it's correct:

@@ -302,7 +302,10 @@
                     raise
             else:
                 # Bytecode seems fine, so try to use it.
-                return marshal.loads(bytecode)
+                try:
+                    return marshal.loads(bytecode)
+                except ValueError:
+                    return ''
         elif source_timestamp is None:
             raise ImportError("no source or bytecode available to create code "
                               "object for {0!r}".format(fullname),

> I am unsure as to whether this is a bug in the test or the implementation.

You should probably investigate a bit more. We don't want to silence exceptions if there's not a good reason to.

The patch could be updated to ignore the 12 bytes unconditionally when reading bytecode but still write out the accurate file size (when possible). That should be fully compatible within Python 3.3 no matter who generated the bytecode (PyPycLoader or import).

Here's a patch that unconditionally switches over to the 12 byte format. I'm assuming the "size" in data[8:12] is the length of the bytecode?

Oops, last attachment included the source timestamp twice instead of timestamp + bytecode size.

Oops. Refactor. :-)

I don't know if I'll have time soon to do the tweaks to Ronan's test (and maybe he wants to do them himself anyway?), but here's the correction of the size calculation in the header (from size of bytecode to size of source -- thanks, Brett!)

Changeset b7463ec1980c has the fix and test update (went with a simple solution for the tests which uses the mock to verify). Thanks to Ronan and Marc for helping out!

Hi Brett, can I get an ack in Misc/ACKS please (to make my Mom proud :-))? Attaching patch.

Marc, can you please submit a contributor form?

Hi Martin,

I already did. See http://bugs.python.org/msg164162 and http://bugs.python.org/msg164164. Maybe it's not "on file" yet?

Maybe you can try emailing in the form again, Marc? Let me know when you have and if it is lost again I will bug the proper people.

Anyway, the original patch I committed added you to Misc/ACKS: http://hg.python.org/cpython/file/56260d30985d/Misc/ACKS#l16