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

ffmpegthumbnailer: enable generation of thumbnailer file #107632

Merged
merged 1 commit into from Jan 1, 2021

Conversation

vs49688
Copy link
Contributor

@vs49688 vs49688 commented Dec 26, 2020

Motivation for this change

Makes ffmpegthumbnailer usable with desktop environments.

I've specifically used it to get video thumbnailing working on MATE (on 20.09).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 107632 run on x86_64-linux 1

4 packages built:
  • ffmpegthumbnailer
  • geeqie
  • spaceFM
  • xfce.tumbler

@vs49688
Copy link
Contributor Author

vs49688 commented Dec 30, 2020

Ping? I've done the requested changes.

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review. If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 107632 run on x86_64-linux 1

4 packages built:
  • ffmpegthumbnailer
  • geeqie
  • spaceFM
  • xfce.tumbler

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

ffmpegthumbnailer.log:

gpl2 is a deprecated license, check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

Near pkgs/development/libraries/ffmpegthumbnailer/default.nix:31:5:

   |
31 |     license = licenses.gpl2;
   |     ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/unclear-gpl.md

xfce.tumbler.log:

gpl2 is a deprecated license, check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

Near lib/attrsets.nix:344:7:

    |
344 |       value = f name (catAttrs name sets);
    |       ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/unclear-gpl.md

@SuperSandro2000
Copy link
Member

Also please squash the commits.

Passing "-DENABLE_THUMBNAILER=ON" to CMake causes it to install
"ffmpegthumbnailer.thumbnailer" into /share/thumbnailers, making it
actually usable with desktop environments.

Cleanups:
- remove trailing whitespace
- change pkgconfig -> pkg-config
- change license from gpl2 to gpl2Plus
@vs49688
Copy link
Contributor Author

vs49688 commented Jan 1, 2021

Done

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review. If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 107632 run on x86_64-linux 1

4 packages built:
  • ffmpegthumbnailer
  • geeqie
  • spaceFM
  • xfce.tumbler

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

xfce.tumbler.log:

gpl2 is a deprecated license, check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

Near lib/attrsets.nix:344:7:

    |
344 |       value = f name (catAttrs name sets);
    |       ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/unclear-gpl.md

@vs49688
Copy link
Contributor Author

vs49688 commented Jan 1, 2021

tumbler is a different package, it should be fixed in a different PR, or at the very least a different commit.

@vs49688
Copy link
Contributor Author

vs49688 commented Jan 1, 2021

Also where's that gpl2 coming from, it appears not to be set for xfce.tumbler:
In: pkgs/desktops/xfce/core/tumbler/default.nix:

  meta = {
    description = "A D-Bus thumbnailer service";
  };

EDIT: Right, so it's from pkgs/desktops/xfce/mkXfceDerivation.nix. Changing that is beyond the scope of this PR imo.

@SuperSandro2000
Copy link
Member

Also where's that gpl2 coming from, it appears not to be set for xfce.tumbler:
In: pkgs/desktops/xfce/core/tumbler/default.nix:

  meta = {
    description = "A D-Bus thumbnailer service";
  };

EDIT: Right, so it's from pkgs/desktops/xfce/mkXfceDerivation.nix. Changing that is beyond the scope of this PR imo.

This is a semi-automatic executed nixpkgs-review.

Please fix at least the ones listed with your changed packages:

@SuperSandro2000 SuperSandro2000 merged commit 988e83b into NixOS:master Jan 1, 2021
@vs49688 vs49688 deleted the fffix branch August 21, 2021 06:15
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