Repository navigation
tools: a failed redraw cannot cut the card picture the repository ships - #3587
Merged
Merged
Conversation
readme_stats.py, readme_featured.py and readme_card.py each ended with img.save(OUT), and Pillow opens the destination before it encodes: a full disk, a failing encoder or a file the run may not rewrite left 16 KB of a 21 KB docs/stats.png under the name the front page shows, while the script printed "Nothing was written.". readme_card.py reads docs/stats.png to stack the card, so one truncated strip also stopped the card being redrawn. The three now publish through tools/picture_file.py, the shape the theme wallpaper scripts already use: encode into a staging folder beside the file, carry the old mode across, os.replace it into place. A symlink keeps its link; a second hard link or a non-regular file is refused before anything is encoded; a read-only file is refused without being truncated; a staging folder that cannot be removed after the rename is a warning, because the picture is already in place.
|
maintainer-bot review · suggests merge (p2) Thanks for the clear ulimit repro showing how a failed save could leave a cut-short docs/stats.png and still say "Nothing was written". Writing the picture beside the file and then renaming it into place, with tests, is a sound tool fix and looks ready to merge. Checks
This is a suggestion; a maintainer makes the final call. A maintainer can comment Reviewed by maintainer-bot · runs on 0G Private Computer |
Owner
|
Thanks @xbsheng, the new tools/picture_file.py stages the card PNG and swaps it in with os.replace(), so a failed redraw no longer cuts docs/stats.png short, with 12 tests behind it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tested on: no TV; the three scripts and
tools/picture_file.pyrun against the checkout, unit tests only.tools/readme_stats.py,tools/readme_featured.pyandtools/readme_card.pyeach ended withimg.save(OUT, ...), and Pillow opens the destination before it encodes (ImageFile.save()opens itw+b, then calls the encoder). A full disk, a raising encoder or a file the run may not rewrite therefore left whatever the encoder had written under the name the front page ships the picture at — and the script still printed "Nothing was written.".docs/stats.pngis also an input:readme_card.pyloads it to stack the card, so one truncated strip stopped the card being redrawn until the picture was restored with git.Reproduced without a stand-in encoder, on
origin/main294d3d5 and on this branch, same command each time:What changed
tools/picture_file.py(new, 73 lines):save(image, path, **options)— the shapetools/scenes_wallpapers.py,tools/blocky_wallpapers.pyandtools/wallpaper_prep.pyalready settled on for a picture that must survive a failed write. The picture is encoded into a staging folder beside its destination andos.replaced over it, so the destination is either the file that was there or the complete new one. A symlink keeps its link and its target is replaced, asimg.save(path)wrote through it; a destination that is not a regular file, or that has a second hard link, is refused before anything is encoded; a read-only file is refused without being truncated (a rename alone can replace a file its caller may not write); an existing file's mode is carried onto the new one and a fresh file keeps the modeopen()left; a staging folder that cannot be removed after the rename is a warning on stderr, because the picture is in place and the caller's message must not say nothing was written.tools/readme_stats.py,readme_featured.py,readme_card.py: the closingimg.save(OUT, ...)becomespicture_file.save(...); theexcept (OSError, ValueError)handler, its message and the exit code are unchanged — the sentence "Nothing was written." is now true. No other line of the three scripts changed.tests/test_card_picture_staging.py(new, 12 tests): a failed encode leaves the old picture and no staging folder; a symlink keeps its link; a second hard link and a FIFO/folder are refused before the encoder runs; the old mode is carried onto the new picture; a fresh file keeps0666 & ~umask; a staging folder that cannot be removed after the rename only warns; each script publishes through the helper rather than.save(OUT; and two end-to-end redraws with real Pillow (mode kept, old picture replaced; a hard-linked destination refused with the script's own message, no traceback, both names untouched).tools/picture_file.pybeside it:tests/test_card_input_pictures.py,tests/test_gen_tables.py,tests/test_gen_tables_font_redraw.py;tests/test_tools_table.pygains the helper in the set of files that are not reader-facing tools (it has no command line and is linked from nothing), andtests/test_card_out_path.py's "the write at the end is guarded" case now holds the scripts topicture_file.save(instead of.save(OUT.+12 tests, all of which fail against
origin/mainand pass here.Checks
CLEANYOURTV_STRICT=1 tools/venv/bin/python -m unittest discover -s tests→Ran 3221 tests, OK (skipped=37)= main 294d3d5's own 3209 + 12 (the extra skip is the read-only case, which root may write regardless of the mode).tools/venv/bin/python -I tools/gen_tables.py --check→ exit 0 (no picture or table in this PR).tv.json, theme, generated table,PROMPT.md,tools/disable.sh/check.pylogic, Home guard,.github/orlauncher/file touched.Dedupe: 87 open PRs, every file list read through the REST API. Three touch one of the three scripts, and each edits the
STATSconstant at@@ -71,7 +71,7 @@alone — #3040, #2569, #2545, all older card-count PRs; this PR's hunks are the import block at@@ -9,6 +9,9 @@and the closing write at@@ -110,8 +113,10 @@, so no shared line. Nobody touchestests/test_card_picture_staging.pyortools/picture_file.py. Merged history: #714ae253 and #6856949b guarded the picture argument before Pillow opens it and 6664fc3 refused an output path that is one of the script's own inputs — all three act before the draw; nothing changed how the write itself publishes, which is what this PR fixes.