Skip to content

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

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

Merged
merged 3 commits into from
Aug 24, 2021

Conversation

ammaraskar
Copy link
Member

@ammaraskar ammaraskar commented Apr 25, 2020

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

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

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

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

@ammaraskar
Copy link
Member Author

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?

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

👍

@ammaraskar
Copy link
Member Author

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

@ammaraskar ammaraskar added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 23, 2021
@bedevere-bot
Copy link

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

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 23, 2021
@serhiy-storchaka serhiy-storchaka added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes type-bug An unexpected behavior, bug, or error labels Aug 24, 2021
@serhiy-storchaka
Copy link
Member

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

@serhiy-storchaka
Copy link
Member

@ammaraskar, please merge this branch with main.

@miss-islington
Copy link
Contributor

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 pushed a commit to miss-islington/cpython that referenced this pull request Aug 24, 2021
…int (pythonGH-19708)

Co-authored-by: Stéphane Wirtel <[email protected]>
(cherry picked from commit bb21e28)

Co-authored-by: Ammar Askar <[email protected]>
@bedevere-bot
Copy link

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

@bedevere-bot bedevere-bot removed needs backport to 3.10 only security fixes needs backport to 3.9 only security fixes labels Aug 24, 2021
@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 24, 2021
…int (pythonGH-19708)

Co-authored-by: Stéphane Wirtel <[email protected]>
(cherry picked from commit bb21e28)

Co-authored-by: Ammar Askar <[email protected]>
@ammaraskar
Copy link
Member Author

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

ammaraskar added a commit that referenced this pull request Aug 24, 2021
…igned int (GH-19708)

(cherry picked from commit bb21e28)

Co-authored-by: Ammar Askar <[email protected]>
Co-authored-by: Stéphane Wirtel <[email protected]>
ammaraskar added a commit that referenced this pull request Aug 24, 2021
…signed int (GH-19708)

(cherry picked from commit bb21e28)

Co-authored-by: Ammar Askar <[email protected]>
Co-authored-by: Stéphane Wirtel <[email protected]>
@ammaraskar ammaraskar deleted the compileall_Y23K branch August 24, 2021 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants