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

inkscape: add option for extra python packages #100565

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@olynch
Copy link
Contributor

@olynch olynch commented Oct 15, 2020

Motivation for this change

Certain inkscape extensions (particularly textext) need extra python packages, and this allows inkscape to be built with these extra packages.

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

@jtojnar jtojnar commented Oct 15, 2020

Is that a built-in or third party extension? If the former, could we add the dependency to inkscape by default, if the latter, could we add the dependency to the extension?

@olynch
Copy link
Contributor Author

@olynch olynch commented Oct 15, 2020

It's a third-party extension. I'm not sure if we could add the dependency to the extension because the extension uses the built-in python in inkscape.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Nov 27, 2020

@jtojnar Do you mean adding it to extensions or the inkscape build with extensions?

@raboof
Copy link
Member

@raboof raboof commented May 2, 2021

I'm not sure if we could add the dependency to the extension because the extension uses the built-in python in inkscape.

https://inkscape.org/develop/extensions/ suggests inkscape extensions may be in python, but don't need to be - so perhaps it is possible to use a different python for inkscape than for the extension.

I started trying this out a bit in https://gist.github.com/raboof/bb744b45eb01443cd155956124a6e83d . I'm currently stuck getting it to find GTK3, though. If we can make this work, another advantage of this (aside better isolation) would be that you can add extensions to Inkscape without having to rebuild Inkscape itself.

(I think we should be careful not to delay incremental improvements, like the one proposed in this PR, by searching for 'even better' solutions like "add the dependency to the extension". So if we can't make "add the dependency to the extension" work soon-ish we should consider the approach proposed in this PR again)

@raboof
Copy link
Member

@raboof raboof commented May 3, 2021

I'm currently stuck getting it to find GTK3, though

Updated the gist to use tkinter: https://gist.github.com/raboof/bb744b45eb01443cd155956124a6e83d

This shows this approach can work: with this shell.nix I can use the textext extension in Inkscape. It needs some work, for example to make sure texlive it properly recognized as a runtime dependency, but I think this is the direction to go in.

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented May 3, 2021

PyGI is in python3.pkgs.pygobject3. It will require properly set environment to run: https://nixos.org/manual/nixpkgs/stable/#ssec-gnome-hooks-gobject-introspection

@raboof raboof mentioned this pull request May 4, 2021
10 tasks
@raboof
Copy link
Member

@raboof raboof commented Aug 14, 2021

This shows this approach can work (...). It needs some work, for example to make sure texlive it properly recognized as a runtime dependency, but I think this is the direction to go in.

Now proposed as #121711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants