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

Jump to bottom

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

Merged
merged 3 commits into from Feb 28, 2019
Merged

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

merged 3 commits into from Feb 28, 2019

Conversation

minijackson
Copy link
Member

@minijackson minijackson commented Feb 27, 2019 •
edited

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
Copy link
Contributor

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:

worldofpeace
worldofpeace previously requested changes Feb 27, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member Author

Done! Thank you for explaining

@worldofpeace worldofpeace dismissed their stale review February 27, 2019 18:35

resolved by author

@worldofpeace
Copy link
Contributor

worldofpeace commented Feb 27, 2019 •
edited

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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
@worldofpeace
Copy link
Contributor

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
Reviewers

@worldofpeace worldofpeace worldofpeace left review comments

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@minijackson @worldofpeace @GrahamcOfBorg @Janik-Haag