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.

Sign up for GitHub

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

Jump to bottom

libredwg: init at 0.10.1 #71864

Merged
merged 1 commit into from Jul 7, 2020
Merged

libredwg: init at 0.10.1 #71864

merged 1 commit into from Jul 7, 2020

Conversation

thorstenweber83
Copy link
Contributor

@thorstenweber83 thorstenweber83 commented Oct 23, 2019 •
edited

Motivation for this change

add library libredwg, including python bindings

fixes #71192

Things done
  • 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 nix-review --run "nix-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.

Still needs testing on non nixos platforms, though.

@jonringer
Copy link
Contributor

does this only work with python2?

@jonringer
Copy link
Contributor 

checking for Python include path... -I/nix/store/4g2ilwnk4mj4g68rc52kygx7gcqxnjd0-python3-3.7.5/include/python3.7m
checking for Python library path... use the official shared library
-L/nix/store/4g2ilwnk4mj4g68rc52kygx7gcqxnjd0-python3-3.7.5/lib -lpython3.7m
checking for Python site-packages path... /nix/store/4g2ilwnk4mj4g68rc52kygx7gcqxnjd0-python3-3.7.5/lib/python3.7/site-packages
checking python extra libraries...  -lpthread -ldl -lcrypt -lncurses -lutil -lm
checking python extra linking flags... -Xlinker -export-dynamic
checking consistency of all components of python development environment... no
configure: error: in `/build/source':
configure: error:
  Could not link test program to Python. Maybe the main Python library has been
  installed in some non-standard library path. If so, pass it to configure,
  via the LIBS environment variable.
  Example: ./configure LIBS="-L/usr/non-standard-path/python/lib"
  ============================================================================
   ERROR!
   You probably have to install the development version of the Python package
   for your distribution.  The exact name of this package varies among them.
  ============================================================================

See `config.log' for more details

@thorstenweber83
Copy link
Contributor Author

thorstenweber83 commented Oct 24, 2019 •
edited

Well it should work with python3.
I'll look into it.
I'm sorry to have wasted your time!

@jonringer
Copy link
Contributor

? don't worry about it :)

@jonringer
Copy link
Contributor

took like 2mins

jonringer
jonringer reviewed Oct 27, 2019
View reviewed changes
pkgs/development/libraries/libredwg/default.nix Outdated
, enablePython ? false, python, swig, ncurses
}:
let
isPython3 = enablePython && lib.versionAtLeast python.version "3";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
isPython3 = enablePython && lib.versionAtLeast python.version "3";
isPython3 = enablePython && python.pythonAtLeast "3.0"

Copy link
Contributor

Choose a reason for hiding this comment

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

another option, you can choose either one

@thorstenweber83
Copy link
Contributor Author

I found the cause of the python 3 failure:

configure somehow detects ncurses as "python extra library",
then tries to link against it and fails.

# config.log:1758:
configure:16470: checking python extra libraries
configure:16477: result:  -lpthread -ldl -lcrypt -lncurses -lutil -lm
[...]
configure:16532: gcc -o conftest -g -O2  -I/nix/store/4g2ilwnk4mj4g68rc52kygx7gcqxnjd0-python3-3.7.5/include/python3.7m  -Xlinker -export-dynamic conftest.c -L/nix/store/4g2ilwnk4mj4g68rc52kygx7gcqxnjd0-python3-3.7.5/lib -L/nix/store/4g2ilwnk4mj4g68rc52kygx7gcqxnjd0-python3-3.7.5/lib -lpython3.7m  -lpthread -ldl -lcrypt -lncurses -lutil -lm >&5
/nix/store/cg0k49h66nkdqx6ccwnqr0i4q0fnfznc-binutils-2.31.1/bin/ld: cannot find -lncurses

When building with pyhon2 on the other hand, no ncurses (or crypt for that matter) "extra libraries" are detected:

# config.log:1758:
configure:16470: checking python extra libraries
configure:16477: result:  -lpthread -ldl  -lutil -lm

Adding ncurses as dependency helps, but feels strange because it's not really a dependency of this library.

@jonringer
Copy link
Contributor

i believe ncurses is already in the python3 closure, so your closure size won't really get hurt if you include it.

$ nix-store -q --tree /nix/store/zdh16dcvjw99ybam59zd2ijb6bx138j0-python3-3.7.5 | grep ncurses
+---/nix/store/iaf6ky905iflcnf0m17rpsh0d24j6vqw-ncurses-6.1-20190112
|   +---/nix/store/iaf6ky905iflcnf0m17rpsh0d24j6vqw-ncurses-6.1-20190112 [...]
|   +---/nix/store/iaf6ky905iflcnf0m17rpsh0d24j6vqw-ncurses-6.1-20190112 [...]

@thorstenweber83
Copy link
Contributor Author

@jonringer

i went with pythonAtLeast.
Also added a checkPhase, so we can have some tests!

@jonringer
Copy link
Contributor

@GrahamcOfBorg build libredwg python27Packages.libredwg python37Packages.libredwg python38Packages.libredwg

jonringer
jonringer approved these changes Oct 27, 2019
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

nix-review passes on NixOS
diff LGTM
executables seem to work

https://github.com/NixOS/nixpkgs/pull/71864
4 package were build:
libredwg python27Packages.libredwg python37Packages.libredwg python38Packages.libredwg

@luzpaz
Copy link
Contributor

luzpaz commented Dec 31, 2019

Any progress on this ?

@jonringer
Copy link
Contributor

@GrahamcOfBorg build libredwg python27Packages.libredwg python37Packages.libredwg python38Packages.libredwg

@stale
Copy link

stale bot commented Jun 28, 2020

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 28, 2020
@luzpaz
Copy link
Contributor

luzpaz commented Jul 5, 2020

soft bump

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 5, 2020
@thorstenweber83 thorstenweber83 changed the title libredwg: init at 0.9 libredwg: init at 0.10.1 Jul 6, 2020
@thorstenweber83
Copy link
Contributor Author

thorstenweber83 commented Jul 6, 2020 •
edited

Rebased the pr, updated to latest release and made sure it still builds.
6a97422 gave me some problems with the "xmlsuite" part of the tests.

jtojnar
jtojnar reviewed Jul 6, 2020
View reviewed changes
pkgs/development/libraries/libredwg/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libredwg/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libredwg/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libredwg/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libredwg/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libredwg/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libredwg/default.nix Outdated Show resolved Hide resolved
@thorstenweber83
Copy link
Contributor Author

@jtojnar thanks for your review!

@thorstenweber83
Copy link
Contributor Author

@jtojnar updated the pr considering your points. Thanks again for the detailed review!

@jtojnar jtojnar merged commit 869dfc0 into NixOS:master Jul 7, 2020
@jtojnar
Copy link
Contributor

jtojnar commented Jul 7, 2020

Looks good, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar left review comments

@jonringer jonringer jonringer approved these changes

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Package request: LibreDWG
4 participants
@thorstenweber83 @jonringer @luzpaz @jtojnar