Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kodiPlugins: init vfs.{sftp, libarchive} #56467

Merged
merged 3 commits into from Feb 28, 2019
Merged

Conversation

@minijackson
Copy link
Contributor

minijackson commented Feb 27, 2019

Motivation for this change

In the recent v18 update, Kodi moved the support for some filesystems into binary add-ons (including SFTP which I was using). Now, adding nixpkgs.config.kodi.enableVFSSFTP = true in the configuration brings back SFTP sources (and similar with RAR and Libarchive) \o/

I tested the SFTP with my sources and works fine, for RAR and Libarchive, the add-ons could be enabled without errors but I did not test them past that.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date (no documentation)
  • Fits CONTRIBUTING.md No LICENSE in any of these add-ons.
@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Feb 27, 2019

In the recent v18 update, Kodi moved the support for some filesystems into binary add-ons (including SFTP which I was using). Now, adding nixpkgs.config.kodi.enableVFSSFTP = true in the configuration brings back SFTP sources (and similar with RAR and Libarchive) \o/

I think this article would describe this change pretty well for those observing:

Copy link
Member

worldofpeace left a comment

Can you break up the addition of these packages into separate commits?
In particular we prefer to have one commit for adding yourself to the maintainers list like:

maintainers: add minijackson

The attribute to use for the plugins would be

kodiPlugins.plugin-name: init at version
@minijackson

This comment has been minimized.

Copy link
Contributor Author

minijackson commented Feb 27, 2019

Done! Thank you for explaining

@worldofpeace worldofpeace dismissed their stale review Feb 27, 2019

resolved by author

@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Feb 27, 2019

I've done the following to test:

  • Built all addons locally
  • Built kodi with the following config
nix-build -E 'with (import ./.{ config = { kodi = { enableVFSSFTP = true; enableVFSRAR = true; enableVFSLibarchive = true;  };  };  }); kodi'
  • enabled the addons within the application

However I didn't test if I could actually use these addons.
For me to merge this I need at least some sort of verification that they're actually usable.

@minijackson

This comment has been minimized.

Copy link
Contributor Author

minijackson commented Feb 27, 2019

For the SFTP test, I'm sure it works (Kodi wouldn't play any remote media before I enabled the plugin). If you want to test it yourself, you could start a local ssh server, add a new video source -> Browse -> Add Network location -> Protocol: "Secure Shell (SSH/SFTP)" -> etc.

If you need a test video that will be recognized by Kodi, you can do: ffmpeg -f lavfi -i testsrc -t 30 'The Room (2003).mp4'

For the RAR and Libarchive plugin, I confess I haven't been able to see how I can get Kodi to recognize an archive, but it might be just a UI issue.

Also, after looking at some issues in the vfs.rar repo, I found that there is no more need for this plugin as rar support is provided in vfs.libarchive (xbmc/vfs.rar#29). Shall I remove it?

@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Feb 27, 2019

Also, after looking at some issues in the vfs.rar repo, I found that there is no more need for this plugin as rar support is provided in vfs.libarchive (xbmc/vfs.rar#29). Shall I remove it?

Yes, I think this is proof that this is true https://github.com/xbmc/vfs.libarchive/blob/2ba11021ef58cf7fa9ca1051e5c0d558cd2a04bc/vfs.libarchive/addon.xml.in#L11

For the RAR and Libarchive plugin, I confess I haven't been able to see how I can get Kodi to recognize an archive, but it might be just a UI issue.

I've tested this simply by archiving a video into a .7z and then I was able to view it within the library 👍
This was with only the vfs.libarchive addon enabled.

@minijackson

This comment has been minimized.

Copy link
Contributor Author

minijackson commented Feb 27, 2019

Removed the vfs.rar commit

@minijackson minijackson changed the title kodi: add vfs.{sftp, rar, libarchive} binary plugins kodi: add vfs.{sftp, libarchive} binary plugins Feb 27, 2019
@worldofpeace worldofpeace changed the title kodi: add vfs.{sftp, libarchive} binary plugins kodiPlugins: init vfs.{sftp, libarchive} Feb 28, 2019
@worldofpeace worldofpeace merged commit 83c92d8 into NixOS:master Feb 28, 2019
10 checks passed
10 checks passed
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Feb 28, 2019

Thanks @minijackson

Hope you had a good time contributing to nixpkgs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.