Skip to content

feat: add 'system' yt-dlp source (PR2 for #182) - #186

Merged
firsttris merged 3 commits into
firsttris:masterfrom
zibaone:feat/ytdlp-system-source
Oct 4, 2026
Merged

firsttris merged 3 commits into
firsttris:masterfrom
zibaone:feat/ytdlp-system-source

Conversation

@zibaone

@zibaone zibaone commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Context

Follow-up to #182 and #185 (merged). Adds system as a third value of ytdlp_source.

What it does

system uses a yt-dlp that is already importable by Kodi's Python. The addon downloads nothing — it only asks whether yt_dlp can be imported. Where the package lives is a packaging concern, resolved the same way deno_manager finds a system binary (shutil.which, no path setting).

Design decisions

  • No path setting. From the addon's point of view the library is importable or it isn't. If system doesn't detect the library, that's a packaging problem — the package and its dependencies must be put where Kodi's Python can import them. The addon has no business knowing or configuring filesystem paths. This is a deliberate trade-off against the single-enum approach: a path setting would be a second configuration axis, but the alternative (silent failure) is worse.
  • Fallback to stable. If system is chosen but no yt_dlp is importable, the addon reverts ytdlp_source to stable and shows a notification. This avoids a silent failure and ensures the addon continues to work.
  • No auto default. On most platforms (LibreELEC, Windows, Android) nothing is importable, so silently skipping the download would be the wrong default. system is an explicit choice.

Changes

  • core/ytdlp_manager.py: YTDLP_SOURCE_SYSTEM, is_managed_source(), resolve_system_ytdlp(), early returns in ensure_ytdlp_ready() and get_runtime_status(), guard in list_available_versions()
  • core/runtime/actions.py: fallback to stable with notification when system has no importable library
  • resources/settings.xml: third option system
  • resources/language/.../strings.po: label #32072
  • README.md: documents the importable-dependencies requirement
  • tests/test_ytdlp_system_source.py: 9 new tests

Testing

  • 294 tests pass (285 existing + 9 new)
  • E2E validated: yt_dlp from Nix store detected, ready=True, runtime_path=None
  • Kodi headless tested: notification + fallback confirmed

Notes

Third value of ytdlp_source: the addon downloads nothing and only asks
whether yt_dlp is importable. Where the package lives is a packaging
concern, resolved the same way deno_manager finds a system binary
(shutil.which, no path setting).

- is_managed_source() / resolve_system_ytdlp() in ytdlp_manager
- early return in ensure_ytdlp_ready() and get_runtime_status()
- explicit error notification when 'system' is chosen with nothing
  importable (the entry point already refuses to resolve in that case)
- settings.xml option + strings.po label + README note on the
  importable-dependencies requirement

9 new tests, 294 passed.
When ytdlp_source is 'system' but no yt_dlp package is importable,
revert the setting to 'stable' and notify the user. This avoids a
silent failure and ensures the addon continues to work.

Also fix list_available_versions to return [] for system source
instead of raising KeyError.
@zibaone
zibaone marked this pull request as ready for review October 3, 2026 20:26
@codecov-commenter

codecov-commenter commented Oct 3, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 96.87500% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.69%. Comparing base (413281c) to head (f0a4471).

Files with missing lines Patch % Lines
core/ytdlp_manager.py 95.65% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #186      +/-   ##
==========================================
+ Coverage   72.79%   74.69%   +1.89%     
==========================================
  Files          14       14              
  Lines        2334     2363      +29     
==========================================
+ Hits         1699     1765      +66     
+ Misses        635      598      -37     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add 2 tests for configure_managed_ytdlp fallback behavior:
- test_system_source_fallback_reverts_to_stable: verifies setSetting,
  notification, and log when system has no importable yt_dlp
- test_system_source_no_fallback_when_importable: verifies no fallback
  when yt_dlp is importable

Also fix 2 log() calls missing the level argument (LOGINFO).

296 tests pass.
@firsttris
firsttris merged commit 63716b7 into firsttris:master Oct 4, 2026
2 checks passed
@firsttris

Copy link
Copy Markdown
Owner

Thanks a lot, merged! 🎉 Really appreciate the quick follow-up and that you tested it on the NixOS box.

I pushed a small follow-up (75c5680) on top:

  • system no longer rewrites the setting or downloads stable. A missing library is just reported (the importable check now uses the ensure_ytdlp_ready() result).
  • With system, "Manage yt-dlp version" / "Update now" show the system version instead of the GitHub release list.
  • The fallback tests now import actions fresh per test.

Would be great if you could give it a quick spin on your appliance with system selected. Thanks again for both PRs!

@zibaone

zibaone commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Gave it the spin you asked for — both directions.

On the appliance (LibreELEC, Kodi 22.0-RC1, master incl. 75c5680), with system
selected and nothing importable, it reports and leaves everything alone — no
rewrite, no download:

08:02:50.748 warning  plugin.video.sendtokodi: yt-dlp source is 'system' but no yt_dlp package is importable by Kodi's Python
08:02:50.749 error    plugin.video.sendtokodi: yt-dlp import failed: No module named 'yt_dlp'

No "reverting to stable" anywhere in the logs.

On the NixOS side (importable case): kodi-21.3 with yt-dlp's site-packages pinned
on Kodi's PYTHONPATH — Using system yt-dlp 2026.08.19, DASH manifest built, aac
audio decoded, playback position advancing; nothing fetched, nothing installed.
The zero-config setup from #182 working as intended.

Also verified the nightly channel still works with both PRs merged — details in #185.

Thanks again for both merges and the follow-up fix — both are working well here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants