Skip to content

Commit

Permalink
PICARD-3013: Fix case-only renaming on case-insensitive filesystems
Browse files Browse the repository at this point in the history
  • Loading branch information
phw committed Dec 28, 2024
1 parent b6cadf3 commit dabaf68
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 24 deletions.
2 changes: 1 addition & 1 deletion picard/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ def make_filename(self, filename, metadata, settings=None, naming_format=None):
new_filename = self._format_filename(new_dirname, new_filename, metadata, settings, naming_format)

new_path = os.path.join(new_dirname, new_filename)
return normpath(new_path)
return normpath(new_path, realpath=False)

def _rename(self, old_filename, metadata, settings=None):
new_filename = self.make_filename(old_filename, metadata, settings)
Expand Down
19 changes: 10 additions & 9 deletions picard/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#
# Copyright (C) 2004 Robert Kaye
# Copyright (C) 2006-2009, 2011-2012, 2014 Lukáš Lalinský
# Copyright (C) 2008-2011, 2014, 2018-2023 Philipp Wolfer
# Copyright (C) 2008-2011, 2014, 2018-2024 Philipp Wolfer
# Copyright (C) 2009 Carlin Mangar
# Copyright (C) 2009 david
# Copyright (C) 2010 fatih
Expand Down Expand Up @@ -254,15 +254,16 @@ def system_supports_long_paths():
return False


def normpath(path):
def normpath(path, realpath=True):
path = os.path.normpath(path)
try:
path = os.path.realpath(path)
except OSError as why:
# realpath can fail if path does not exist or is not accessible
# or on Windows if drives are mounted without mount manager
# (see https://tickets.metabrainz.org/browse/PICARD-2425).
log.warning("Failed getting realpath for `%s`: %s", path, why)
if realpath:
try:
path = os.path.realpath(path)
except OSError as why:
# realpath can fail if path does not exist or is not accessible
# or on Windows if drives are mounted without mount manager
# (see https://tickets.metabrainz.org/browse/PICARD-2425).
log.warning("Failed getting realpath for `%s`: %s", path, why)
# If the path is longer than 259 characters on Windows, prepend the \\?\
# prefix. This enables access to long paths using the Windows API. See
# https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
Expand Down
28 changes: 14 additions & 14 deletions test/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#
# Picard, the next-generation MusicBrainz tagger
#
# Copyright (C) 2018-2022 Philipp Wolfer
# Copyright (C) 2018-2022, 2024 Philipp Wolfer
# Copyright (C) 2019-2022 Laurent Monin
# Copyright (C) 2021 Bob Swift
# Copyright (C) 2021 Sophist-UK
Expand Down Expand Up @@ -225,66 +225,66 @@ def setUp(self):

def test_make_filename_no_move_and_rename(self):
filename = self.file.make_filename(self.file.filename, self.metadata)
self.assertEqual(os.path.realpath(self.file.filename), filename)
self.assertEqual(os.path.normpath(self.file.filename), filename)

def test_make_filename_rename_only(self):
config.setting['rename_files'] = True
filename = self.file.make_filename(self.file.filename, self.metadata)
self.assertEqual(os.path.realpath('/somepath/sometitle.mp3'), filename)
self.assertEqual(os.path.normpath('/somepath/sometitle.mp3'), filename)

def test_make_filename_move_only(self):
config.setting['move_files'] = True
filename = self.file.make_filename(self.file.filename, self.metadata)
self.assertEqual(
os.path.realpath('/media/music/somealbum/somefile.mp3'),
os.path.normpath('/media/music/somealbum/somefile.mp3'),
filename)

def test_make_filename_move_and_rename(self):
config.setting['rename_files'] = True
config.setting['move_files'] = True
filename = self.file.make_filename(self.file.filename, self.metadata)
self.assertEqual(
os.path.realpath('/media/music/somealbum/sometitle.mp3'),
os.path.normpath('/media/music/somealbum/sometitle.mp3'),
filename)

def test_make_filename_move_relative_path(self):
config.setting['move_files'] = True
config.setting['move_files_to'] = 'subdir'
filename = self.file.make_filename(self.file.filename, self.metadata)
self.assertEqual(
os.path.realpath('/somepath/subdir/somealbum/somefile.mp3'),
os.path.normpath('/somepath/subdir/somealbum/somefile.mp3'),
filename)

def test_make_filename_empty_script(self):
config.setting['rename_files'] = True
config.setting['file_renaming_scripts'] = {'test_id': {'script': '$noop()'}}
filename = self.file.make_filename(self.file.filename, self.metadata)
self.assertEqual(os.path.realpath('/somepath/somefile.mp3'), filename)
self.assertEqual(os.path.normpath('/somepath/somefile.mp3'), filename)

def test_make_filename_empty_basename(self):
config.setting['move_files'] = True
config.setting['rename_files'] = True
config.setting['file_renaming_scripts'] = {'test_id': {'script': '/somedir/$noop()'}}
filename = self.file.make_filename(self.file.filename, self.metadata)
self.assertEqual(os.path.realpath('/media/music/somedir/somefile.mp3'), filename)
self.assertEqual(os.path.normpath('/media/music/somedir/somefile.mp3'), filename)

def test_make_filename_no_extension(self):
config.setting['rename_files'] = True
file_ = FakeMp3File('/somepath/_')
filename = file_.make_filename(file_.filename, self.metadata)
self.assertEqual(os.path.realpath('/somepath/sometitle.mp3'), filename)
self.assertEqual(os.path.normpath('/somepath/sometitle.mp3'), filename)

def test_make_filename_lowercase_extension(self):
config.setting['rename_files'] = True
file_ = FakeMp3File('/somepath/somefile.MP3')
filename = file_.make_filename(file_.filename, self.metadata)
self.assertEqual(os.path.realpath('/somepath/sometitle.mp3'), filename)
self.assertEqual(os.path.normpath('/somepath/sometitle.mp3'), filename)

def test_make_filename_scripted_extension(self):
config.setting['rename_files'] = True
config.setting['file_renaming_scripts'] = {'test_id': {'script': '$set(_extension,.foo)%title%'}}
filename = self.file.make_filename(self.file.filename, self.metadata)
self.assertEqual(os.path.realpath('/somepath/sometitle.foo'), filename)
self.assertEqual(os.path.normpath('/somepath/sometitle.foo'), filename)

def test_make_filename_replace_trailing_dots(self):
config.setting['rename_files'] = True
Expand All @@ -296,7 +296,7 @@ def test_make_filename_replace_trailing_dots(self):
})
filename = self.file.make_filename(self.file.filename, metadata)
self.assertEqual(
os.path.realpath('/media/music/somealbum_/sometitle.mp3'),
os.path.normpath('/media/music/somealbum_/sometitle.mp3'),
filename)

@unittest.skipUnless(not IS_WIN, "non-windows test")
Expand All @@ -310,7 +310,7 @@ def test_make_filename_keep_trailing_dots(self):
})
filename = self.file.make_filename(self.file.filename, metadata)
self.assertEqual(
os.path.realpath('/media/music/somealbum./sometitle.mp3'),
os.path.normpath('/media/music/somealbum./sometitle.mp3'),
filename)

def test_make_filename_replace_leading_dots(self):
Expand All @@ -323,7 +323,7 @@ def test_make_filename_replace_leading_dots(self):
})
filename = self.file.make_filename(self.file.filename, metadata)
self.assertEqual(
os.path.realpath('/media/music/_somealbum/_sometitle.mp3'),
os.path.normpath('/media/music/_somealbum/_sometitle.mp3'),
filename)


Expand Down

0 comments on commit dabaf68

Please sign in to comment.