fix: keep attribute preservation best-effort when os.utime fails#180
Open
seonghobae wants to merge 1 commit into
Open
fix: keep attribute preservation best-effort when os.utime fails#180seonghobae wants to merge 1 commit into
seonghobae wants to merge 1 commit into
Conversation
preserve_file_attributes documents best-effort semantics ("failures are
ignored so a completed conversion is not lost"), and guards chmod and the
xattr copy accordingly. The two os.utime calls that restore nanosecond
atime/mtime were unguarded, so an OSError (read-only destination, a
filesystem without timestamp support) propagated up through convert_file
and discarded an otherwise completed conversion — and skipped the macOS
creation-time restore.
Extract the utime restore into a _restore_timestamps helper that swallows
OSError, matching the surrounding best-effort guards. Add a regression
test that patches os.utime to raise and asserts the copy neither raises
nor skips the creation-time step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJRVbDrp1vGYkJgNHMGPpG
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
1 participant
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.
Problem
preserve_file_attributespromises best-effort semantics in its docstring — "Non-critical metadata copy failures are ignored so a completed conversion is not lost" — and guardsos.chmodand the extended-attribute copy accordingly.But the two
os.utimecalls that restore nanosecondatime/mtimewere unguarded. On a destination that rejectsutime(a read-only filesystem, or one without timestamp support)os.utimeraisesOSError, which propagates up throughconvert_fileat thepreserve_file_attributes(source, first_result)call site — discarding an otherwise completed conversion, and skipping the macOS creation-time restore that runs afterward.Fix
Extract the timestamp restore into a
_restore_timestampshelper that swallowsOSError, matching the best-effort guards already used forchmodand xattrs. Both restore points now use it.Test
Added
test_preserve_file_attributes_ignores_utime_error, which patchesos.utimeto raiseOSErrorand asserts thatpreserve_file_attributes:The test fails on the current code (OSError propagates) and passes with the fix.
Gates
coverage report --include=media_shrinker.py: 100%interrogate media_shrinker.py: 100%os.listxattrerrors unchanged (all in_copy_extended_attributestests, untouched here); no new failures.🤖 Generated with Claude Code