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

avidemux: rewrite derivation #35080

Merged
merged 1 commit into from Feb 24, 2018
Merged

avidemux: rewrite derivation #35080

merged 1 commit into from Feb 24, 2018

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Feb 17, 2018

Motivation for this change

This drastically reduces the complexity of the avidemux derivation
and adds QT5 support (see #33248).

Rather than invoking cmake over preconfigured hooks, it's much easier
to use the bootStrap.bash script provided by the developers to do the
installation tasks. Furthermore this script makes it way easier to
configure which parts of avidemux should be used (e.g. CLI-only) or
without the plugins.

In order to create a CLI-only instance you can simply override the
derivation:

avidemux.override {
  withQT = false;
}

It's possible to set the default executable as well (avidemux creates
a avidemux_qt5 and avidemux_cli executable by default):

avidemux.override {
  default = "cli"; # default is `qt5`
}

The GTK support has been dropped entirely since it was originally broken
in our system and can't be built ATM. Other distros such as ArchLinux
don't support GTK anymore (see https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/avidemux#n64)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

This drastically reduces the complexity of the `avidemux` derivation
and adds QT5 support (see NixOS#33248).

Rather than invoking `cmake` over preconfigured hooks, it's much easier
to use the `bootStrap.bash` script provided by the developers to do the
installation tasks. Furthermore this script makes it way easier to
configure which parts of `avidemux` should be used (e.g. CLI-only) or
without the plugins.

In order to create a CLI-only instance you can simply override the
derivation:

```
avidemux.override {
  withQT = false;
}
```

It's possible to set the default executable as well (`avidemux` creates
a `avidemux_qt5` and `avidemux_cli` executable by default):

```
avidemux.override {
  default = "cli"; # default is `qt5`
}
```

The GTK support has been dropped entirely since it was originally broken
in our system and can't be built ATM. Other distros such as ArchLinux
don't support GTK anymore (see https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/avidemux#n64)
meta = with stdenv.lib; {
homepage = http://fixounet.free.fr/avidemux/;
description = "Free video editor designed for simple video editing tasks";
maintainers = with maintainers; [ viric abbradar ma27 ];
Copy link
Member Author

Choose a reason for hiding this comment

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

@viric @abbradar is it ok to keep you as maintainers as I changed way to much in this derivation :)

@abbradar
Copy link
Member

My original motivation for this complexity was to split Avidemux over several derivations so that one can build GTK and Qt plugins separately from core libraries. However given that there is only one backend now which people would likely use (Qt 5) it doesn't seem to worth it now. Thanks!

P.S.: I'm okay with keeping me as a maintainer.

@Ma27
Copy link
Member Author

Ma27 commented Feb 24, 2018

great @abbradar .
Btw is there anything that keeps us from merging? :)

@abbradar
Copy link
Member

@Ma27 Nope, I'll test and merge if everything is okay today. Thanks for your work!

@abbradar abbradar merged commit f027e82 into NixOS:master Feb 24, 2018
@Ma27 Ma27 deleted the fix-avidemux branch February 24, 2018 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants