Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpo-34990: Treat the pyc header's mtime in compileall as an unsigned int #19708

Merged
merged 3 commits into from Aug 24, 2021
Merged

bpo-34990: Treat the pyc header's mtime in compileall as an unsigned int #19708

merged 3 commits into from Aug 24, 2021

Conversation

@ammaraskar
Copy link
Member

@ammaraskar ammaraskar commented Apr 25, 2020 •
edited by bedevere-bot

Created with @matrixise's blessing to continue their work. This is an alternative to changing the timestamps to 64-bit. Should last for a while.

I didn't update Tools/checkpyc.py like the original PR there pending a decision on a bug I made to remove it

https://bugs.python.org/issue34990
@ammaraskar ammaraskar mentioned this pull request Apr 25, 2020
Closed
@brettcannon brettcannon removed their request for review Apr 26, 2020
vstinner
vstinner reviewed Apr 29, 2020
View changes
Copy link
Member

@vstinner vstinner left a comment

I would prefer to mimick importlib._bootstrap_external which uses:

def _pack_uint32(x):
    """Convert a 32-bit integer to little-endian."""
    return (int(x) & 0xFFFFFFFF).to_bytes(4, 'little')

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Apr 29, 2020

See my comments to #9892. If they be addressed it would be merged a long time ago.

serhiy-storchaka
serhiy-storchaka reviewed Apr 29, 2020
View changes
Lib/compileall.py Show resolved Hide resolved
Lib/test/test_compileall.py Outdated Show resolved Hide resolved
Lib/test/test_zipimport.py Outdated Show resolved Hide resolved
@ammaraskar @matrixise
bpo-34990: Treat the pyc header's mtime in compileall as an unsigned int
3911731 
Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>
@ammaraskar
Copy link
Member Author

@ammaraskar ammaraskar commented Apr 29, 2020

Updated to just drop the number down to the lower 32-bits like importlib._bootstrap_external

Not sure if the new test I added is safe though, as in can we assume that file-systems in general will support timestamps of larger than 32-bit?

serhiy-storchaka
serhiy-storchaka reviewed Apr 30, 2020
View changes
Lib/test/test_compileall.py Outdated Show resolved Hide resolved
Lib/test/test_compileall.py Outdated Show resolved Hide resolved
Lib/test/test_zipimport.py Show resolved Hide resolved
Lib/test/test_zipimport.py Outdated Show resolved Hide resolved
serhiy-storchaka
serhiy-storchaka approved these changes Apr 30, 2020
View changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

👍

vstinner
vstinner reviewed Apr 30, 2020
View changes
Lib/test/test_compileall.py Outdated Show resolved Hide resolved
@ammaraskar ammaraskar mentioned this pull request Aug 31, 2020
Closed
@ammaraskar
Copy link
Member Author

@ammaraskar ammaraskar commented Mar 24, 2021

@serhiy-storchaka Just a little ping, is this ready to go or are there any other changes that should be made here?

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Aug 23, 2021

🤖 New build scheduled with the buildbot fleet by @ammaraskar for commit 91b54fd 🤖

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Aug 24, 2021

Sorry, I missed that this PR was not merged yet.

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Aug 24, 2021

@ammaraskar, please merge this branch with main.

@serhiy-storchaka serhiy-storchaka mentioned this pull request Aug 24, 2021
Draft
@serhiy-storchaka serhiy-storchaka merged commit bb21e28 into python:main Aug 24, 2021
80 of 81 checks passed
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Aug 24, 2021

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

miss-islington added a commit to miss-islington/cpython that referenced this issue Aug 24, 2021
@ammaraskar @miss-islington
bpo-34990: Treat the pyc header's mtime in compileall as an unsigned …
0865039 
…int (pythonGH-19708)

Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>
(cherry picked from commit bb21e28)

Co-authored-by: Ammar Askar <ammar@ammaraskar.com>
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Aug 24, 2021

GH-27928 is a backport of this pull request to the 3.10 branch.

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Aug 24, 2021

GH-27929 is a backport of this pull request to the 3.9 branch.

miss-islington added a commit to miss-islington/cpython that referenced this issue Aug 24, 2021
@ammaraskar @miss-islington
bpo-34990: Treat the pyc header's mtime in compileall as an unsigned …
7ff1810 
…int (pythonGH-19708)

Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>
(cherry picked from commit bb21e28)

Co-authored-by: Ammar Askar <ammar@ammaraskar.com>
@ammaraskar
Copy link
Member Author

@ammaraskar ammaraskar commented Aug 24, 2021

Thank you for merging Serhiy, I was asleep when you LGTM'd :)

ammaraskar added a commit that referenced this issue Aug 24, 2021
@ammaraskar @miss-islington @matrixise
[3.9] bpo-34990: Treat the pyc header's mtime in compileall as an uns…
9d3b6b2 
…igned int (GH-19708)

(cherry picked from commit bb21e28)

Co-authored-by: Ammar Askar <ammar@ammaraskar.com>
Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>
ammaraskar added a commit that referenced this issue Aug 24, 2021
@ammaraskar @miss-islington @matrixise
[3.10] bpo-34990: Treat the pyc header's mtime in compileall as an un…
0af681b 
…signed int (GH-19708)

(cherry picked from commit bb21e28)

Co-authored-by: Ammar Askar <ammar@ammaraskar.com>
Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>
@ammaraskar ammaraskar deleted the compileall_Y23K branch Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner

@serhiy-storchaka serhiy-storchaka

Assignees
No one assigned
Labels
CLA signed type-bugfix
Projects
None yet
Milestone
No milestone
6 participants
@ammaraskar @serhiy-storchaka @bedevere-bot @miss-islington @vstinner @the-knights-who-say-ni