Skip to content

bpo-34726: Fix handling of hash-based pycs in zipimport#10327

Merged
serhiy-storchaka merged 3 commits into
python:masterfrom
elprans:fix-zipimport-hash-pyc
Nov 7, 2018
Merged

bpo-34726: Fix handling of hash-based pycs in zipimport#10327
serhiy-storchaka merged 3 commits into
python:masterfrom
elprans:fix-zipimport-hash-pyc

Conversation

@elprans

@elprans elprans commented Nov 4, 2018 •
edited by serhiy-storchaka
Loading

Copy link
Copy Markdown
Contributor

Current support for hash-based bytecode files in zipimport is rather
sparse, which leads to test failures when the test suite is ran with
the SOURCE_DATE_EPOCH environment variable set.

This teaches zipimport to handle hash-based pycs properly.

https://bugs.python.org/issue34022

https://bugs.python.org/issue34726

@elprans
bpo-34022: Fix handling of hash-based pycs in zipimport
78c5db7 
Current support for hash-based bytecode files in `zipimport` is rather
sparse, which leads to test failures when the test suite is ran with
the ``SOURCE_DATE_EPOCH`` environment variable set.

This teaches zipimport to handle hash-based pycs properly.
@elprans elprans requested a review from a team November 4, 2018 23:20
ncoghlan
ncoghlan approved these changes Nov 5, 2018
View reviewed changes

@ncoghlan ncoghlan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new tests look good to me, and the changes required to make them pass seem sensible.

@ncoghlan

ncoghlan commented Nov 5, 2018

Copy link
Copy Markdown
Contributor

Thanks for the PR @elprans!

@serhiy-storchaka @warsaw @brettcannon This looks good to me, but I'd be happier if someone more familiar with the new zipimport code gave it a look before we hit the big green merge button :)

@serhiy-storchaka serhiy-storchaka self-requested a review November 5, 2018 13:54
@serhiy-storchaka serhiy-storchaka changed the title bpo-34022: Fix handling of hash-based pycs in zipimport bpo-34726: Fix handling of hash-based pycs in zipimport Nov 5, 2018
@serhiy-storchaka

serhiy-storchaka commented Nov 5, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

I created bpo-34726, but I'm not sure that any checks for pyc files are needed at all. They slow down import, and we can expect that pyc files in a zip archive are up to date with py files. See bpo-23734.

serhiy-storchaka
serhiy-storchaka reviewed Nov 5, 2018
View reviewed changes
Comment thread Lib/zipimport.py Outdated
Comment thread Lib/zipimport.py Outdated
Comment thread Misc/NEWS.d/next/Core and Builtins/2018-11-04-18-13-40.bpo-34022.U3btVj.rst
@elprans elprans force-pushed the fix-zipimport-hash-pyc branch from 9297c95 to a6a5eda Compare November 6, 2018 03:18
@serhiy-storchaka

serhiy-storchaka commented Nov 7, 2018

Copy link
Copy Markdown
Member

You need to rebuild frozen modules. Run make regen-importlib.

@elprans

elprans commented Nov 7, 2018

Copy link
Copy Markdown
Contributor Author

Done

@serhiy-storchaka serhiy-storchaka merged commit a6e956b into python:master Nov 7, 2018
yan12125 pushed a commit to archlinuxcn/repo that referenced this pull request Nov 12, 2018
python-git: re-enable previous-falling tests
164ea02 
These tests are fixed in python/cpython#10327
@vstinner

vstinner commented Nov 12, 2018

Copy link
Copy Markdown
Member

test_zipfile_compiled_checked_hash() fails on Python 3.7: we should backport this bugfix to 3.7, no?

@miss-islington

miss-islington commented Nov 12, 2018

Copy link
Copy Markdown
Contributor

Thanks @elprans for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Nov 12, 2018

Copy link
Copy Markdown
Contributor

Sorry, @elprans and @serhiy-storchaka, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker a6e956bcb0edbfe7f18af9be2215a5326ea6bf05 3.7

@vstinner

vstinner commented Nov 12, 2018

Copy link
Copy Markdown
Member

Ah, the change cannot be cherry-picked automatically. Maybe because of the generated file.

@elprans

elprans commented Nov 12, 2018

Copy link
Copy Markdown
Contributor Author

@vstinner I'll make a backport PR.

@elprans

elprans commented Nov 13, 2018

Copy link
Copy Markdown
Contributor Author

It's a bit more difficult than that. In 3.7 zipimport is in C.

@serhiy-storchaka

serhiy-storchaka commented Nov 13, 2018

Copy link
Copy Markdown
Member

It is easier to skip the test on 3.7.

@serhiy-storchaka serhiy-storchaka removed their assignment Nov 13, 2018
dundee pushed a commit to dundee/pkgbuilds that referenced this pull request Feb 13, 2022
@dundee
re-enable previous-falling tests
6ab3ee7 
These tests are fixed in python/cpython#10327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ncoghlan ncoghlan ncoghlan approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@elprans @ncoghlan @serhiy-storchaka @vstinner @miss-islington @the-knights-who-say-ni @bedevere-bot