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

vapoursynth: split python module out #58859

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@tadeokondrak
Copy link
Contributor

commented Apr 2, 2019

Motivation for this change

Previously, vapoursynth was not a python module in how most python libraries are in nixpkgs. It would only work as part of a library or other program, and its included vspipe program was broken.

There still isn't a convenient way to load plugins (it relies on a global search path on other distros), which I plan to address in a future PR, probably by adding a vapoursynth.withPlugins function.

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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
    • vspipe
    • mpv with vapoursynth enabled
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@tadeokondrak tadeokondrak requested a review from FRidh as a code owner Apr 2, 2019

@tadeokondrak

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

cc @rnhmjoj (maintainer)

@tadeokondrak tadeokondrak force-pushed the tadeokondrak:vapoursynth-fix-vspipe branch 2 times, most recently from c7f6f37 to c331d9f Apr 3, 2019

@tadeokondrak

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

I can't seem to get vspipe working if it's in the out output, seems to create a dependency cycle if so

Edit: Thinking about it more, it makes sense that it's in the python output (it evaluates python scripts). Might be a bit unintuitive, but still better than the previous behaviour of it not working

@rnhmjoj

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

The only way I've found to load plugins is to create an environment with buildEnv containing the packages needed, then set $XDG_CONFIG_HOME/vapoursynth/vapoursynth.conf to

SystemPluginDir=/run/current-system/sw/lib/vapoursynth

I tested my setup for frame interpolation in mpv and looks good. I've never used vspipe directly, so you probably know better than me.

@@ -21,7 +21,7 @@ stdenv.mkDerivation rec {
sha256 = "09fj4k75cksx1imivqfyr945swlr8k392kkdgzldwc4404qv82s6";
};

nativeBuildInputs = [ pkgconfig autoreconfHook nasm ];
nativeBuildInputs = [ pkgconfig autoreconfHook nasm python3 makeWrapper ];

This comment has been minimized.

Copy link
@Shados

Shados Jun 11, 2019

Contributor

You now have two separate python3 executables in $PATH in the builder. Given the libraries included in the one a couple lines down, I'm assuming that one is actually used during the build process, not this one?

Does vapoursynth's Python module have dependencies on any non-standard modules?

This comment has been minimized.

Copy link
@tadeokondrak

tadeokondrak Jun 12, 2019

Author Contributor

This python3 seems to be needed to get the toPythonPath in the wrapProgram call below to work.

Does vapoursynth's Python module have dependencies on any non-standard modules?

No, the current ones are only used during the build.

moveToOutput lib/${python3.libPrefix} $python
moveToOutput bin/vspipe $python
wrapProgram $python/bin/vspipe \
--prefix PYTHONPATH : $(toPythonPath $python)

This comment has been minimized.

Copy link
@Shados

Shados Jun 11, 2019

Contributor

Use wrapPythonPrograms from python3.pkgs.wrapPython, instead.

This comment has been minimized.

Copy link
@tadeokondrak

tadeokondrak Jun 12, 2019

Author Contributor

This doesn't seem to work with binary programs.

This comment has been minimized.

Copy link
@Shados

Shados Jun 12, 2019

Contributor

It does work with binary programs (it iterally calls wrapProgram under the hood), but it relies on $pythonPath being set in the builder. This happens automatically for buildPythonPackage, but not for stdenv.mkDerivation.

You could either set pythonPath appropriately, or actually use buildPythonPackage, which would possibly be appropriate given it provides a Python library. I suspect if you do that and specify format = "other";, it may Just Work (tm) and you can drop the manual wrapping entirely, but I haven't tested it 🤷‍♂.

Show resolved Hide resolved pkgs/development/libraries/vapoursynth/default.nix Outdated

@tadeokondrak tadeokondrak force-pushed the tadeokondrak:vapoursynth-fix-vspipe branch from c331d9f to d6ca994 Jun 12, 2019

vapoursynth: split python module, mpv: use it
previously, vapoursynth was not a python module in how most python
libraries are in nixpkgs. it would only work as part of a library or
other program, and not with its included `vspipe` program.

this fixes it by adding a new output for its python module and uses
toPythonModule to allow it to work standalone, and without conflicting
with other python packages in a nix-shell

@tadeokondrak tadeokondrak force-pushed the tadeokondrak:vapoursynth-fix-vspipe branch from d6ca994 to 37b5844 Jun 12, 2019

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