The magic number change in 3.5.3 broke the world when Fedora attempted to rebase from 3.5.2, but upstream CPython developers don't currently get an automated notification that it isn't permitted to change the magic number in a maintenance release - instead, the current approach relies on people "just knowing" not to do that, and we just saw the limitations of relying on the code review process to catch this particular problem.


I think we can keep this from happening again by having an importlib test case that fails if sys.version_info.releaselevel is "candidate" or "final" and either:

- no "expected magic number" is defined; or
- the actual magic number in importlib.util.MAGIC_NUMBER doesn't match the expected one

The comments on the new test case would then explain the policy and why it isn't OK to break pyc file compatibility in a maintenance release.

We'd also propose an update to the release PEP warning release managers to expect this error as part of prepping the first release candidate, and that they should lock in the magic number for the release at that time.

For the record, the magic number was changed in issue27286

Why shouldn't the magic number be bumped when a bug in the bytecode is fixed in a bugfix release? How did this break Fedora? Are you not shipping source code but only .pyc files?

I view the whole point of having the magic number is to cover things like this, otherwise we might as well just embed the CPython version number in the file name and be done with it.

There are a couple of pieces to it:

- first, there's the problem of breaking anyone that actually is doing pyc-only distribution, even though nothing in Fedora itself does that. While that aspect doesn't break Fedora per se, it does break Fedora's compatibility expectations for end user components, which maintenance updates shouldn't do

- however, more importantly in this case, ordinary users don't have write access to the system site-packages directory, so the pyc files have to be pre-compiled when generating the system RPMs.

While CPython falls back to the slow path of recompiling from source when those precompiled files are invalidated so nothing obviously breaks, it still generates a lot of SELinux warnings related to unauthorised write attempts to system-owned directories: https://bodhi.fedoraproject.org/updates/python3-3.5.3-1.fc25%20python3-docs-3.5.3-1.fc25#comment-559458

(Debian uses a similar pre-compilation trick for the same reasons, so I expect you'd also see comparable warnings under AppArmor if packages weren't rebuilt after a magic number change)

This all means that CPython changing the magic number in a maintenance release ends up being akin to breaking the C ABI in a maintenance release - while it isn't quite as dire (since there's still the slow path fallback to recompiling from source), it does still invalidate all of the pyc files in the system RPMs until they're rebuilt against the new version. "Requires a mass rebuild of all the Python packages in the distro to get things working properly again" is again a major concern for what's nominally a low impact maintenance update.

I realise those consequences aren't particularly obvious for non-distro developers, which is why I'd like to see the previous policy of "don't change the magic number in maintenance releases" captured as a test case.

OK, but how would you propose to fix broken bytecodes in a bugfix be fixed if you can't bump the magic number? And why aren't you regenerating the bytecode when updating Python or shipping the bytecode with the install (and I bet the answer is "RPM doesn't support it" and "it goes against packaging guidelines to include auto-generated files", but I thought I would ask :) .

We *do* ship the bytecode in the built RPMs - the issue is that Python maintenance releases shouldn't be triggering rebuilds of thousands of system components (cf http://fedora.portingdb.xyz/ ) and potentially breaking otherwise working end user applications.

As far as "What about bytecode generation bugs?" goes, then my answer would be to find a way to fix the bug for freshly generated bytecode, without forcing stale bytecode to be regenerated. That way folks that actually hit the original bug have a way of solving it (clear their bytecode cache), while we also don't forcibly break the world.

That is, the problem with the resolution of issue 27836 wasn't fixing the bytecode generation when using double-star unpacking twice in a single function call - it was forcing the regeneration of *all* bytecode just because *some* bytecode would be broken if it used a particular new language feature. It's the equivalent of deliberately breaking the ABI for all extension modules just because we found a header bug affecting one particular function signature.

Note that Fedora doesn't even rebuild all the extension modules when bumping CPython to a new maintenance release, let alone rebuilding and re-releasing all the pure Python ones. (RPM supports doing that just fine, but it would mean shipping thousands of updated binary artifacts instead of just one - the new CPython maintenance release)

On Feb 10, 2017, at 05:46 PM, Nick Coghlan wrote:

>Note that Fedora doesn't even rebuild all the extension modules when bumping
>CPython to a new maintenance release, let alone rebuilding and re-releasing
>all the pure Python ones. (RPM supports doing that just fine, but it would
>mean shipping thousands of updated binary artifacts instead of just one - the
>new CPython maintenance release)

Debian/Ubuntu doesn't rebuild extension modules either.  We don't ship .pyc
files in binary packages, but instead build them at install time on the user's
machine.

> How would you propose to fix broken bytecodes in a bugfix be fixed if you can't bump the magic number?

That would depend on the situation. I can imagine that if the bug is severe enough, the number could be bumped, after careful discussion, and with the change being advertised rather loudly.
The test case that's proposed can be changed if it's indeed the best thing to do, but it should at the very least be mentioned in release notes.


FWIW, I'm not convinced the bug here was severe enough. Practically no one is using the 3.5+ syntax in libraries: we checked to find that *zero* packages in Fedora are using the affected opcode. But that's moot now – wider discussion on that should have happened before the release.

Ah, interesting. The install-time generation case is even trickier, since you presumably can't force in-place regeneration of existing pyc files on systems that previously had 3.5.2 and upgraded to 3.5.3.

When I last spoke to Peter about this yesterday, he was exploring the idea of a downstream-only patch to 3.5.3 that gave it the ability to load pyc files that used the 3.5.2 magic number (so it would *emit* files with the new magic number, but still allow use of pyc files with the old one). Assuming he can get that to work, then I guess it may be of interest to Debian & Ubuntu as well.

According to issue27286, Ubuntu 16.04LTS has the fix and uses the new magic number. (And this is one reason undoing the change in CPython is out of the question.)


The patch Nick mentioned is brewing at https://github.com/encukou/cpython/commit/magic-number-workaround .

This is strictly a problem for the system Python, right?  In that case, can't the dist package clear __pycache__ under the system site-packages directory (and any other user-read-only dirs) during install of the updated Python?

Is the concern that upgrading Python may force all .pyc files to be re-written (perhaps unnecessarily)?  If so, I'm not clear on why it's okay between major versions but not minor ones.  Frequency perhaps?  Is the cost of re-compiling all .pyc files (which mostly won't happen all at the same time) significant enough to warrant changing the status quo, particularly since the magic number rarely changes in a minor release?

As one (poor) alternative, we could require re-compiling a .pyc file only if it contains the affected bytecode(s).  Granted, that would probably require associating every bytecode with a magic number.  The required comparison would likely be more expensive than just re-compiling. :/

Another alternative would be to leverage sys.dont_write_bytecode somehow.

Providing some additional background:

- 3.5.3 was the *first time ever* in more than two decades that CPython changed the magic number in a maintenance release. It *wasn't* necessary to do so - it could have been left alone (as 3.5.0, 3.5.1, and 3.5.2 can read the revised bytecode just fine - no changes were made to ceval, only to the code generator). If the magic number had been left alone, recompilation would have been limited solely to folks with affected bytecode files, which is approximately zero people in practice (as anyone impacted by the problem would have been getting weird runtime behaviour and presumably found a way to work around it).

- because the pyc caches for system packages on Linux are in system-owned directories, they're generated either at build time (Fedora et al) or install time (Debian et al) by the package that owns the corresponding source files and then installed with elevated privileges (e.g. "sudo dnf install python-requests"). These files *do not* belong to the CPython package, they belong to the individual Python modules that depend on CPython, so messing with them when installing a new version of CPython would be a significant breach of distro policies

- as a result, removing CPython 3.5.3's ability to read pyc files generated by 3.5.0, 3.5.1 or 3.5.2 means that either the distros have to regenerate all of their Python dependent packages (to force recompilation of the affected bytecode files), or else patch the system Python so it can still read the old pyc files (which is the approach Peter is currently pursuing, since requiring a mass rebuild for a maintenance update is *also* against distro policy)

Thus this request to take the previously unwritten de facto policy (which worked fine and clearly isn't a major limitation on CPython maintainability, since the one time it has been done in 20+ years didn't actually need to do it) and make it an explicit official policy that's enforced by the test suite.

The reason that none of these concerns apply to Python feature releases is because those are only rolled out as part of distro version upgrades (e.g. the switch from 3.5 to 3.6 as the system Python is one of the upgrades going from F25 to F26), where both distro maintainers and distro users already expect to have to rebuild and reinstall almost everything.

My apologies for breaking the world. But I considered this bug as security issue. Unlikely, but not impossible. The BUILD_MAP_UNPACK_WITH_CALL opcode is rarely used, but if it is used, it can cause reading arbitrary memory and either crashing or even exposing the content of this memory in error message that can be leaked to remote user. This hole would be better to close early. I expected this change would be included in 3.5.2.

The more robust solution would be to add the support of two magic numbers and check on loading whether the BUILD_MAP_UNPACK_WITH_CALL opcode is occurred in a precompiled file with old magic number. But this solution looked too excessive.

It would be nice to design a mechanism for possible future bytecode fixes.

OK, so it sounds like Serhiy had good reason to do what he did.

If we do add a test then I would argue that the message should be very clear that the magic number can change in a bugfix release but that it should be discussed with python-dev first so that downstream can be aware of the issue ahead of time (as Nick pointed out, it took a very special circumstance to make this come up and so chances are it shouldn't again, but institutional memory doesn't necessarily last 20 years either ;) .

Could anyone please explain to me the comment from _bootstrap_external.py?

# Starting with the adoption of PEP 3147 in Python 3.2, every bump in magic
# number also includes a new "magic tag", i.e. a human readable string used
# to represent the magic number in __pycache__ directories.  When you change
# the magic number, you must also set a new unique magic tag.  Generally this
# can be named after the Python major version of the magic number bump, but
# it can really be anything, as long as it's different than anything else
# that's come before.  The tags are included in the following table, starting
# with Python 3.2a0.

Now I think that it would be safer to omit the function name in error message rather than fixing the compilation and bumping the magic number.

Nick, if you want I could write a patch that fixes the possible security issue for .pyc files with old magic number.

That comment is poorly worded. All that is really necessary is that the tag differ when you want to have a .pyc file exist side-by-side with another .pyc file. So far we have only changed it when releasing a new version of Python, but for instance Python 3.5.2 could have had a different tag and that would have prevented trying to overwrite any pre-existing .pyc files for 3.5.1 and instead create new ones.

So the tag doesn't need to change for every new magic number, just when you don't want to overwrite .pyc files created by another version of Python.

I added PR #54 with a test as Nick describes for checking magic number changes and explaining to the developer the procedure to follow if this is required in a maintenance release.

The patch looks overcomplicated to me. The test could be as simple as

    self.assertEqual(importlib.util.MAGIC_NUMBER, ...)

Thanks Eric! I've left some detailed comments on the PR, with the main one being to include the "expected" table directly in the test case so it doesn't require any changes to importlib itself.

For Serhiy - thanks for the explanation of the potential security impact of the original bug, as well as the additional patch that resolves that aspect even for the invalid bytecode.

As for as the complexity of the test case itself goes, the main cases we're interested in here are how it fails:

- when an RM goes to make a candidate release, they may forget to record the now expected magic number for that release series and get prompted by the failing test case. We'd like them to be able to just copy the magic number from _bootstrap_external.py directly into the table of expected magic numbers

- when someone attempts to bump the magic number in a maintenance release, we want the warning that that's a higher impact change than it may first appear (one we know how to deal with now, but still something we'd prefer to avoid if we possibly can)

So I think it makes sense to have a relatively verbose test case, even though the core of it is a really simple check of a single value.

What is the purpose of having a *table* of magic numbers? We need just one value. Different Python versions run different tests.

And I don't think this test needs adding separate file.

I think the table of expected magic numbers will be easier to maintain than a single value, as otherwise you either have to:

- update it every time the magic number changes during the alpha/beta cycle (which would be a bad habit to encourage, since it runs counter to the purpose of the test)

- change it to None for the alpha/beta cycle (to effectively disable the test) before setting it again at the next release candidate

By contrast, the version table will typically be append only - the only time it will routinely require modification is for the first release candidate of a particular feature release, and the change will be to add a new entry for that release.

I see it as being similar to why we have the full history of all the magic numbers in _bootstrap_external.py, even though we technically only need the latest one.

Regarding the naming of the test file: I agree this can just be added to Lib/test/test_importlib/test_util:MagicNumberTests rather than being added as a new test file.

I don't understand how the table can make maintaining easier. You need to support multiple values in every branch even if the only one value is used. I'm sure unused values will became outdated pretty fast.

"alpha" and "beta" stages are not related here. The test should be skipped at these stages (I recommend to use skipTest() or the skipUnless() decorator rather than just make test always success). Magic number can be changed multiple times at "alpha" and "beta" stages. Release manager needs to update the test only when forming the first release candidate. And his should not do anything if the magic number was not changed in this feature release.

I commented on the original issue where the magic number was changed. This broke the world at work for me. Our distribution mechanism for Python programs is zipped bundles of .pyc and .so files, the optimized variant doesn't keep .py files around. So suddenly otherwise correct bundles were refusing to start. Better yet, since the rollout of Python is staged and takes a while to do safely, new packages started appear with the new magic number that were refusing to start on 3.5.0. This was not a fun day :)

So, while I understand Brett's and Serhiy's reasoning, I'd be very careful about ever bumping magic numbers in minor releases again, and definitely communicate loudly if doing so.

