Bug 257920 - [NEW PORT] devel/py-pycapnp: Cython wrapping of the C++ Cap'n Proto library
Summary: [NEW PORT] devel/py-pycapnp: Cython wrapping of the C++ Cap'n Proto library
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Fernando Apesteguía
URL:
Keywords: feature, needs-qa
Depends on:
Blocks:
 
Reported: 2021-08-18 03:23 UTC by Maxim Sobolev
Modified: 2021-09-01 07:31 UTC (History)
3 users (show)

See Also:
sobomax: maintainer-feedback+


Attachments
New port. (3.17 KB, patch)
2021-08-18 03:23 UTC, Maxim Sobolev
no flags Details | Diff
New port. (2.54 KB, patch)
2021-08-30 18:59 UTC, Maxim Sobolev
sobomax: maintainer-approval+
Details | Diff
New port (rev 2). (2.62 KB, patch)
2021-08-31 16:32 UTC, Maxim Sobolev
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maxim Sobolev freebsd_committer 2021-08-18 03:23:40 UTC
Created attachment 227297 [details]
New port.

This patch adds a new port py-pycapnp, a Python bindings of the Cap'n Proto library.
Comment 1 Maxim Sobolev freebsd_committer 2021-08-26 04:10:31 UTC
Hi Fernando, can you please take a look at this as well? Since you've helped me with devel/capnproto upgrade, this is closely related. Thanks!
Comment 2 Fernando Apesteguía freebsd_committer 2021-08-26 06:03:38 UTC
(In reply to Maxim Sobolev from comment #1)
Python ports are not my cup of tea, but I will give it a try.

Would you update the patch? There are references to emulators/virtualbox-ose that I suspect are unintended.
Comment 3 Maxim Sobolev freebsd_committer 2021-08-30 18:59:00 UTC
Created attachment 227558 [details]
New port.
Comment 4 Maxim Sobolev freebsd_committer 2021-08-30 18:59:49 UTC
(In reply to Fernando Apesteguía from comment #2)

Patch updated, sorry about that. Thanks!

-Max
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-31 01:35:50 UTC
Review items:

- Match COMMENT to upstream titl,e modulo portlint needs. In this case: "Cython wrapping of the C++ Cap'n Proto library"

- Package installs files into non Python version specific (shared) locations, port needs to be made concurrent (installation) safe using USE_PYTHON=concurrent [1]

- Upstream says it wants a C++14 compiler, probably worth using USES=compiler:<lang> here. [2]

- Upstream mentions needing special compiler args for 32-bit, does this also apply to us? [3]

- Using system (ports) LIB_DEPENDS is preferable, by default this uses the bundled libpcap code. Use --force-system-libcapnp" added to PYDISTUTILS_BUILDARGS [4]

- Add TEST_DEPENDS and (do-)test target when a package ships a test suite. In this case TEST_DEPENDS=pytest and do-test: cd ${WRKSRC} && ${PYTHON_CMD} -m pytest -v -rs -o addopts= should get it to run (please confirm QA)

[1] "console_scripts": ["capnpc-cython = capnp._gen:main"]
[2]  C++14 supported compiler 
[3] 32-bit Linux requires that capnproto be compiled with -fPIC. This is usually set correctly unless you are compiling canproto yourself. This is also called -DCMAKE_POSITION_INDEPENDENT_CODE=1 for cmake.
[4] setup.py: force_system_libcapnp = "--force-system-libcapnp" in sys.argv
Comment 6 Maxim Sobolev freebsd_committer 2021-08-31 16:32:30 UTC
Created attachment 227572 [details]
New port (rev 2).

Changes since last revision (thanks koobs@).

- Match COMMENT to upstream titl,e modulo portlint needs. In this case: "Cython wrapping of the C++ Cap'n Proto library"

- Package installs files into non Python version specific (shared) locations, port needs to be made concurrent (installation) safe using USE_PYTHON=concurrent [1]

- Upstream says it wants a C++14 compiler, probably worth using USES=compiler:<lang> here. [2]

- Using system (ports) LIB_DEPENDS is preferable, by default this uses the bundled libpcap code. Use --force-system-libcapnp" added to PYDISTUTILS_BUILDARGS [4]
Comment 7 Maxim Sobolev freebsd_committer 2021-08-31 16:43:35 UTC
(In reply to Kubilay Kocak from comment #5)

Thanks Kubilay for a detailed review. Most of the changes have been incorporated (see the diff for the detailed list), few notes:

[3] Does not apply, since it's only relevant when linking module with a *static* libcapnp.a (when bundled version is linked). In our case, we are linking with the libcapnp.so, so it's compiled as PIC on all arches of relevance.

[4] The setup.py would correctly discover system capnproto library when I worked on the port, in any case I've added --force-system-libcapnp to enforce it.

- I've tried the test suite and it fails unfortunately with many module not found errors. However, all of the examples bundled with the package seems to be working fine, I verified that before opening the bug request. I am really out of my time (and depth debugging those issues), which most likely caused by the test suite itself never tested on anything but Linux, so unless it's a hard requirement to have a working do-test in port I'd probably skip that for now. It takes a non-trivial amount of time to make 3rd party test suite to run on FreeBSD check this out as an example: https://github.com/capnproto/capnproto/pull/1321

Thanks again!

-Max
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2021-09-01 01:04:07 UTC
(In reply to Maxim Sobolev from comment #7)

Nice work creating PR upstream :) Can you reference the import errors here for future reference

Updated patch looks good. If it passes QA (poudriere, portlint at least), feel free to self-assign and land the port
Comment 9 Fernando Apesteguía freebsd_committer 2021-09-01 07:31:12 UTC
I don't think it detects the installed libcapnp properly:

=======================<phase: configure      >============================
===>   py38-pycapnp-1.1.0 depends on executable: cmake - found
===>   py38-pycapnp-1.1.0 depends on package: py38-pkgconfig>0 - found
===>   py38-pycapnp-1.1.0 depends on package: py38-cython>0 - found
===>   py38-pycapnp-1.1.0 depends on package: py38-setuptools>0 - found
===>   py38-pycapnp-1.1.0 depends on file: /usr/local/bin/python3.8 - found
===>   py38-pycapnp-1.1.0 depends on shared library: libcapnp.so - found (/usr/local/lib/libcapnp.so)
===>  Configuring for py38-pycapnp-1.1.0
...
...
*WARNING* no libcapnp detected or rebuild forced. Attempting to build it from source now. If you have C++ Cap'n Proto installed, it may be out of date or is not being detected. This may take a while...
fetching https://capnproto.org/capnproto-c++-0.8.0.tar.gz into bundled
*** Error code 1

It fails because it tries to fetch from a build stage. If inside a poudriere jail (with network connection), it downloads and builds with the bundled library because it still does not detect the installed one.

Stop.