inkscape: add scour as optional dependency #49903

bennylb · 2018-11-08T13:41:22Z

Motivation for this change

Inkscape has an option to export to an optimised svg. To do this it requires scour. This PR adds scour as an optional dependency.

Things done

Added:

pythonPackages.scour
scour to python-packages.nix
scourSupport option to inkscape/default.nix arguments
Myself to maintainers-list.nix

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 nox --run "nox-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)
Fits CONTRIBUTING.md.

FRidh

There is already a python2Env that can be extended. Also, does everyone want to have this optional dependency included?

pkgs/applications/graphics/inkscape/default.nix

FRidh · 2018-11-08T16:44:20Z

@GrahamcOfBorg build python2.pkgs.scour

GrahamcOfBorg · 2018-11-08T16:44:44Z

Success on aarch64-linux (full log)

Attempted: python2.pkgs.scour

Partial log (click to expand)

writing entry points to scour.egg-info/entry_points.txt
reading manifest file 'scour.egg-info/SOURCES.txt'
writing manifest file 'scour.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/pi45m1rlk4kxan1gylrmfrnz9i15avd4-python2.7-scour-0.37

GrahamcOfBorg · 2018-11-08T16:44:50Z

Success on x86_64-darwin (full log)

Attempted: python2.pkgs.scour

Partial log (click to expand)

writing entry points to scour.egg-info/entry_points.txt
reading manifest file 'scour.egg-info/SOURCES.txt'
writing manifest file 'scour.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/8l250hacjbcrw65l3i1k9s1iw73dkf3q-python2.7-scour-0.37

GrahamcOfBorg · 2018-11-08T16:45:04Z

Success on x86_64-linux (full log)

Attempted: python2.pkgs.scour

Partial log (click to expand)

writing entry points to scour.egg-info/entry_points.txt
reading manifest file 'scour.egg-info/SOURCES.txt'
writing manifest file 'scour.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/phsswfx37jv5ps1w9w5awpw7dcs8y8c4-python2.7-scour-0.37

FRidh · 2018-11-08T16:48:29Z

Ran 0 tests in 0.000s

No tests are found. Either the test runner cannot find tests and needs to be patched, or there simply are no tests in which case the tests need to be disabled. Do include a comment explaining why the tests are disabled.

worldofpeace · 2018-11-08T16:48:40Z

I already added scour to nixpkgs https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/graphics/scour/default.nix

FRidh · 2018-11-08T16:51:55Z

Thanks @worldofpeace. Now I could like to know, how does Inkscape use it. Do they import a Python scour module, or do they run the scour executable?

worldofpeace · 2018-11-08T16:57:12Z

@FRidh https://gitlab.com/inkscape/extensions/blob/master/output_scour.py

bennylb · 2018-11-09T03:36:26Z

Ran 0 tests in 0.000s
No tests are found. Either the test runner cannot find tests and needs to be patched, or there simply are no tests in which case the tests need to be disabled. Do include a comment explaining why the tests are disabled.

Unfortunately I don't have the requisite knowledge of python to enable tests. I used the existing package https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/graphics/scour/default.nix as a template.

bennylb · 2018-11-09T03:41:33Z

I already added scour to nixpkgs https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/graphics/scour/default.nix

This is a python 3 package. Don't we require python 2?

FRidh · 2018-11-11T08:58:34Z

scour is available as a Python library since 961180a, so please rebase onto that.

bennylb · 2018-11-12T19:36:13Z

scour is available as a Python library since 961180a, so please rebase onto that.

Optional scour dependency now included in existing python2Env and rebased against current master

worldofpeace · 2018-11-12T19:53:33Z

pkgs/applications/graphics/inkscape/default.nix

 }:

 let
-  python2Env = python2.withPackages(ps: with ps; [ numpy lxml ]);
+  python2Env = python2.withPackages(ps: with ps;
+    [ numpy lxml ] ++ stdenv.lib.optional scourSupport ps.scour);


