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: Closed Feedback Timeout
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: 2023-03-29 06:49 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
Fixed patch (detects devel/capnproto (2.64 KB, patch)
2022-01-25 15:52 UTC, Fernando Apesteguía
fernape: maintainer-approval? (sobomax)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maxim Sobolev freebsd_committer freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 2021-08-30 18:59:00 UTC
Created attachment 227558 [details]
New port.
Comment 4 Maxim Sobolev freebsd_committer freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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.
Comment 10 Fernando Apesteguía freebsd_committer freebsd_triage 2022-01-25 15:52:04 UTC
Created attachment 231315 [details]
Fixed patch (detects devel/capnproto

This one should do it.

It seems setup.py has several fallbacks for finding capnproto before trying to download the code and building it by itself. Add USES localbase and pkgconfig to cover those fallbacks.

Builds fine in 12.2{amd64,i386}, 13.0amd64 and -CURRENT amd64

Would you test this?
Comment 11 Fernando Apesteguía freebsd_committer freebsd_triage 2022-02-03 11:42:03 UTC
Ping :-)
Comment 12 Fernando Apesteguía freebsd_committer freebsd_triage 2022-02-25 08:41:01 UTC
I would like, in a few days, to close this PR as Feedback Timeout if there are no changes.

Cheers.