Skip to content

gh-136170: Revert adding ZipFile.data_offset #136950

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 5 commits into from
Jul 21, 2025

Conversation

emmatyping
Copy link
Member

@emmatyping emmatyping commented Jul 21, 2025

As reported in #136170, ZipFile.data_offset isn't always correct depending on how one measures offsets. With 3.14rc1 tomorrow, I decided it would be best to push this to 3.15 and take more time to think about the feature and it's implementation.

This PR reverts PRs #132165 and #132178.

Note that I had to resolve a merge conflict in ZipFile._RealGetContents due to #134250 landing after the two PRs adding data_offset, so this isn't quite a clean revert of the two above PRs.


📚 Documentation preview 📚: https://cpython-previews--136950.org.readthedocs.build/

@emmatyping emmatyping requested a review from gpshead July 21, 2025 19:45
@emmatyping emmatyping added skip news needs backport to 3.14 bugs and security fixes labels Jul 21, 2025
@emmatyping emmatyping changed the title gh-136170: Revert ZipFile.data_offset gh-136170: Revert adding ZipFile.data_offset Jul 21, 2025
@gpshead gpshead removed the skip news label Jul 21, 2025
Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

I added a news entry for completeness as someone reading the verbose news could find the earlier news entry and wonder otherwise.

@gpshead gpshead enabled auto-merge (squash) July 21, 2025 22:39
@picnixz
Copy link
Member

picnixz commented Jul 21, 2025

I've updated the NEWS entry to use the exact versioning as in PEP-745.

@gpshead gpshead merged commit 6bf1c0a into python:main Jul 21, 2025
41 checks passed
@miss-islington-app
Copy link

Thanks @emmatyping for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 21, 2025
* Revert "pythongh-84481: Make ZipFile.data_offset more robust (pythonGH-132178)"

This reverts commit 6cd1d6c.

* Revert "pythongh-84481: Add ZipFile.data_offset attribute (pythonGH-132165)"

This reverts commit 0788948.

---------
(cherry picked from commit 6bf1c0ab3497b1b193812654bcdfd0c11b4192d8)

Co-authored-by: Emma Smith <emma@emmatyping.dev>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
@bedevere-app
Copy link

bedevere-app bot commented Jul 21, 2025

GH-136955 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 21, 2025
@emmatyping
Copy link
Member Author

I added a news entry for completeness as someone reading the verbose news could find the earlier news entry and wonder otherwise.

Thanks! That's a good idea.

gpshead added a commit that referenced this pull request Jul 22, 2025
…36955)

gh-136170: Revert adding `ZipFile.data_offset` (GH-136950)

* Revert "gh-84481: Make ZipFile.data_offset more robust (GH-132178)"

This reverts commit 6cd1d6c.

* Revert "gh-84481: Add ZipFile.data_offset attribute (GH-132165)"

This reverts commit 0788948.

---------
(cherry picked from commit 6bf1c0a)

Co-authored-by: Emma Smith <emma@emmatyping.dev>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM Raspbian Linux Asan 3.14 (no tier) has failed when building commit caef946.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1816/builds/25) and take a look at the build logs.
  4. Check if the failure is related to this commit (caef946) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1816/builds/25

Summary of the results of the build (if available):

Click to see traceback logs
remote: Enumerating objects: 16, done.        
remote: Counting objects:   8% (1/12)        
remote: Counting objects:  16% (2/12)        
remote: Counting objects:  25% (3/12)        
remote: Counting objects:  33% (4/12)        
remote: Counting objects:  41% (5/12)        
remote: Counting objects:  50% (6/12)        
remote: Counting objects:  58% (7/12)        
remote: Counting objects:  66% (8/12)        
remote: Counting objects:  75% (9/12)        
remote: Counting objects:  83% (10/12)        
remote: Counting objects:  91% (11/12)        
remote: Counting objects: 100% (12/12)        
remote: Counting objects: 100% (12/12), done.        
remote: Compressing objects:   8% (1/12)        
remote: Compressing objects:  16% (2/12)        
remote: Compressing objects:  25% (3/12)        
remote: Compressing objects:  33% (4/12)        
remote: Compressing objects:  41% (5/12)        
remote: Compressing objects:  50% (6/12)        
remote: Compressing objects:  58% (7/12)        
remote: Compressing objects:  66% (8/12)        
remote: Compressing objects:  75% (9/12)        
remote: Compressing objects:  83% (10/12)        
remote: Compressing objects:  91% (11/12)        
remote: Compressing objects: 100% (12/12)        
remote: Compressing objects: 100% (12/12), done.        
remote: Total 16 (delta 0), reused 0 (delta 0), pack-reused 4 (from 1)        
From https://github.com/python/cpython
 * branch                    3.14       -> FETCH_HEAD
Note: switching to 'caef946a25aac557f13cb66fbc70f06225546e67'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at caef946a25a [3.14] gh-136170: Revert adding `ZipFile.data_offset` (GH-136950) (#136955)
Switched to and reset branch '3.14'

configure: WARNING: no system libmpdec found; falling back to bundled libmpdec (deprecated and scheduled for removal in Python 3.16)

In file included from ./Include/internal/pycore_dict.h:11,
                 from Objects/typeobject.c:7:
In function ‘Py_DECREF_MORTAL’,
    inlined from ‘PyStackRef_XCLOSE’ at ./Include/internal/pycore_stackref.h:660:9,
    inlined from ‘_PyThreadState_PopCStackRef’ at ./Include/internal/pycore_stackref.h:746:5,
    inlined from ‘vectorcall_maybe’ at Objects/typeobject.c:2980:9:
./Include/internal/pycore_object.h:481:8: warning: array subscript 0 is outside array bounds of ‘PyObject[0]’ {aka ‘struct _object[]’} [-Warray-bounds]
  481 |     if (--op->ob_refcnt == 0) {
      |        ^

Timeout (0:05:00)!
Thread 0x0000007f9b0ef100 [Thread-2] (most recent call first):
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/subprocess.py", line 2058 in _wait
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/subprocess.py", line 1278 in wait
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 194 in _run_process
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 299 in run_tmp_files
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 363 in _runtest
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 403 in run
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/threading.py", line 1081 in _bootstrap_inner
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/threading.py", line 1043 in _bootstrap

Thread 0x0000007f9b8ff100 [Thread-1] (most recent call first):
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/subprocess.py", line 2022 in _try_wait
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/subprocess.py", line 2047 in _wait
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/subprocess.py", line 1278 in wait
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 194 in _run_process
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 299 in run_tmp_files
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 363 in _runtest
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 403 in run
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/threading.py", line 1081 in _bootstrap_inner
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/threading.py", line 1043 in _bootstrap

Thread 0x0000007fa767c6c0 [python] (most recent call first):
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/utils.py", line 61 in format_duration
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 464 in get_running
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 551 in _get_result
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/run_workers.py", line 610 in run
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/main.py", line 455 in _run_tests_mp
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/main.py", line 561 in _run_tests
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/main.py", line 598 in run_tests
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/main.py", line 767 in main
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/libregrtest/main.py", line 775 in main
  File "/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/Lib/test/__main__.py", line 2 in <module>
  File "<frozen runpy>", line 88 in _run_code
  File "<frozen runpy>", line 198 in _run_module_as_main
make: *** [Makefile:2462: buildbottest] Error 1

Cannot open file '/home/buildbot/buildarea/3.14.pablogsal-rasp.asan/build/test-results.xml' for upload

danny0838 added a commit to danny0838/zipremove that referenced this pull request Jul 22, 2025
- The `data_offset` attribute is retracted in 3.14rc:
  ref: python/cpython#136950
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants