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

gimp: fix all plugins build #25393

Merged
merged 3 commits into from
May 4, 2017
Merged

gimp: fix all plugins build #25393

merged 3 commits into from
May 4, 2017

Conversation

avnik
Copy link
Contributor

@avnik avnik commented May 1, 2017

Motivation for this change

Some plugins was broken, and prevent build gimp with all plugins installed

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
    • Linux
  • 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.

@mention-bot
Copy link

@avnik, thanks for your PR! By analyzing the history of the files in this pull request, we identified @cillianderoiste, @MarcWeber and @edolstra to be potential reviewers.

buildInputs = with pkgs; [
fftw
autoreconfHook
intltool
Copy link
Member

Choose a reason for hiding this comment

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

autoreconfHook and intltool belong to nativeBuildInputs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

intltool is enough common for most plugins, should it be default?

Copy link
Member

Choose a reason for hiding this comment

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

could be.

src = fetchurl {
url = mirror://sourceforge/gimp-texturize/texturize-2.1_src.tgz;
sha256 = "0cdjq25g3yfxx6bzx6nid21kq659s1vl9id4wxyjs2dhcv229cg3";
};
buildInputs = with pkgs; [ intltool perl ];
Copy link
Member

Choose a reason for hiding this comment

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

intltool belongs to nativeBuildInputs

@@ -156,7 +159,7 @@ rec {
Filters/Enhance/Wavelet sharpen
*/
name = "wavelet-sharpen-0.1.2";
buildInputs = [ gimp ] ++ gimp.nativeBuildInputs;
buildInputs = with pkgs; [ intltool ];
Copy link
Member

Choose a reason for hiding this comment

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

same here.

@@ -169,7 +172,7 @@ rec {
Layer/Liquid Rescale
*/
name = "lqr-plugin-0.6.1";
buildInputs = [ pkgconfig libLQR gimp ] ++ gimp.nativeBuildInputs;
buildInputs = with pkgs; [ intltool libLQR ];
Copy link
Member

Choose a reason for hiding this comment

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

same here.

// { name = "${a.name}-${gimp.name}-plugin"; }
// {
name = "${a.name}-${gimp.name}-plugin";
buildInputs = [ gimp gimp.gtk glib pkgconfig ] ++ (a.buildInputs or []);
Copy link
Member

Choose a reason for hiding this comment

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

pkgconfig belongs to nativeBuildInputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinked about split nativeBuildInputs, but was urge to rebuild my system. Will do.

@avnik
Copy link
Contributor Author

avnik commented May 3, 2017

@Mic92 all stuff you requested was fixed

It used by gimp itself (so should be already present on machine builds
gimp+plugins) and 90% of plugins.
@avnik
Copy link
Contributor Author

avnik commented May 4, 2017

@Mic92 also promoted intltool as default build input, I think it ready to merge

@7c6f434c 7c6f434c merged commit 6c149ee into NixOS:master May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants