tehkillerbee / mopidy-tidal

Tidal Backend plugin for Mopidy
Apache License 2.0
93 stars 28 forks source link

Filenames with colon break Mopidy-Tidal on fat32 formatted partition #93

Closed grasdk closed 1 year ago

grasdk commented 2 years ago

Cache filenames are stored by Mopidy-Tidal using colons.

Anonymised example: xyz/mopidy/tidal/track/61/tidal:track:1234567:123456789:123456789.cache

If your mopidy cache directory is on a fat32 formatted volume, such filenames are not allowed and thus the cache files are not written, causing Mopidy-Tidal to fail with an exception:

OSError: [Errno 22] Invalid argument: '/mnt/FAT32PART/mopidy-cache/mopidy/tidal/track/61/tidal:track:1234567:123456789:123456789.cache'
ERROR    2022-10-09 17:06:26,989 [1831:Core-9 (_actor_loop)] mopidy.core.library
  TidalBackend backend caused an exception.
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/mopidy/core/library.py", line 17, in _backend_error_handling
    yield
  File "/usr/lib/python3/dist-packages/mopidy/core/library.py", line 230, in lookup
    result = future.get()
  File "/usr/lib/python3/dist-packages/pykka/_threading.py", line 45, in get
    _compat.reraise(*self._data['exc_info'])
  File "/usr/lib/python3/dist-packages/pykka/_compat/__init__.py", line 29, in reraise
    raise value
  File "/usr/lib/python3/dist-packages/pykka/_actor.py", line 193, in _actor_loop
    response = self._handle_receive(envelope.message)
  File "/usr/lib/python3/dist-packages/pykka/_actor.py", line 299, in _handle_receive
    return callee(*message.args, **message.kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/mopidy_tidal/library.py", line 312, in lookup
    getattr(self, cache_name).update(new_data)
  File "/home/user/.local/lib/python3.10/site-packages/mopidy_tidal/lru_cache.py", line 136, in update
    super().update(*args, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/mopidy_tidal/lru_cache.py", line 100, in __setitem__
    with open(cache_file, "wb") as f:
OSError: [Errno 22] Invalid argument: '/mnt/FAT32PART/mopidy-cache/mopidy/tidal/track/61/tidal:track:1234567:123456789:123456789.cache'
tehkillerbee commented 2 years ago

@grasdk Good catch. It would probably be a good idea to use a different character, eg. "_" or similar. Any thoughts @BlackLight ?

blacklight commented 2 years ago

Any thoughts

Maybe just don't use FAT32 for any data storage in 2022? 😆

Jokes aside, I can easily put together a PR that replaces : with - or something else for the cache filenames, but this should probably be done in a way that is back-compatible - i.e. it should not skip existing cache files with :.

tehkillerbee commented 2 years ago

@BlackLight Looks like a good solution. :+1:

I agree, FAT32 on a Linux system is not really a supported use-case so I would not be surprised if other functionality does not work correctly.

grasdk commented 2 years ago

I will try this fix out, when it's available. It is a bit weird, I agree ;-)