I think this can be

stdenv.lib.optional scourSupport scour

because there's with ps;

Yeah right you are. I'll test it before pushing

worldofpeace · 2018-11-12T19:55:02Z

pkgs/applications/graphics/inkscape/default.nix

@@ -3,10 +3,12 @@
 , glibmm, libsigcxx, lcms, boost, gettext, makeWrapper
 , gsl, python2, poppler, imagemagick, libwpg, librevenge
 , libvisio, libcdr, libexif, potrace, cmake, hicolor-icon-theme
+, scourSupport ? false


Does this really need to be false?
It shouldn't affect the closure too much.

I changed that to false as @FRidh queried it in the initial PR. But I can change it back to true.

I haven't checked the closure size. How is Inkscape with optional dependencies? Are there a lot?

Arch linux seems to have a list of them.

@FRidh, @worldofpeace this is the closure of scour:

/nix/store/g2yk54hifqlsjiha3szr4q3ccmdzyrdv-glibc-2.27 25010568 /nix/store/pmvnf35kg6k2p0z91ac663akazlf7wz6-bzip2-1.0.6.0.1 25083424 /nix/store/1vb4m694aj9bdm7aq3911yjjb73gn4ii-attr-2.4.47 25090384 /nix/store/di0d17idlw6fxf3qwpfyxciw94ywi664-zlib-1.2.11 25135080 /nix/store/f6a7rladm0r8k62qvr4yb3gyj81cr9yp-acl-2.2.52 25190248 /nix/store/9l9mnc25y5rw4m6jfbgihv610si6gsiv-gdbm-1.18 25644808 /nix/store/r47p5pzx52m3n34vdgqpk5rvqgm0m24m-bash-4.4-p23 26186104 /nix/store/ck5ay23hsmlc67pg3m34kzd1k2hhvww0-sqlite-3.24.0 26294200 /nix/store/dzvbhkzfgwkq97ljkcvi4gs7vmflh6a0-coreutils-8.30 26877336 /nix/store/iirmif7qgp7pgbv80z5x1sj3hbay893d-ncurses-6.1 28370672 /nix/store/ra2kcrzvp2hmsvmq82kma077h19yx0hr-openssl-1.0.2p 28686952 /nix/store/nhfaids6kfq0x6xrff9bl5qkyg83vvmf-readline-6.3p08 28758392 /nix/store/3v5r7fkrbkw2qajadvjbf6p6qriz9p1i-gcc-7.3.0-lib 30635424 /nix/store/i209v6zy67lpyybx6kxkrlvz6crv8dkp-db-5.3.28 34871512 /nix/store/2brlr94ahy3a9mvcjy0qbqpv8zrb7b7s-python-2.7.15 94103328 /nix/store/cx4364dn6qx4xci990q1bk6i5z4880kw-python2.7-setuptools-40.2.0 95128600 /nix/store/281q69f2wkzi9bdwfs2rlalcyrhrd2yb-python2.7-six-1.11.0 95196656 /nix/store/8baq3rijziz6ws95jih09la954achxvl-python2.7-scour-0.37 95514616

worldofpeace · 2018-11-12T20:34:47Z

@bennylb
Also a commit msg that meets the contribution guidelines would be:

inkscape: add scour as optional dependency

Notice how these messages always use attribute names of the affected software.

FRidh · 2018-11-12T20:36:55Z

Note also that commit message and PR title are not the same :)

bennylb · 2018-11-12T20:37:04Z

@bennylb
Also a commit msg that meets the contribution guidelines would be:
inkscape: add scour as optional dependency
Notice how these messages always use attribute names of the affected software.

@worldofpeace thanks for pointing that out.

bennylb · 2018-11-12T20:39:50Z

@FRidh thanks I'll fix that up too.

FRidh · 2018-11-18T12:41:37Z

I've pushed fca9a06, always including scour.

bennylb · 2018-11-18T13:47:38Z

