msg165464 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2012-07-14 19:41 |
Today I was hacking on a patch that changes the marshaling format for Code objects. When building the patch I was met with: ValueError: bad marshal data (unknown type code) make: *** [Lib/_sysconfigdata.py] Abort trap: 6 This is because the frozen importlib was not rebuilt with the new marshaling format. |
|
|
msg165465 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2012-07-14 19:44 |
Patch attached. |
|
|
msg165468 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-07-14 20:11 |
Looks good to me. It probably won't cover all cases (such as e.g. changing the bytecode format), but it's a good step forward. |
|
|
msg165501 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2012-07-15 03:48 |
FYI: the .pyc magic number is now built/kept in Lib/importlib/_bootstrap.py, so updates to it will trigger a rebuild of frozen importlib during make. |
|
|
msg165503 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2012-07-15 03:56 |
What I mean is, shouldn't changes to marshal have an accompanying bump to the pyc magic number? |
|
|
msg165544 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2012-07-15 18:55 |
Eric, that is a good point, but if someone forgets (like I did) or just hasn't gotten around to bumping the number yet, then the build breaks because the interpreter crashes. I think we should always try to avoid building an interpreter that is in an inconsistent state. Anyway, I am hitting another problem now -- _freeze_importlib is *not* idempotent. Thus adding this rule might cause some noise in the builds because importlib.h is different when nothing has actually changed. I am still investigating that problem. Assuming I can fix the idempotency problem, then maybe _freeze_importlib should just be always run. |
|
|
msg165571 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2012-07-16 05:42 |
Hmmm, I guess the idempotency issue is no worse than it already is -- the same thing can still happen with trivial changes to the other prerequisites for importlib.h. Consider this small example (you might have to run sample program multiple times to see a difference): $ cat dis-closure.py import dis def adder(a, b): def add(): return a + b return add print(dis.dis(adder(1, 2).__code__)) $ ./python.exe dis-closure.py 5 0 LOAD_DEREF 0 (a) 3 LOAD_DEREF 1 (b) 6 BINARY_ADD 7 RETURN_VALUE None $ ./python.exe dis-closure.py 5 0 LOAD_DEREF 1 (a) 3 LOAD_DEREF 0 (b) 6 BINARY_ADD 7 RETURN_VALUE None The order of 'co_cellvars', 'co_varnames', and 'co_freevars' can be different from compile to compile, thus the bytecode can be different from compile to compile (I am not sure if this is worth fixing). Thus there may be times where importlib.h is regenerated, but the changes in the bytecode aren't significant. I will just commit this patch as is. |
|
|
msg165583 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-07-16 10:20 |
> Anyway, I am hitting another problem now -- _freeze_importlib is *not* idempotent. What do you mean with "idempotent"? Deterministic? |
|
|
msg165592 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-07-16 12:13 |
Le lundi 16 juillet 2012 à 05:42 +0000, Meador Inge a écrit : > The order of 'co_cellvars', 'co_varnames', and 'co_freevars' can be > different from compile to compile, thus the bytecode can be different > from compile to compile (I am not sure if this is worth fixing). I don't know, but you could open an issue just for the record. People may be surprised if bytecode generation isn't deterministic. |
|
|
msg165595 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2012-07-16 12:42 |
On Mon, Jul 16, 2012 at 5:20 AM, Antoine Pitrou <report@bugs.python.org> wrote: >> Anyway, I am hitting another problem now -- _freeze_importlib is *not* idempotent. > > What do you mean with "idempotent"? Deterministic? Whoops, yeah, deterministic. I got mixed up with my terminology. |
|
|
msg165597 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2012-07-16 12:54 |
On Mon, Jul 16, 2012 at 7:13 AM, Antoine Pitrou <report@bugs.python.org> wrote: > I don't know, but you could open an issue just for the record. > People may be surprised if bytecode generation isn't deterministic. Yeah, I was somewhat surprised and it took some digging to figure out exactly what was changing. Opened to track this. |
|
|
msg167395 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2012-08-04 03:19 |
Meador, is this still a problem? There was a flurry of activity in this area. |
|
|
msg167538 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2012-08-06 05:02 |
Yup, it is still an issue. The recent activity was mostly related to Windows builds (). |
|
|
msg168973 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2012-08-24 02:50 |
FWIW, the patch looks good to me. This is probably the last week to get this in for 3.3.0. |
|
|
msg274949 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-09-08 01:50 |
New changeset 344f44bd793f by Eric Snow in branch 'default': Issue #15352: Rebuild frozen modules when marshal.c is changed. https://hg.python.org/cpython/rev/344f44bd793f |
|
|
msg274950 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2016-09-08 01:51 |
Thanks for the patch Meador. |
|
|
msg275145 - (view) |
Author: Meador Inge (meador.inge) *  |
Date: 2016-09-08 20:42 |
Hmmm, not sure why I forgot to apply this myself. Thanks for committing it Eric. |
|
|