Use os.replace where applicable (#793)

When using 
```py
os.remove(encodeFilename(filename))
os.rename(encodeFilename(temp_filename), encodeFilename(filename))
```
the `os.remove` need not be atomic and so can be executed arbitrarily compared to the immediately following rename call. It is better to use `os.replace` instead

Authored by: paulwrubel
This commit is contained in:
Paul Wrubel 2021-08-26 21:27:20 -05:00 committed by GitHub
parent 691d5823d6
commit d75201a873
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 10 additions and 18 deletions

2
.gitignore vendored
View file

@ -19,6 +19,8 @@ cookies.txt
*.wav *.wav
*.ape *.ape
*.mkv *.mkv
*.flac
*.avi
*.swf *.swf
*.part *.part
*.part-* *.part-*

View file

@ -207,12 +207,9 @@ def try_rename(self, old_filename, new_filename):
if old_filename == new_filename: if old_filename == new_filename:
return return
try: try:
if self.params.get('overwrites', False): os.replace(old_filename, new_filename)
if os.path.isfile(encodeFilename(new_filename)):
os.remove(encodeFilename(new_filename))
os.rename(encodeFilename(old_filename), encodeFilename(new_filename))
except (IOError, OSError) as err: except (IOError, OSError) as err:
self.report_error('unable to rename file: %s' % error_to_compat_str(err)) self.report_error(f'unable to rename file: {err}')
def try_utime(self, filename, last_modified_hdr): def try_utime(self, filename, last_modified_hdr):
"""Try to set the last-modified time of the given file.""" """Try to set the last-modified time of the given file."""

View file

@ -222,8 +222,7 @@ def run(self, info):
raise EmbedThumbnailPPError('Supported filetypes for thumbnail embedding are: mp3, mkv/mka, ogg/opus/flac, m4a/mp4/mov') raise EmbedThumbnailPPError('Supported filetypes for thumbnail embedding are: mp3, mkv/mka, ogg/opus/flac, m4a/mp4/mov')
if success and temp_filename != filename: if success and temp_filename != filename:
os.remove(encodeFilename(filename)) os.replace(temp_filename, filename)
os.rename(encodeFilename(temp_filename), encodeFilename(filename))
self.try_utime(filename, mtime, mtime) self.try_utime(filename, mtime, mtime)

View file

@ -520,8 +520,7 @@ def run(self, information):
temp_filename = prepend_extension(filename, 'temp') temp_filename = prepend_extension(filename, 'temp')
self.to_screen('Embedding subtitles in "%s"' % filename) self.to_screen('Embedding subtitles in "%s"' % filename)
self.run_ffmpeg_multiple_files(input_files, temp_filename, opts) self.run_ffmpeg_multiple_files(input_files, temp_filename, opts)
os.remove(encodeFilename(filename)) os.replace(temp_filename, filename)
os.rename(encodeFilename(temp_filename), encodeFilename(filename))
files_to_delete = [] if self._already_have_subtitle else sub_filenames files_to_delete = [] if self._already_have_subtitle else sub_filenames
return files_to_delete, information return files_to_delete, information
@ -628,8 +627,7 @@ def ffmpeg_escape(text):
itertools.chain(self._options(info['ext']), *options)) itertools.chain(self._options(info['ext']), *options))
if chapters: if chapters:
os.remove(metadata_filename) os.remove(metadata_filename)
os.remove(encodeFilename(filename)) os.replace(temp_filename, filename)
os.rename(encodeFilename(temp_filename), encodeFilename(filename))
return [], info return [], info
@ -673,8 +671,7 @@ def _fixup(self, msg, filename, options):
self.to_screen(f'{msg} of "{filename}"') self.to_screen(f'{msg} of "{filename}"')
self.run_ffmpeg(filename, temp_filename, options) self.run_ffmpeg(filename, temp_filename, options)
os.remove(encodeFilename(filename)) os.replace(temp_filename, filename)
os.rename(encodeFilename(temp_filename), encodeFilename(filename))
class FFmpegFixupStretchedPP(FFmpegFixupPostProcessor): class FFmpegFixupStretchedPP(FFmpegFixupPostProcessor):
@ -866,9 +863,7 @@ def fixup_webp(self, info, idx=-1):
if thumbnail_ext != 'webp' and self.is_webp(thumbnail_filename): if thumbnail_ext != 'webp' and self.is_webp(thumbnail_filename):
self.to_screen('Correcting thumbnail "%s" extension to webp' % thumbnail_filename) self.to_screen('Correcting thumbnail "%s" extension to webp' % thumbnail_filename)
webp_filename = replace_extension(thumbnail_filename, 'webp') webp_filename = replace_extension(thumbnail_filename, 'webp')
if os.path.exists(webp_filename): os.replace(thumbnail_filename, webp_filename)
os.remove(webp_filename)
os.rename(encodeFilename(thumbnail_filename), encodeFilename(webp_filename))
info['thumbnails'][idx]['filepath'] = webp_filename info['thumbnails'][idx]['filepath'] = webp_filename
info['__files_to_move'][webp_filename] = replace_extension( info['__files_to_move'][webp_filename] = replace_extension(
info['__files_to_move'].pop(thumbnail_filename), 'webp') info['__files_to_move'].pop(thumbnail_filename), 'webp')

View file

@ -84,8 +84,7 @@ def run(self, information):
stdout = process_communicate_or_kill(p)[0] stdout = process_communicate_or_kill(p)[0]
if p.returncode == 0: if p.returncode == 0:
os.remove(encodeFilename(filename)) os.replace(temp_filename, filename)
os.rename(encodeFilename(temp_filename), encodeFilename(filename))
self.to_screen('Sponsor sections have been %s' % ('removed' if self.cutout else 'marked')) self.to_screen('Sponsor sections have been %s' % ('removed' if self.cutout else 'marked'))
elif p.returncode == 3: elif p.returncode == 3:
self.to_screen('No segments in the SponsorBlock database') self.to_screen('No segments in the SponsorBlock database')