Skip to content

Reset the embed update log when it cannot be read - #3376

Merged
gaborbernat merged 3 commits into
pypa:mainfrom
pasmud:gsz/reset-unreadable-embed-update-log
Oct 2, 2026
Merged

gaborbernat merged 3 commits into
pypa:mainfrom
pasmud:gsz/reset-unreadable-embed-update-log

Conversation

@pasmud

@pasmud pasmud commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)

  • ran the linter to address style issues (tox -e fix)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation
  • for changes to activation scripts, pyvenv.cfg, wheel downloads, app-data or release workflows: explained in the
    PR text which outside input the change handles, such as a path, a prompt or a downloaded file, and how it stops that
    input from running as code. See the
    security scope.

The bug

The embed update log lives at <app-data>/wheel/<major.minor>/embed/3/<distribution>.json and records which newer
seed wheels are known. UpdateLog.from_app_data() in src/virtualenv/seed/wheels/periodic_update.py passed whatever it
read straight into from_dict(), which calls .get() on the result. A value of the wrong shape raises, and the seed
fails with a message that names neither the cause nor a way forward:

fail
Traceback (most recent call last):
  ...
  File ".../periodic_update.py", line 60, in periodic_update
    u_log = UpdateLog.from_app_data(app_data, distribution, for_py_version)
  File ".../periodic_update.py", line 196, in from_app_data
    return cls.from_dict(raw_json)
  File ".../periodic_update.py", line 187, in from_dict
    load_datetime(dictionary.get("started")),
                  ^^^^^^^^^^^^^^
AttributeError: 'list' object has no attribute 'get'
AttributeError("'list' object has no attribute 'get'")
RuntimeError: seed failed due to failing to download wheels pip

Five shapes all do it, verified by calling periodic_update() directly:

list                 -> RAISED AttributeError: 'list' object has no attribute 'get'
string               -> RAISED AttributeError: 'str' object has no attribute 'get'
versions-as-string   -> RAISED TypeError: string indices must be integers, not 'str'
completed-as-int     -> RAISED TypeError: strptime() argument 1 must be str, not int
versions-junk        -> RAISED TypeError: string indices must be integers, not 'str'

End to end, virtualenv <dest> --no-periodic-update exits 1 for each of those and does not create the environment.

This matters because the app data folder is shared and persists between runs. A log written by a different virtualenv
version, or left half written by an interrupted one, is enough to make every later seed fail until the user works out
that they need to delete a cache file.

The change

from_app_data() catches the shape errors, logs a warning naming the distribution, removes the file and returns an
empty log. The bundled wheel is then used and the environment is created, and the next run starts without the bad
log instead of warning again.

This matches what the surrounding code already does: JSONStoreDisk.read() deletes the file and returns None when
the JSON does not parse, so a corrupt log was already survivable, only a log that parses into the wrong shape was not.

Tests

Five cases, all failing on unmodified main (288f13c) and passing with the fix.

$ tox -e py -- tests/unit/seed/wheels/test_periodic_update.py -p no:randomly -k malformed

Before, with src/virtualenv/seed/wheels/periodic_update.py reverted to main:

E       AttributeError: 'list' object has no attribute 'get'
E       AttributeError: 'str' object has no attribute 'get'
E       TypeError: string indices must be integers, not 'str'
E       TypeError: string indices must be integers, not 'str'
E       TypeError: strptime() argument 1 must be str, not int
FAILED tests/unit/seed/wheels/test_periodic_update.py::test_periodic_update_drops_malformed_log[not-a-mapping]
FAILED tests/unit/seed/wheels/test_periodic_update.py::test_periodic_update_drops_malformed_log[string]
FAILED tests/unit/seed/wheels/test_periodic_update.py::test_periodic_update_drops_malformed_log[versions-not-a-list]
FAILED tests/unit/seed/wheels/test_periodic_update.py::test_periodic_update_drops_malformed_log[version-not-a-mapping]
FAILED tests/unit/seed/wheels/test_periodic_update.py::test_periodic_update_drops_malformed_log[completed-not-a-datetime]
======================= 5 failed, 35 deselected in 1.08s =======================

After:

======================= 5 passed, 35 deselected in 0.64s =======================

Each case writes the bad log into a temporary app data folder and asserts the bundled wheel is returned, the exact
warning is logged and the file is gone.

End to end, with a wrong-shape log planted in the app data folder, virtualenv <dest> --no-periodic-update now exits 0
for all four shapes I tried where it previously exited 1.

Wider selections, all passing:

tox -e py -- tests/unit/seed/ -p no:randomly    165 passed
tox -e py                                      474 passed, 50 skipped, 11 failed

Linter and type checks:

tox -e fix        -> OK, all 16 hooks passed
tox -e type       -> All checks passed!
tox -e type-3.9   -> All checks passed!

Full suite

The 11 failures above are unchanged by this branch. I compared the failing test IDs before and after by stashing the
change and re-running:

$ diff <failing IDs on main> <failing IDs on this branch>
IDENTICAL: same set of failures
before: 11  after: 11

They are artifacts of this sandbox, all reproducible on unmodified main:

  • tests/unit/test_util.py (2): the safe_delete tests rely on directory permissions denying an unlink, which does
    not happen for uid 0. This container runs as root.
  • tests/unit/activation/test_bash.py (9): bash renders the escaped \$ prompt sequence as # for uid 0 and as $
    for any other user, so the expected prompt text does not match. Confirmed by running the same PS1 as root and as
    uid 65534: root gives (a%n#HOME\b...), a normal user gives (a%n$HOME\b...).

fish, csh and tcsh are installed here, so those activator tests ran. powershell and nu are not installed, so
the PowerShell and Nushell activator tests skip. CI runs them, so a green local run does not guarantee a green CI run
for those files.

Security scope

The outside input is the embed update log file, which sits in the shared app data folder. It is data read with
json.loads, and nothing on this path executes or evaluates it. Before, a value of an unexpected shape propagated an
exception out of the seed; now it is reported and discarded, which narrows what a bad file can do rather than widening
it. No new input is read, and the fallback only ever selects the bundled wheel, which is the same one used when no log
exists at all.

pasmud and others added 2 commits October 2, 2026 15:39
The app data folder is shared between runs and outlives a single one, so the
embed update log on disk may have been written by another version or left
incomplete by an interrupted run. UpdateLog.from_app_data() passed whatever it
read straight to from_dict(), which raised on a value of the wrong shape, and
the seed then failed with "seed failed due to failing to download wheels pip",
which names neither the cause nor a way out.

Treat an unreadable log the way JSONStoreDisk.read() already treats
unparseable JSON: drop it and carry on. from_app_data() now catches the shape
errors, logs a warning naming the distribution, and returns an empty log, so
the bundled wheel is used and the environment is still created.

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
@gaborbernat
gaborbernat force-pushed the gsz/reset-unreadable-embed-update-log branch from e0c0cf4 to 56b8815 Compare October 2, 2026 15:48
The fallback warned that it reset the log but left the file on disk, so
each later run warned again and --no-periodic-update left it in place.
Delete the file the way JSONStoreDisk.read() deletes invalid JSON, test
against a real app data folder instead of a mocked read, assert the
exact warning and that the file is gone, and name the fragment after
this pull request.
@gaborbernat
gaborbernat force-pushed the gsz/reset-unreadable-embed-update-log branch from 56b8815 to 7f74f0f Compare October 2, 2026 16:34
@gaborbernat
gaborbernat merged commit 5b9b7de into pypa:main Oct 2, 2026
76 checks passed
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.

2 participants