From 798bbbc62c11938aeced406e4c2ce60bdcd953ef Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Tue, 28 Feb 2023 17:07:03 -0800 Subject: [PATCH] [BACKEND] Windows support (#476) --- .github/workflows/ci-windows.yaml | 125 ++++++++++++++++++ .github/workflows/ci.yaml | 86 ++++-------- .gitignore | 5 +- README.md | 6 +- src/ytdl_sub/cli/main.py | 48 +------ src/ytdl_sub/config/config_validator.py | 35 ++++- src/ytdl_sub/config/preset_options.py | 16 ++- src/ytdl_sub/downloaders/downloader.py | 2 +- src/ytdl_sub/plugins/nfo_tags.py | 5 +- src/ytdl_sub/plugins/split_by_chapters.py | 5 +- src/ytdl_sub/plugins/subtitles.py | 3 +- .../subscriptions/subscription_download.py | 7 + .../subscription_ytdl_options.py | 2 + src/ytdl_sub/utils/chapters.py | 6 +- src/ytdl_sub/utils/ffmpeg.py | 32 ++++- src/ytdl_sub/utils/file_handler.py | 17 ++- src/ytdl_sub/utils/file_lock.py | 64 +++++++++ src/ytdl_sub/utils/logger.py | 8 ++ src/ytdl_sub/utils/system.py | 3 + src/ytdl_sub/utils/thumbnail.py | 5 +- .../validators/file_path_validators.py | 22 +++ tests/conftest.py | 14 +- tests/e2e/conftest.py | 28 ++-- tests/expected_download.py | 11 +- tests/expected_transaction_log.py | 4 +- tests/resources.py | 6 +- tests/unit/cli/test_working_directory_lock.py | 12 +- tests/unit/utils/test_logger.py | 3 + tests/unit/utils/test_yaml.py | 14 +- 29 files changed, 438 insertions(+), 156 deletions(-) create mode 100644 .github/workflows/ci-windows.yaml create mode 100644 src/ytdl_sub/utils/file_lock.py create mode 100644 src/ytdl_sub/utils/system.py create mode 100644 src/ytdl_sub/validators/file_path_validators.py diff --git a/.github/workflows/ci-windows.yaml b/.github/workflows/ci-windows.yaml new file mode 100644 index 000000000..ebc8ebcbe --- /dev/null +++ b/.github/workflows/ci-windows.yaml @@ -0,0 +1,125 @@ +name: ytld-sub CI (Windows) + +on: + pull_request: + branches: + - master + push: + branches: + - master +jobs: + test-unit: + runs-on: windows-latest + permissions: + contents: read + + steps: + - uses: actions/checkout@v3 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: "3.10" + + - name: Run unit tests with coverage + run: | + curl.exe -L -o ffmpeg.zip https://github.com/yt-dlp/FFmpeg-Builds/releases/download/latest/ffmpeg-master-latest-win64-gpl.zip + tar -xf ffmpeg.zip + move "ffmpeg-master-latest-win64-gpl\bin\ffmpeg.exe" "ffmpeg.exe" + move "ffmpeg-master-latest-win64-gpl\bin\ffprobe.exe" "ffprobe.exe" + + python -m pip install -e .[test] + python -m pytest tests/unit + + test-soundcloud: + runs-on: windows-latest + permissions: + contents: read + + steps: + - uses: actions/checkout@v3 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: "3.10" + + - name: Run e2e soundcloud tests with coverage + run: | + curl.exe -L -o ffmpeg.zip https://github.com/yt-dlp/FFmpeg-Builds/releases/download/latest/ffmpeg-master-latest-win64-gpl.zip + tar -xf ffmpeg.zip + move "ffmpeg-master-latest-win64-gpl\bin\ffmpeg.exe" "ffmpeg.exe" + move "ffmpeg-master-latest-win64-gpl\bin\ffprobe.exe" "ffprobe.exe" + + python -m pip install -e .[test] + python -m pytest tests/e2e/soundcloud + + test-bandcamp: + runs-on: windows-latest + permissions: + contents: read + + steps: + - uses: actions/checkout@v3 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: "3.10" + + - name: Run e2e soundcloud tests with coverage + run: | + curl.exe -L -o ffmpeg.zip https://github.com/yt-dlp/FFmpeg-Builds/releases/download/latest/ffmpeg-master-latest-win64-gpl.zip + tar -xf ffmpeg.zip + move "ffmpeg-master-latest-win64-gpl\bin\ffmpeg.exe" "ffmpeg.exe" + move "ffmpeg-master-latest-win64-gpl\bin\ffprobe.exe" "ffprobe.exe" + + python -m pip install -e .[test] + python -m pytest tests/e2e/bandcamp + + + test-youtube: + runs-on: windows-latest + permissions: + contents: read + + steps: + - uses: actions/checkout@v3 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: "3.10" + + - name: Run e2e youtube tests with coverage + run: | + curl.exe -L -o ffmpeg.zip https://github.com/yt-dlp/FFmpeg-Builds/releases/download/latest/ffmpeg-master-latest-win64-gpl.zip + tar -xf ffmpeg.zip + move "ffmpeg-master-latest-win64-gpl\bin\ffmpeg.exe" "ffmpeg.exe" + move "ffmpeg-master-latest-win64-gpl\bin\ffprobe.exe" "ffprobe.exe" + + python -m pip install -e .[test] + python -m pytest tests/e2e/youtube + + test-plugins: + runs-on: windows-latest + permissions: + contents: read + + steps: + - uses: actions/checkout@v3 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: "3.10" + + - name: Run e2e plugin tests with coverage + run: | + curl.exe -L -o ffmpeg.zip https://github.com/yt-dlp/FFmpeg-Builds/releases/download/latest/ffmpeg-master-latest-win64-gpl.zip + tar -xf ffmpeg.zip + move "ffmpeg-master-latest-win64-gpl\bin\ffmpeg.exe" "ffmpeg.exe" + move "ffmpeg-master-latest-win64-gpl\bin\ffprobe.exe" "ffprobe.exe" + + python -m pip install -e .[test] + python -m pytest tests/e2e/plugins diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 00d2db553..bea651dca 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1,4 +1,4 @@ -name: ytld-sub CI +name: ytld-sub CI (Linux) on: pull_request: @@ -8,70 +8,42 @@ on: branches: - master jobs: - test-build: - runs-on: ubuntu-22.04 - strategy: - matrix: - python-version: ["3.10"] - steps: - - uses: actions/checkout@v3 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python-version }} - - - name: Install dependencies - run: | - python -m venv /opt/env - source /opt/env/bin/activate - pip install -e .[lint,test] - - name: Save Python build cache - uses: actions/cache@v3 - with: - path: /opt/env - key: ${{github.sha}}-env - test-lint: runs-on: ubuntu-22.04 - needs: test-build permissions: contents: read steps: - uses: actions/checkout@v3 - - name: Restore Python build cache - uses: actions/cache@v3 + - name: Set up Python + uses: actions/setup-python@v4 with: - path: /opt/env - key: ${{github.sha}}-env + python-version: "3.10" - name: Run linters run: | - source /opt/env/bin/activate + pip install -e .[lint] make check_lint test-unit: runs-on: ubuntu-22.04 - needs: test-build permissions: contents: read steps: - uses: actions/checkout@v3 - - name: Restore Python build cache - uses: actions/cache@v3 + - name: Set up Python + uses: actions/setup-python@v4 with: - path: /opt/env - key: ${{github.sha}}-env + python-version: "3.10" - name: Run unit tests with coverage run: | + pip install -e .[test] sudo apt-get update sudo apt-get install -y ffmpeg - source /opt/env/bin/activate coverage run -m pytest tests/unit && coverage xml -o /opt/coverage/unit/coverage.xml - name: Save coverage @@ -82,46 +54,42 @@ jobs: test-soundcloud: runs-on: ubuntu-22.04 - needs: test-build permissions: contents: read steps: - uses: actions/checkout@v3 - - name: Restore Python build cache - uses: actions/cache@v3 + - name: Set up Python + uses: actions/setup-python@v4 with: - path: /opt/env - key: ${{github.sha}}-env + python-version: "3.10" - name: Run e2e soundcloud tests with coverage run: | + pip install -e .[test] sudo apt-get update sudo apt-get install -y ffmpeg - source /opt/env/bin/activate coverage run -m pytest tests/e2e/soundcloud && coverage xml -o /opt/coverage/soundcloud/coverage.xml test-bandcamp: runs-on: ubuntu-22.04 - needs: test-build permissions: contents: read steps: - uses: actions/checkout@v3 - - name: Restore Python build cache - uses: actions/cache@v3 + - name: Set up Python + uses: actions/setup-python@v4 with: - path: /opt/env - key: ${{github.sha}}-env + python-version: "3.10" - name: Run e2e soundcloud tests with coverage run: | + pip install -e .[test] sudo apt-get update sudo apt-get install -y ffmpeg - source /opt/env/bin/activate coverage run -m pytest tests/e2e/bandcamp && coverage xml -o /opt/coverage/bandcamp/coverage.xml - name: Save coverage @@ -132,24 +100,22 @@ jobs: test-youtube: runs-on: ubuntu-22.04 - needs: test-build permissions: contents: read steps: - uses: actions/checkout@v3 - - name: Restore Python build cache - uses: actions/cache@v3 + - name: Set up Python + uses: actions/setup-python@v4 with: - path: /opt/env - key: ${{github.sha}}-env + python-version: "3.10" - name: Run e2e youtube tests with coverage run: | + pip install -e .[test] sudo apt-get update sudo apt-get install -y ffmpeg - source /opt/env/bin/activate coverage run -m pytest tests/e2e/youtube && coverage xml -o /opt/coverage/youtube/coverage.xml - name: Save coverage @@ -160,24 +126,22 @@ jobs: test-plugins: runs-on: ubuntu-22.04 - needs: test-build permissions: contents: read steps: - uses: actions/checkout@v3 - - name: Restore Python build cache - uses: actions/cache@v3 + - name: Set up Python + uses: actions/setup-python@v4 with: - path: /opt/env - key: ${{github.sha}}-env + python-version: "3.10" - name: Run e2e plugin tests with coverage run: | + pip install -e .[test] sudo apt-get update sudo apt-get install -y ffmpeg - source /opt/env/bin/activate coverage run -m pytest tests/e2e/plugins && coverage xml -o /opt/coverage/plugins/coverage.xml - name: Save coverage diff --git a/.gitignore b/.gitignore index 42230edf4..4d638c332 100644 --- a/.gitignore +++ b/.gitignore @@ -142,4 +142,7 @@ docker/*.whl docker/root/*.whl docker/root/defaults/examples -.local/ \ No newline at end of file +.local/ + +ffmpeg.exe +ffprobe.exe \ No newline at end of file diff --git a/README.md b/README.md index 3d9a81239..5ab3062c1 100644 --- a/README.md +++ b/README.md @@ -34,11 +34,11 @@ maximum flexibility while maintaining simplicity. ![jelly_mv](https://user-images.githubusercontent.com/10107080/182677256-43aeb029-0c3f-4648-9fd2-352b9666b262.PNG) ### SoundCloud Albums and Singles -#### MusicBee (any file or tag-based music player) +#### MusicBee (any file or tag-based music players) ![sc_mb](https://user-images.githubusercontent.com/10107080/182685415-06adf477-3dd3-475d-bbcd-53b0152b9f0a.PNG) ### Bandcamp Discography -#### Navidrome (any file or tag-based music server) +#### Navidrome (any file or tag-based music servers) ![bc_nav](https://user-images.githubusercontent.com/10107080/212503861-1d8748e6-6f6d-4043-b543-84226cd1f662.png) @@ -285,7 +285,7 @@ docker run -d \ Download and use our latest executable using the command below. For Windows users, use this method in [WSL](https://learn.microsoft.com/en-us/windows/wsl/). FFmpeg is a required dependency. ```commandline -curl -L https://github.com/jmbannon/ytdl-sub/releases/latest/download/ytdl-sub > ytdl-sub +curl -L -o ytdl-sub https://github.com/jmbannon/ytdl-sub/releases/latest/download/ytdl-sub chmod +x ytdl-sub ./ytdl-sub -h ``` diff --git a/src/ytdl_sub/cli/main.py b/src/ytdl_sub/cli/main.py index f38dbe935..f88f74e82 100644 --- a/src/ytdl_sub/cli/main.py +++ b/src/ytdl_sub/cli/main.py @@ -1,11 +1,6 @@ import argparse -import errno -import fcntl import gc -import os import sys -from contextlib import contextmanager -from pathlib import Path from typing import List from typing import Tuple @@ -13,8 +8,8 @@ from ytdl_sub.cli.main_args_parser import parser from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.subscriptions.subscription import Subscription -from ytdl_sub.utils.exceptions import ValidationException from ytdl_sub.utils.file_handler import FileHandlerTransactionLog +from ytdl_sub.utils.file_lock import working_directory_lock from ytdl_sub.utils.logger import Logger logger = Logger.get() @@ -119,45 +114,6 @@ def _view_url_from_cli( return subscription, subscription.download(dry_run=True) -@contextmanager -def _working_directory_lock(config: ConfigFile): - """ - Create and try to lock the file /tmp/working_directory_name - - Raises - ------ - ValidationException - Lock is acquired from another process running ytdl-sub in the same working directory - OSError - Other lock error occurred - """ - working_directory_path = Path(os.getcwd()) / config.config_options.working_directory - lock_file_path = ( - Path(os.getcwd()) - / config.config_options.lock_directory - / str(working_directory_path).replace("/", "_") - ) - - lock_file = open(lock_file_path, "w", encoding="utf-8") - - try: - fcntl.lockf(lock_file, fcntl.LOCK_EX | fcntl.LOCK_NB) - except OSError as exc: - if exc.errno in (errno.EACCES, errno.EAGAIN): - raise ValidationException( - "Cannot run two instances of ytdl-sub " - "with the same working directory at the same time" - ) from exc - lock_file.close() - raise exc - - try: - yield - finally: - fcntl.flock(lock_file, fcntl.LOCK_UN) - lock_file.close() - - def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]: """ Entrypoint for ytdl-sub, without the error handling @@ -172,7 +128,7 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]: config: ConfigFile = ConfigFile.from_file_path(args.config).initialize() transaction_logs: List[Tuple[Subscription, FileHandlerTransactionLog]] = [] - with _working_directory_lock(config=config): + with working_directory_lock(config=config): if args.subparser == "sub": transaction_logs = _download_subscriptions_from_yaml_files(config=config, args=args) diff --git a/src/ytdl_sub/config/config_validator.py b/src/ytdl_sub/config/config_validator.py index 88b13dacb..52dd62c4c 100644 --- a/src/ytdl_sub/config/config_validator.py +++ b/src/ytdl_sub/config/config_validator.py @@ -5,14 +5,24 @@ from mergedeep import mergedeep from ytdl_sub.prebuilt_presets import PREBUILT_PRESETS +from ytdl_sub.utils.system import IS_WINDOWS from ytdl_sub.validators.strict_dict_validator import StrictDictValidator from ytdl_sub.validators.validators import LiteralDictValidator from ytdl_sub.validators.validators import StringValidator +if IS_WINDOWS: + _DEFAULT_LOCK_DIRECTORY = "" # Not supported in Windows + _DEFAULT_FFMPEG_PATH = ".\\ffmpeg.exe" + _DEFAULT_FFPROBE_PATH = ".\\ffprobe.exe" +else: + _DEFAULT_LOCK_DIRECTORY = "/tmp" + _DEFAULT_FFMPEG_PATH = "/usr/bin/ffmpeg" + _DEFAULT_FFPROBE_PATH = "/usr/bin/ffprobe" + class ConfigOptions(StrictDictValidator): _required_keys = {"working_directory"} - _optional_keys = {"umask", "dl_aliases", "lock_directory"} + _optional_keys = {"umask", "dl_aliases", "lock_directory", "ffmpeg_path", "ffprobe_path"} def __init__(self, name: str, value: Any): super().__init__(name, value) @@ -27,7 +37,14 @@ def __init__(self, name: str, value: Any): key="dl_aliases", validator=LiteralDictValidator ) self._lock_directory = self._validate_key( - key="lock_directory", validator=StringValidator, default="/tmp" + key="lock_directory", validator=StringValidator, default=_DEFAULT_LOCK_DIRECTORY + ) + # TODO: Validate these exist + self._ffmpeg_path = self._validate_key( + key="ffmpeg_path", validator=StringValidator, default=_DEFAULT_FFMPEG_PATH + ) + self._ffprobe_path = self._validate_key( + key="ffprobe_path", validator=StringValidator, default=_DEFAULT_FFPROBE_PATH ) @property @@ -82,6 +99,20 @@ def lock_directory(self) -> str: """ return self._lock_directory.value + @property + def ffmpeg_path(self) -> str: + """ + TODO: Fill out! + """ + return self._ffmpeg_path.value + + @property + def ffprobe_path(self) -> str: + """ + TODO: Fill out! + """ + return self._ffprobe_path.value + class ConfigValidator(StrictDictValidator): _required_keys = {"configuration", "presets"} diff --git a/src/ytdl_sub/config/preset_options.py b/src/ytdl_sub/config/preset_options.py index af67d5fb6..e90b46294 100644 --- a/src/ytdl_sub/config/preset_options.py +++ b/src/ytdl_sub/config/preset_options.py @@ -7,6 +7,10 @@ from yt_dlp.utils import sanitize_filename from ytdl_sub.entries.entry import Entry +from ytdl_sub.validators.file_path_validators import ( + OverridesStringFormatterValidatorFilePathValidator, +) +from ytdl_sub.validators.file_path_validators import StringFormatterFilePathValidator from ytdl_sub.validators.strict_dict_validator import StrictDictValidator from ytdl_sub.validators.string_datetime import StringDatetimeValidator from ytdl_sub.validators.string_formatter_validators import DictFormatterValidator @@ -221,19 +225,19 @@ def __init__(self, name, value): # Output directory should resolve without any entry variables. # This is to check the directory for any download-archives before any downloads begin - self._output_directory: OverridesStringFormatterValidator = self._validate_key( - key="output_directory", validator=OverridesStringFormatterValidator + self._output_directory = self._validate_key( + key="output_directory", validator=OverridesStringFormatterValidatorFilePathValidator ) # file name and thumbnails however can use entry variables - self._file_name: StringFormatterValidator = self._validate_key( - key="file_name", validator=StringFormatterValidator + self._file_name = self._validate_key( + key="file_name", validator=StringFormatterFilePathValidator ) self._thumbnail_name = self._validate_key_if_present( - key="thumbnail_name", validator=StringFormatterValidator + key="thumbnail_name", validator=StringFormatterFilePathValidator ) self._info_json_name = self._validate_key_if_present( - key="info_json_name", validator=StringFormatterValidator + key="info_json_name", validator=StringFormatterFilePathValidator ) self._maintain_download_archive = self._validate_key_if_present( diff --git a/src/ytdl_sub/downloaders/downloader.py b/src/ytdl_sub/downloaders/downloader.py index fef211858..5d55c66f2 100644 --- a/src/ytdl_sub/downloaders/downloader.py +++ b/src/ytdl_sub/downloaders/downloader.py @@ -439,7 +439,7 @@ def _separate_download_archives(self, clear_info_json_files: bool = False): if path.endswith(".info.json") ] for info_json_file in info_json_files: - os.remove(info_json_file) + FileHandler.delete(info_json_file) def _extract_entry_info_with_retry(self, entry: Entry) -> Entry: download_entry_dict = self.extract_info_with_retry( diff --git a/src/ytdl_sub/plugins/nfo_tags.py b/src/ytdl_sub/plugins/nfo_tags.py index 4bde7d4c7..cb6df5a9a 100644 --- a/src/ytdl_sub/plugins/nfo_tags.py +++ b/src/ytdl_sub/plugins/nfo_tags.py @@ -16,6 +16,7 @@ from ytdl_sub.utils.xml import to_max_3_byte_utf8_dict from ytdl_sub.utils.xml import to_max_3_byte_utf8_string from ytdl_sub.utils.xml import to_xml +from ytdl_sub.validators.file_path_validators import StringFormatterFilePathValidator from ytdl_sub.validators.nfo_validators import NfoTagsValidator from ytdl_sub.validators.string_formatter_validators import DictFormatterValidator from ytdl_sub.validators.string_formatter_validators import StringFormatterValidator @@ -46,7 +47,7 @@ def __init__(self, name, value): super().__init__(name, value) self._nfo_name = self._validate_key_if_present( - key="nfo_name", validator=StringFormatterValidator + key="nfo_name", validator=StringFormatterFilePathValidator ) self._nfo_root = self._validate_key_if_present( key="nfo_root", validator=StringFormatterValidator @@ -57,7 +58,7 @@ def __init__(self, name, value): ).value @property - def nfo_name(self) -> StringFormatterValidator: + def nfo_name(self) -> StringFormatterFilePathValidator: """ The NFO file name. """ diff --git a/src/ytdl_sub/plugins/split_by_chapters.py b/src/ytdl_sub/plugins/split_by_chapters.py index 5f531c40e..447d6f257 100644 --- a/src/ytdl_sub/plugins/split_by_chapters.py +++ b/src/ytdl_sub/plugins/split_by_chapters.py @@ -168,7 +168,10 @@ def split(self, entry: Entry) -> Optional[List[Tuple[Entry, FileMetadata]]]: if self.is_dry_run: chapters = Chapters.from_entry_chapters(entry=entry) else: - chapters = Chapters.from_embedded_chapters(file_path=entry.get_download_file_path()) + chapters = Chapters.from_embedded_chapters( + ffprobe_path=FFMPEG.ffprobe_path(), + file_path=entry.get_download_file_path(), + ) # If no chapters, do not split anything if not chapters.contains_any_chapters(): diff --git a/src/ytdl_sub/plugins/subtitles.py b/src/ytdl_sub/plugins/subtitles.py index 8fefa973e..c63d87b00 100644 --- a/src/ytdl_sub/plugins/subtitles.py +++ b/src/ytdl_sub/plugins/subtitles.py @@ -11,6 +11,7 @@ from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.file_handler import FileMetadata from ytdl_sub.utils.logger import Logger +from ytdl_sub.validators.file_path_validators import StringFormatterFilePathValidator from ytdl_sub.validators.string_formatter_validators import StringFormatterValidator from ytdl_sub.validators.string_select_validator import StringSelectValidator from ytdl_sub.validators.validators import BoolValidator @@ -58,7 +59,7 @@ class SubtitleOptions(PluginOptions): def __init__(self, name, value): super().__init__(name, value) self._subtitles_name = self._validate_key_if_present( - key="subtitles_name", validator=StringFormatterValidator + key="subtitles_name", validator=StringFormatterFilePathValidator ) self._subtitles_type = self._validate_key_if_present( key="subtitles_type", validator=SubtitlesTypeValidator, default="srt" diff --git a/src/ytdl_sub/subscriptions/subscription_download.py b/src/ytdl_sub/subscriptions/subscription_download.py index 05460ba1b..1e9fb43cc 100644 --- a/src/ytdl_sub/subscriptions/subscription_download.py +++ b/src/ytdl_sub/subscriptions/subscription_download.py @@ -11,6 +11,7 @@ from ytdl_sub.subscriptions.subscription_ytdl_options import SubscriptionYTDLOptions from ytdl_sub.utils.datetime import to_date_range from ytdl_sub.utils.exceptions import ValidationException +from ytdl_sub.utils.ffmpeg import FFMPEG from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.file_handler import FileHandlerTransactionLog from ytdl_sub.utils.file_handler import FileMetadata @@ -265,6 +266,12 @@ def download(self, dry_run: bool = False) -> FileHandlerTransactionLog: If true, do not download any video/audio files or move anything to the output directory. """ + # Set ffmpeg paths + FFMPEG.set_paths( + ffmpeg_path=self._config_options.ffmpeg_path, + ffprobe_path=self._config_options.ffprobe_path, + ) + self._enhanced_download_archive.reinitialize(dry_run=dry_run) plugins = self._initialize_plugins() diff --git a/src/ytdl_sub/subscriptions/subscription_ytdl_options.py b/src/ytdl_sub/subscriptions/subscription_ytdl_options.py index e922f1ede..420e46612 100644 --- a/src/ytdl_sub/subscriptions/subscription_ytdl_options.py +++ b/src/ytdl_sub/subscriptions/subscription_ytdl_options.py @@ -15,6 +15,7 @@ from ytdl_sub.plugins.match_filters import MatchFiltersPlugin from ytdl_sub.plugins.plugin import Plugin from ytdl_sub.plugins.subtitles import SubtitlesPlugin +from ytdl_sub.utils.ffmpeg import FFMPEG from ytdl_sub.ytdl_additions.enhanced_download_archive import EnhancedDownloadArchive PluginT = TypeVar("PluginT", bound=Plugin) @@ -57,6 +58,7 @@ def _global_options(self) -> Dict: "outtmpl": str(Path(self._working_directory) / "%(id)s.%(ext)s"), # Always write thumbnails "writethumbnail": True, + "ffmpeg_location": FFMPEG.ffmpeg_path(), } return ytdl_options diff --git a/src/ytdl_sub/utils/chapters.py b/src/ytdl_sub/utils/chapters.py index 48ab2753d..b35577742 100644 --- a/src/ytdl_sub/utils/chapters.py +++ b/src/ytdl_sub/utils/chapters.py @@ -221,10 +221,12 @@ def from_string(cls, input_str: str) -> "Chapters": return Chapters(timestamps=timestamps, titles=titles) @classmethod - def from_embedded_chapters(cls, file_path: str) -> "Chapters": + def from_embedded_chapters(cls, ffprobe_path: str, file_path: str) -> "Chapters": """ Parameters ---------- + ffprobe_path + Path to ffprobe executable file_path File to read ffmpeg chapter metadata from @@ -234,7 +236,7 @@ def from_embedded_chapters(cls, file_path: str) -> "Chapters": """ proc = subprocess.run( [ - "ffprobe", + ffprobe_path, "-loglevel", "quiet", "-print_format", diff --git a/src/ytdl_sub/utils/ffmpeg.py b/src/ytdl_sub/utils/ffmpeg.py index 2d63de8ed..c67d61ce5 100644 --- a/src/ytdl_sub/utils/ffmpeg.py +++ b/src/ytdl_sub/utils/ffmpeg.py @@ -23,10 +23,31 @@ def _ffmpeg_metadata_escape(str_to_escape: str) -> str: class FFMPEG: + _FFMPEG_PATH: str = "" + _FFPROBE_PATH: str = "" + + @classmethod + def set_paths(cls, ffmpeg_path: str, ffprobe_path: str) -> None: + """Set ffmpeg paths for usage""" + cls._FFMPEG_PATH = ffmpeg_path + cls._FFPROBE_PATH = ffprobe_path + + @classmethod + def ffmpeg_path(cls) -> str: + """Ensure the ffmpeg path has been set and return it""" + assert cls._FFMPEG_PATH, "ffmpeg has not been set" + return cls._FFMPEG_PATH + + @classmethod + def ffprobe_path(cls) -> str: + """Ensure the ffprobe path has been set and return it""" + assert cls._FFPROBE_PATH, "ffprobe has not been set" + return cls._FFPROBE_PATH + @classmethod def _ensure_installed(cls): try: - subprocess.check_output(["which", "ffmpeg"]) + subprocess.check_output([cls.ffmpeg_path(), "-version"]) except subprocess.CalledProcessError as subprocess_error: raise ValidationException( "Trying to use a feature which requires ffmpeg, but it cannot be found" @@ -63,7 +84,7 @@ def run(cls, ffmpeg_args: List[str]) -> None: """ cls._ensure_installed() - cmd = ["ffmpeg"] + cmd = [cls.ffmpeg_path()] cmd.extend(ffmpeg_args) logger.debug("Running %s", " ".join(cmd)) with Logger.handle_external_logs(name="ffmpeg"): @@ -130,10 +151,13 @@ def set_ffmpeg_metadata_chapters( lines += _create_metadata_chapters(chapters=chapters, file_duration_sec=file_duration_sec) tmp_file_path = FFMPEG.tmp_file_path(relative_file_path=file_path) - with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", encoding="utf-8") as metadata_file: + with tempfile.NamedTemporaryFile( + mode="w", suffix=".txt", encoding="utf-8", delete=False + ) as metadata_file: metadata_file.write("\n".join(lines)) metadata_file.flush() + try: FFMPEG.run( [ "-i", @@ -151,6 +175,8 @@ def set_ffmpeg_metadata_chapters( ] ) FileHandler.move(tmp_file_path, file_path) + finally: + FileHandler.delete(metadata_file.name) def add_ffmpeg_metadata_key_values(file_path: str, key_values: Dict[str, str]) -> None: diff --git a/src/ytdl_sub/utils/file_handler.py b/src/ytdl_sub/utils/file_handler.py index fdc8eabd1..be5f1f588 100644 --- a/src/ytdl_sub/utils/file_handler.py +++ b/src/ytdl_sub/utils/file_handler.py @@ -155,6 +155,15 @@ class FileHandlerTransactionLog: Tracks file 'transactions' performed by a FileHandler """ + @classmethod + def format_path_str(cls, path_str: Path | str) -> str: + """ + Returns + ------- + str formatted to always look like a unix string + """ + return str(path_str).replace(os.sep, "/") + def __init__(self): self.files_created: Dict[str, FileMetadata] = {} self.files_modified: Dict[str, FileMetadata] = {} @@ -241,6 +250,10 @@ def _to_output_message( file_directory = os.path.dirname(Path(output_directory) / file_path) file_name = os.path.basename(Path(output_directory) / file_path) + # Format file directories/names to always look like unix + file_directory = cls.format_path_str(file_directory) + file_name = cls.format_path_str(file_name) + directory_set[file_directory][file_name] = file_metadata lines: List[str] = [file_set_title, "-" * 40] @@ -307,7 +320,9 @@ def to_output_message(self, output_directory: str) -> str: ) if self.is_empty: - lines.append(f"No new, modified, or removed files in '{output_directory}'") + lines.append( + f"No new, modified, or removed files in '{self.format_path_str(output_directory)}'" + ) return "\n".join(lines) diff --git a/src/ytdl_sub/utils/file_lock.py b/src/ytdl_sub/utils/file_lock.py new file mode 100644 index 000000000..2e21acfe1 --- /dev/null +++ b/src/ytdl_sub/utils/file_lock.py @@ -0,0 +1,64 @@ +import errno +import os +from contextlib import contextmanager +from pathlib import Path + +from ytdl_sub.config.config_file import ConfigFile +from ytdl_sub.utils.exceptions import ValidationException +from ytdl_sub.utils.logger import Logger +from ytdl_sub.utils.system import IS_WINDOWS + +logger = Logger.get() + +if IS_WINDOWS: + + @contextmanager + def working_directory_lock(config: ConfigFile): + """Windows does not support working directory lock""" + logger.info( + "Working directory lock not supported in Windows. " + "Ensure only one instance of ytdl-sub runs at once using working directory %s", + config.config_options.working_directory, + ) + yield + +else: + import fcntl + + @contextmanager + def working_directory_lock(config: ConfigFile): + """ + Create and try to lock the file /tmp/working_directory_name + + Raises + ------ + ValidationException + Lock is acquired from another process running ytdl-sub in the same working directory + OSError + Other lock error occurred + """ + working_directory_path = Path(os.getcwd()) / config.config_options.working_directory + lock_file_path = ( + Path(os.getcwd()) + / config.config_options.lock_directory + / str(working_directory_path).replace("/", "_") + ) + + lock_file = open(lock_file_path, "w", encoding="utf-8") + + try: + fcntl.lockf(lock_file, fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError as exc: + if exc.errno in (errno.EACCES, errno.EAGAIN): + raise ValidationException( + "Cannot run two instances of ytdl-sub " + "with the same working directory at the same time" + ) from exc + lock_file.close() + raise exc + + try: + yield + finally: + fcntl.flock(lock_file, fcntl.LOCK_UN) + lock_file.close() diff --git a/src/ytdl_sub/utils/logger.py b/src/ytdl_sub/utils/logger.py index 2d811de30..095c86aa1 100644 --- a/src/ytdl_sub/utils/logger.py +++ b/src/ytdl_sub/utils/logger.py @@ -90,6 +90,9 @@ class Logger: _DEBUG_LOGGER_FILE = tempfile.NamedTemporaryFile(prefix="ytdl-sub.", delete=False) # pylint: enable=R1732 + # Keep track of all Loggers created + _LOGGERS: List[logging.Logger] = [] + @classmethod def debug_log_filename(cls) -> str: """ @@ -151,6 +154,7 @@ def _get( if debug_file: logger.addHandler(cls._get_debug_file_handler()) + cls._LOGGERS.append(logger) return logger @classmethod @@ -200,6 +204,10 @@ def cleanup(cls, delete_debug_file: bool = True): delete_debug_file Whether to delete the debug log file. Defaults to True. """ + for logger in cls._LOGGERS: + for handler in logger.handlers: + handler.close() + cls._DEBUG_LOGGER_FILE.close() if delete_debug_file: diff --git a/src/ytdl_sub/utils/system.py b/src/ytdl_sub/utils/system.py new file mode 100644 index 000000000..eec08c900 --- /dev/null +++ b/src/ytdl_sub/utils/system.py @@ -0,0 +1,3 @@ +import sys + +IS_WINDOWS = sys.platform.startswith("win32") diff --git a/src/ytdl_sub/utils/thumbnail.py b/src/ytdl_sub/utils/thumbnail.py index 9ba26244c..db5597cfe 100644 --- a/src/ytdl_sub/utils/thumbnail.py +++ b/src/ytdl_sub/utils/thumbnail.py @@ -64,9 +64,10 @@ def convert_url_thumbnail(thumbnail_url: str, output_thumbnail_path: str) -> Opt """ # timeout after 8 seconds with urlopen(thumbnail_url, timeout=1.0) as file: - with tempfile.NamedTemporaryFile() as thumbnail: + with tempfile.NamedTemporaryFile(delete=False) as thumbnail: thumbnail.write(file.read()) + try: os.makedirs(os.path.dirname(output_thumbnail_path), exist_ok=True) tmp_output_path = FFMPEG.tmp_file_path( @@ -76,6 +77,8 @@ def convert_url_thumbnail(thumbnail_url: str, output_thumbnail_path: str) -> Opt # Have FileHandler handle the move to a potential cross-device FileHandler.move(tmp_output_path, output_thumbnail_path) + finally: FileHandler.delete(tmp_output_path) + FileHandler.delete(thumbnail.name) return True diff --git a/src/ytdl_sub/validators/file_path_validators.py b/src/ytdl_sub/validators/file_path_validators.py new file mode 100644 index 000000000..0ba4e070e --- /dev/null +++ b/src/ytdl_sub/validators/file_path_validators.py @@ -0,0 +1,22 @@ +import os +from pathlib import Path +from typing import Dict + +from ytdl_sub.validators.string_formatter_validators import OverridesStringFormatterValidator +from ytdl_sub.validators.string_formatter_validators import StringFormatterValidator + + +class StringFormatterFilePathValidator(StringFormatterValidator): + _expected_value_type_name = "filepath" + + def apply_formatter(self, variable_dict: Dict[str, str]) -> str: + """Turn into a Path, then a string, to get correct directory separators""" + return str(Path(super().apply_formatter(variable_dict))) + + +class OverridesStringFormatterValidatorFilePathValidator(OverridesStringFormatterValidator): + _expected_value_type_name = "static filepath" + + def apply_formatter(self, variable_dict: Dict[str, str]) -> str: + """Turn into a Path, then a string, to get correct directory separators""" + return os.path.realpath(super().apply_formatter(variable_dict)) diff --git a/tests/conftest.py b/tests/conftest.py index f1c1d589a..a38e8c03e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,9 @@ import contextlib import json import logging +import os import tempfile +from pathlib import Path from typing import Any from typing import Callable from typing import Dict @@ -10,17 +12,18 @@ import pytest +from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.logger import Logger @pytest.fixture -def working_directory() -> str: +def working_directory() -> Path: with tempfile.TemporaryDirectory() as temp_dir: yield temp_dir @pytest.fixture() -def output_directory(): +def output_directory() -> Path: with tempfile.TemporaryDirectory() as temp_dir: yield temp_dir @@ -83,9 +86,12 @@ def preset_dict_to_subscription_yaml_generator() -> Callable: @contextlib.contextmanager def _preset_dict_to_subscription_yaml_generator(subscription_name: str, preset_dict: Dict): subscription_dict = {subscription_name: preset_dict} - with tempfile.NamedTemporaryFile(suffix=".yaml") as tmp_file: + with tempfile.NamedTemporaryFile(suffix=".yaml", delete=False) as tmp_file: tmp_file.write(json.dumps(subscription_dict).encode("utf-8")) - tmp_file.flush() + + try: yield tmp_file.name + finally: + FileHandler.delete(tmp_file.name) return _preset_dict_to_subscription_yaml_generator diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index 1e387f0fa..e307abd57 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -3,6 +3,7 @@ import shutil import sys import tempfile +from pathlib import Path from typing import List from typing import Tuple from unittest.mock import patch @@ -14,6 +15,7 @@ from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.subscriptions.subscription import Subscription from ytdl_sub.subscriptions.subscription_download import SubscriptionDownload +from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.file_handler import FileHandlerTransactionLog from ytdl_sub.utils.logger import Logger from ytdl_sub.utils.yaml import load_yaml @@ -48,11 +50,11 @@ def _assert_working_directory_empty(self, is_error: bool = False): @pytest.fixture() -def music_video_config_path(): - return "examples/music_videos_config.yaml" +def music_video_config_path() -> Path: + return Path("examples/music_videos_config.yaml") -def _load_config(config_path: str, working_directory: str) -> ConfigFile: +def _load_config(config_path: Path, working_directory: Path) -> ConfigFile: config_dict = load_yaml(file_path=config_path) config_dict["configuration"]["working_directory"] = working_directory @@ -66,23 +68,26 @@ def music_video_config(music_video_config_path, working_directory) -> ConfigFile @pytest.fixture() def music_video_config_for_cli(music_video_config) -> str: - with tempfile.NamedTemporaryFile(suffix=".yaml") as tmp_file: + with tempfile.NamedTemporaryFile(suffix=".yaml", delete=False) as tmp_file: tmp_file.write(json.dumps(music_video_config._value).encode("utf-8")) - tmp_file.flush() + + try: yield tmp_file.name + finally: + FileHandler.delete(tmp_file.name) @pytest.fixture() def channel_as_tv_show_config(working_directory) -> ConfigFile: return _load_config( - config_path="examples/tv_show_config.yaml", working_directory=working_directory + config_path=Path("examples/tv_show_config.yaml"), working_directory=working_directory ) @pytest.fixture() def music_audio_config(working_directory) -> ConfigFile: return _load_config( - config_path="examples/music_audio_config.yaml", working_directory=working_directory + config_path=Path("examples/music_audio_config.yaml"), working_directory=working_directory ) @@ -97,10 +102,15 @@ def timestamps_file_path(): "00:01:01 Part 5\n", ] - with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8", suffix=".txt") as tmp: + with tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", suffix=".txt", delete=False + ) as tmp: tmp.writelines(timestamps) - tmp.seek(0) + + try: yield tmp.name + finally: + FileHandler.delete(tmp.name) def mock_run_from_cli(args: str) -> List[Tuple[Subscription, FileHandlerTransactionLog]]: diff --git a/tests/expected_download.py b/tests/expected_download.py index c2faacbfa..ab53baed7 100644 --- a/tests/expected_download.py +++ b/tests/expected_download.py @@ -1,5 +1,6 @@ import json import os.path +import sys from dataclasses import dataclass from pathlib import Path from typing import List @@ -9,15 +10,20 @@ from resources import RESOURCE_PATH from ytdl_sub.utils.file_handler import get_file_md5_hash +from ytdl_sub.utils.system import IS_WINDOWS _EXPECTED_DOWNLOADS_SUMMARY_PATH = RESOURCE_PATH / "expected_downloads_summaries" def _get_files_in_directory(relative_directory: Path | str) -> List[Path]: + relative_path_part_idx = 3 # Cuts /tmp/ + if IS_WINDOWS: + relative_path_part_idx = 7 # Cuts C:\Users\\AppData\Local\Temp\ + relative_file_paths: List[Path] = [] for path in Path(relative_directory).rglob("*"): if path.is_file(): - relative_path = Path(*path.parts[3:]) + relative_path = Path(*path.parts[relative_path_part_idx:]) relative_file_paths.append(relative_path) return relative_file_paths @@ -70,7 +76,8 @@ def assert_files_exist( full_path = Path(relative_directory) / path assert os.path.isfile(full_path), f"Expected {path} to be a file but it is not" - if path in ignore_md5_hashes_for or path.endswith(".info.json"): + # TODO: Implement file hash for tests in Windows + if IS_WINDOWS or path in ignore_md5_hashes_for or path.endswith(".info.json"): continue md5_hash = get_file_md5_hash(full_file_path=full_path) diff --git a/tests/expected_transaction_log.py b/tests/expected_transaction_log.py index 55e081d3b..75646f6ff 100644 --- a/tests/expected_transaction_log.py +++ b/tests/expected_transaction_log.py @@ -43,7 +43,9 @@ def assert_transaction_log_matches( # Read the expected summary file with open(transaction_log_path, "r", encoding="utf-8") as summary_file: - expected_summary = summary_file.read().format(output_directory=output_directory) + expected_summary = summary_file.read().format( + output_directory=FileHandlerTransactionLog.format_path_str(output_directory) + ) # Split, ensure there are the same number of new lines summary_lines: List[str] = summary.split("\n") diff --git a/tests/resources.py b/tests/resources.py index c867bb71e..911b330ae 100644 --- a/tests/resources.py +++ b/tests/resources.py @@ -3,9 +3,9 @@ REGENERATE_FIXTURES: bool = False -RESOURCE_PATH = Path("tests/resources") -_FILE_FIXTURE_PATH = RESOURCE_PATH / "file_fixtures" +RESOURCE_PATH: Path = Path("tests") / "resources" +_FILE_FIXTURE_PATH: Path = RESOURCE_PATH / "file_fixtures" -def copy_file_fixture(fixture_name: str, output_file_path: str | Path) -> None: +def copy_file_fixture(fixture_name: str, output_file_path: Path) -> None: shutil.copy(_FILE_FIXTURE_PATH / fixture_name, output_file_path) diff --git a/tests/unit/cli/test_working_directory_lock.py b/tests/unit/cli/test_working_directory_lock.py index 317004580..8787ac785 100644 --- a/tests/unit/cli/test_working_directory_lock.py +++ b/tests/unit/cli/test_working_directory_lock.py @@ -3,9 +3,10 @@ import pytest -from ytdl_sub.cli.main import _working_directory_lock from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.utils.exceptions import ValidationException +from ytdl_sub.utils.file_lock import working_directory_lock +from ytdl_sub.utils.system import IS_WINDOWS @pytest.fixture @@ -16,17 +17,20 @@ def config() -> ConfigFile: def test_working_directory_lock(config: ConfigFile): + if IS_WINDOWS: + return + new_pid = os.fork() if new_pid == 0: # is child - with _working_directory_lock(config=config): + with working_directory_lock(config=config): time.sleep(3) return time.sleep(1) with pytest.raises(ValidationException, match="Cannot run two instances of ytdl-sub"): - with _working_directory_lock(config=config): + with working_directory_lock(config=config): time.sleep(1) time.sleep(3) - with _working_directory_lock(config=config): + with working_directory_lock(config=config): pass diff --git a/tests/unit/utils/test_logger.py b/tests/unit/utils/test_logger.py index e7a5fe516..d46534bda 100644 --- a/tests/unit/utils/test_logger.py +++ b/tests/unit/utils/test_logger.py @@ -95,6 +95,9 @@ def test_logger_always_outputs_to_debug_file(self, log_level): assert lines == ["[ytdl-sub:name_test] info test\n", "[ytdl-sub:name_test] debug test\n"] # Ensure the file cleans up too + for handler in logger.handlers: + handler.close() + Logger.cleanup(delete_debug_file=True) assert not os.path.isfile(Logger._DEBUG_LOGGER_FILE.name) diff --git a/tests/unit/utils/test_yaml.py b/tests/unit/utils/test_yaml.py index 1f257745e..b61cc413b 100644 --- a/tests/unit/utils/test_yaml.py +++ b/tests/unit/utils/test_yaml.py @@ -1,9 +1,12 @@ +import os +import re import tempfile import pytest from ytdl_sub.utils.exceptions import FileNotFoundException from ytdl_sub.utils.exceptions import InvalidYamlException +from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.yaml import load_yaml @@ -21,10 +24,15 @@ def bad_yaml() -> str: @pytest.fixture def bad_yaml_file_path(bad_yaml) -> str: - with tempfile.NamedTemporaryFile(suffix=".yaml") as tmp_file: + # Do not delete the file in the context manager - for Windows compatibility + with tempfile.NamedTemporaryFile(suffix=".yaml", delete=False) as tmp_file: tmp_file.write(bad_yaml.encode("utf-8")) tmp_file.flush() + + try: yield tmp_file.name + finally: + FileHandler.delete(tmp_file.name) def test_load_yaml_file_not_found(): @@ -36,6 +44,8 @@ def test_load_yaml_file_not_found(): def test_load_yaml_invalid_syntax(bad_yaml_file_path): with pytest.raises( InvalidYamlException, - match=f"'{bad_yaml_file_path}' has invalid YAML, copy-paste it into a YAML checker to find the issue.", + match=re.escape( + f"'{bad_yaml_file_path}' has invalid YAML, copy-paste it into a YAML checker to find the issue." + ), ): load_yaml(file_path=bad_yaml_file_path)