Thanks again @FRidh.

bennylb requested a review from FRidh as a code owner November 8, 2018 13:41

GrahamcOfBorg added 6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Nov 8, 2018

FRidh requested changes Nov 8, 2018

View reviewed changes

pkgs/applications/graphics/inkscape/default.nix Outdated Show resolved Hide resolved

bennylb force-pushed the package/inkscape branch from b604c4b to eb04e99 Compare November 12, 2018 19:30

GrahamcOfBorg removed the 6.topic: python label Nov 12, 2018

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Nov 12, 2018

worldofpeace reviewed Nov 12, 2018

View reviewed changes

inkscape: add scour as optional dependency

d11a9b9

bennylb force-pushed the package/inkscape branch from eb04e99 to d11a9b9 Compare November 12, 2018 21:45

bennylb changed the title ~~inkscape: Add optional dependency scour~~ inkscape: add scour as optional dependency Nov 12, 2018

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 and removed 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Nov 12, 2018

FRidh closed this Nov 18, 2018

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

inkscape: add scour as optional dependency #49903

inkscape: add scour as optional dependency #49903

bennylb commented Nov 8, 2018

FRidh left a comment

FRidh commented Nov 8, 2018

GrahamcOfBorg commented Nov 8, 2018

GrahamcOfBorg commented Nov 8, 2018

GrahamcOfBorg commented Nov 8, 2018

FRidh commented Nov 8, 2018

worldofpeace commented Nov 8, 2018

FRidh commented Nov 8, 2018

worldofpeace commented Nov 8, 2018

bennylb commented Nov 9, 2018

bennylb commented Nov 9, 2018

FRidh commented Nov 11, 2018

bennylb commented Nov 12, 2018

worldofpeace Nov 12, 2018

bennylb Nov 12, 2018

worldofpeace Nov 12, 2018

bennylb Nov 12, 2018

FRidh Nov 12, 2018

worldofpeace Nov 12, 2018 •

edited

bennylb Nov 12, 2018 •

edited

worldofpeace commented Nov 12, 2018

FRidh commented Nov 12, 2018

bennylb commented Nov 12, 2018

bennylb commented Nov 12, 2018

FRidh commented Nov 18, 2018

bennylb commented Nov 18, 2018

inkscape: add scour as optional dependency #49903

inkscape: add scour as optional dependency #49903

Conversation

bennylb commented Nov 8, 2018

Motivation for this change

Things done

FRidh left a comment

Choose a reason for hiding this comment

FRidh commented Nov 8, 2018

GrahamcOfBorg commented Nov 8, 2018

GrahamcOfBorg commented Nov 8, 2018

GrahamcOfBorg commented Nov 8, 2018

FRidh commented Nov 8, 2018

worldofpeace commented Nov 8, 2018

FRidh commented Nov 8, 2018

worldofpeace commented Nov 8, 2018

bennylb commented Nov 9, 2018

bennylb commented Nov 9, 2018

FRidh commented Nov 11, 2018

bennylb commented Nov 12, 2018

worldofpeace Nov 12, 2018

Choose a reason for hiding this comment

bennylb Nov 12, 2018

Choose a reason for hiding this comment

worldofpeace Nov 12, 2018

Choose a reason for hiding this comment

bennylb Nov 12, 2018

Choose a reason for hiding this comment

FRidh Nov 12, 2018

Choose a reason for hiding this comment

worldofpeace Nov 12, 2018 • edited

Choose a reason for hiding this comment

bennylb Nov 12, 2018 • edited

Choose a reason for hiding this comment

worldofpeace commented Nov 12, 2018

FRidh commented Nov 12, 2018

bennylb commented Nov 12, 2018

bennylb commented Nov 12, 2018

FRidh commented Nov 18, 2018

bennylb commented Nov 18, 2018

worldofpeace Nov 12, 2018 •

edited

bennylb Nov 12, 2018 •

edited