[issue29514] Add a test case that prevents magic number changes in minor releases

[Ned Deily]

Ned Deily  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Ned Deily]

Ned Deily  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Ned Deily]

Ned Deily  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Ned Deily]

Change by Ned Deily :


--
pull_requests: +7555

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Ned Deily]

Change by Ned Deily :


--
pull_requests: +7554

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Ned Deily]

Ned Deily  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Ned Deily]

Ned Deily  added the comment:


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


--
nosy: +ned.deily

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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!

--
resolution:  -> fixed
stage: backport needed -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Changes by Nick Coghlan :


--
pull_requests: +2211

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Changes by Nick Coghlan :


--
pull_requests: +2209

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Changes by Nick Coghlan :


--
pull_requests: +2208

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--
assignee:  -> ncoghlan
versions:  -Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Eric Snow]

Eric Snow added the comment:

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'
)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Mariatta Wijaya]

Changes by Mariatta Wijaya :


--
stage: patch review -> backport needed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Łukasz Langa]

Łukasz Langa added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Eric Appelt]

Eric Appelt added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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"

--
stage:  -> patch review
type:  -> enhancement

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Carson Lam]

Changes by Carson Lam :


--
nosy: +Carson Lam

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Eric Snow]

Eric Snow added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Barry A. Warsaw]

Barry A. Warsaw added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Petr Viktorin]

Petr Viktorin added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Iryna]

Changes by Iryna :


--
nosy: +ishcherb

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Mark Dickinson]

Mark Dickinson added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Petr Viktorin]

Petr Viktorin added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Mark Dickinson]

Mark Dickinson added the comment:

[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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

On 12 Feb 2017 11:54 pm, "Serhiy Storchaka"  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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Mark Dickinson]

Mark Dickinson added the comment:

[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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Mark Dickinson]

Changes by Mark Dickinson :


--
nosy: +mark.dickinson

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Łukasz Langa]

Łukasz Langa added the comment:

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 ;)

--
nosy: +lukasz.langa

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Serhiy Storchaka]

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Eric Appelt]

Eric Appelt added the comment:

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.

--
nosy: +Eric Appelt
pull_requests: +49

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Brett Cannon]

Brett Cannon added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Brett Cannon]

Brett Cannon added the comment:

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 ;) .

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Eric Snow]

Eric Snow added the comment:

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.

--
nosy: +eric.snow

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Petr Viktorin]

Petr Viktorin added the comment:

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 .

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Petr Viktorin]

Petr Viktorin added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Barry A. Warsaw]

Barry A. Warsaw added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Brett Cannon]

Brett Cannon added the comment:

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 :) .

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

Nick Coghlan added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Brett Cannon]

Brett Cannon added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Petr Viktorin]

Petr Viktorin added the comment:

For the record, the magic number was changed in issue27286

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29514] Add a test case that prevents magic number changes in minor releases

[Nick Coghlan]

New submission from Nick Coghlan:

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.

--
messages: 287428
nosy: barry, brett.cannon, doko, encukou, ncoghlan
priority: normal
severity: normal
status: open
title: Add a test case that prevents magic number changes in minor releases
versions: Python 2.7, Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com