Oh, if you're wondering why I even hit this problem before 3.5.3: since 3.5.2 had a few regressions we couldn't live with, I released 3.5.2+ from the latest commit on the 3.5 branch at the time (after checking the buildbots, running tests on our own, etc.). I repeated this feat with 3.6.0+ and again hit a thing that would likely upset some people in 3.6.1 (see #29519) but this time decided to patch it instead of just complaining ;)

[Nick]
> "Requires a mass rebuild of all the Python packages in the distro to get things working properly again" is again a major concern for what's nominally a low impact maintenance update.

With my Enthought developer hat on, I'd like to second this comment. We'll shortly be shipping Python 3.5.2 in Enthought Canopy, and this bytecode change presents us with a major issue to resolve (internal discussion about how to do so is ongoing). If/when we make 3.5.3 available, and the user upgrades their core Python, all user-installed 3rd party packages become semi-broken, and we potentially need to ask the user to upgrade many hundreds of packages. (We also need to rebuild and re-test all those packages for our own repository, which is a time-consuming process.)

[Łukasz]
> I'd be very careful about ever bumping magic numbers in minor releases again, and definitely communicate loudly if doing so.

Also seconded.

Given some of the other folks encountering problems, perhaps Petr's patch
to accept both magic numbers should be submitted upstream so it's the
default in 3.5.4+? And then we ask Larry and the other release management
folks nicely for an early 3.5.4 release?

On 12 Feb 2017 11:54 pm, "Serhiy Storchaka" <report@bugs.python.org> wrote:

Serhiy Storchaka added the comment:

I don't understand how the table can make maintaining easier. You need to
support multiple values in every branch even if the only one value is used.
I'm sure unused values will became outdated pretty fast.

"alpha" and "beta" stages are not related here. The test should be skipped
at these stages (I recommend to use skipTest() or the skipUnless()
decorator rather than just make test always success). Magic number can be
changed multiple times at "alpha" and "beta" stages. Release manager needs
to update the test only when forming the first release candidate. And his
should not do anything if the magic number was not changed in this feature
release.

Serhiy's argument here & the fact we've switched to a cherrypick model for
maintenance branches has persuaded me we just want a single "expected magic
number" in the test case rather than the full table that covers multiple
releases.

[Nick]
> perhaps Petr's patch to accept both magic numbers should be submitted upstream so it's the default in 3.5.4+? [...]

Enthought's primary buildsystem maintainer is unfortunately out of contact for the week, so I can't speak with any authority on this, but I believe that we'd find this helpful.

> perhaps Petr's patch to accept both magic numbers should be submitted upstream so it's the default in 3.5.4+?

I don't think it can be submitted in the current state; it's a one-time hack intended to be removed and forgotten at the end of 3.5's lifetime.

I could write a patch with proper support for accepting a range of magic numbers, but such a feature probably won't be needed again.

> it's a one-time hack intended to be removed and forgotten at the end of 3.5's lifetime.

Fair enough. If necessary, one of our options is to apply that patch or something like it to the Python executables that we distribute for Canopy. But in general, staying as close as possible to the upstream Python is desirable, so if there were a possibility for it to go into upstream Python, that would be helpful.

In Fedora also want to stay close to upstream: every downstream patch is like an open issue, something we'd like to merge given enough time and people. But, for a problem that:
- is fixed in the latest version (3.6)
- is getting a test to not happen again
- only affects downstream distributions
I'm fine with a downstream patch.

I'd also be happy to work on making the patch acceptable upstream, but I need to hear some core devs' expectations on that. Where on the spectrum from throwaway hack to full-scale mechanism for accepting ranges of magic numbers should such a patch patch be?

On Feb 12, 2017, at 05:42 PM, Brett Cannon wrote:

>That comment is poorly worded.

Pretty sure at one time it was accurately worded, but things have changed
since PEP 3147 so the comment could use an update.

> These files *do not* belong to the CPython package, they belong to the
> individual Python modules that depend on CPython, so messing with them
> when installing a new version of CPython would be a significant breach
> of distro policies.

Thanks for clarifying, Nick.  This was the piece I'd missed.

Serhiy posted a patch at http://bugs.python.org/issue29537 that addresses the original problem from http://bugs.python.org/issue27286 in a different way: it simply omits the function name in cases where the stack reference is ambiguous.

So what I think we should do for 3.5.4 is implement the one-off patch that allows the remainder of the 3.5.x series to accept both the original 3.5.0 magic number and the 3.5.3+ magic number, in combination with Serhiy's change to the eval loop to render any malformed legacy bytecode harmless.

The 2.7, 3.6 and master branches can then just get the test case that locks the bytecode to a particular value in any given branch that has reached release candidate status.

I just merged https://github.com/python/cpython/commit/93602e3af70d3b9f98ae2da654b16b3382b68d50 as the fix for issue #29537, so 3.5.4+ should handle legacy bytecode files without any complaints (and redistributors can backport it to 3.5.3 as needed).

For *this* issue, the main requested change to the proposed test case is to simplify it to use Serhiy's suggested approach: just define a single "EXPECTED_MAGIC_NUMBER" in the test suite, and have that be different on different maintenance branches.

For branches in alpha and beta phase, the test case should be skipped with a message like "Bytecode stability is not checked in alpha & beta releases"

Thank you for the review and suggestions Nick, Serhiy, and all. I believe that I have implemented the suggestions here and on github into a new commit on my pull request which moves the test into an existing module and removes the notion of a table in favor of a single expected value.

New changeset d6d344d8330a5975fc102e8f275d47044294f1d1 by Łukasz Langa (Eric Appelt) in branch 'master':
bpo-29514: Check magic number for bugfix release (#54)
https://github.com/python/cpython/commit/d6d344d8330a5975fc102e8f275d47044294f1d1

Sorry I didn't see the just-landed PR earlier, but it may make sense to not skip that test under *any* release level for a micro version > 0:

    @unittest.skipUnless(
        (sys.version_info.micro > 0 or
         sys.version_info.releaselevel in ('final', 'release')),
        'only applies to candidate or final python release levels'
    )

We don't use the alpha and beta phases for maintenance releases, so I think the checks are functionally equivalent.

For example, the 3.6 branch currently reports itself as "3.6.1 final", will temporarily report itself as "3.6.2 release" during the rc period, and then go back to reporting itself as "3.6.2 final".

Assigning to myself to do the backports.

New changeset d071a20f136547a490783fcfa969c6528025a6d2 by Nick Coghlan in branch '3.5':
[3.5] bpo-29514: Check magic number for bugfix releases (#2158)
https://github.com/python/cpython/commit/d071a20f136547a490783fcfa969c6528025a6d2

New changeset 92e9e35292d4e2044c25970389f3cdb30cfc903a by Nick Coghlan in branch '3.6':
[3.6] bpo-29514: Check magic number for bugfix releases (#2157)
https://github.com/python/cpython/commit/92e9e35292d4e2044c25970389f3cdb30cfc903a

New changeset a51f12f154baef9e10698f43bd19b36d01af62f4 by Nick Coghlan in branch '2.7':
[2.7] bpo-29514: Check magic number for bugfix releases
https://github.com/python/cpython/commit/a51f12f154baef9e10698f43bd19b36d01af62f4

This has now been backported to all active maintenance branches.

For 2.7, I backported the test as a standalone one in Lib/test/test_import_magic.py, since there's only a partial importlib implementation in 2.7, and hence no test_importlib directory.

Thanks all!

Is it intentionally that old magic number is used in 3.7? The test will become failing just after reaching the RC stage.

Serhiy: I believe that magic number was current when the test was written, and it hasn't been getting updated since the test is currently being skipped.

Given how we designed the skip logic, I think that counts as "working as intended" - we don't really care about magic number stability until the release candidate stage, so that's when the test needs to be updated to match the implementation.

New changeset e7d3cccd8e0da93a15ca80f4b6aac2ca3435fd06 by Ned Deily in branch 'master':
bpo-29514: Make magic number test work for candidates
https://github.com/python/cpython/commit/e7d3cccd8e0da93a15ca80f4b6aac2ca3435fd06

New changeset 8511b448a55929f15aa5730c9e30cbb654978ea4 by Ned Deily in branch '3.7':
bpo-29514: Make magic number test work for candidates
https://github.com/python/cpython/commit/8511b448a55929f15aa5730c9e30cbb654978ea4

New changeset 57389dbaa897fadc97de78af67d5e555e28be60b by Ned Deily in branch '3.6':
bpo-29514: Make magic number test work for candidates (GH-7945)
https://github.com/python/cpython/commit/57389dbaa897fadc97de78af67d5e555e28be60b

New changeset f79386f669c22199e3ce99cfba2e5945c73beb47 by Ned Deily in branch '2.7':
bpo-29514: Make magic number test work for candidates (GH-7946)
https://github.com/python/cpython/commit/f79386f669c22199e3ce99cfba2e5945c73beb47

New changeset 828351716ec436bfa174cfdb33def7ee7c0d2e63 by Ned Deily in branch '3.7':
bpo-29514: Make magic number test work for candidates
https://github.com/python/cpython/commit/828351716ec436bfa174cfdb33def7ee7c0d2e63