Bug 257131 - [NEW PORT] devel/py-cadquery-pywrap: C++ binding generator for Python
Summary: [NEW PORT] devel/py-cadquery-pywrap: C++ binding generator for Python
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Yasuhiro Kimura
URL: https://github.com/CadQuery/pywrap
Keywords: feature, needs-qa
Depends on: 256924 256925 258199
  Show dependency treegraph
Reported: 2021-07-12 13:37 UTC by Neal Nelson
Modified: 2021-10-28 08:47 UTC (History)
4 users (show)

See Also:
koobs: maintainer-feedback+

Git diff of new port. (5.95 KB, patch)
2021-07-12 13:37 UTC, Neal Nelson
no flags Details | Diff
Git diff of new port. (4.12 KB, patch)
2021-07-14 14:08 UTC, Neal Nelson
no flags Details | Diff
Git diff of new port. (4.35 KB, patch)
2021-08-26 15:31 UTC, Neal Nelson
ports: maintainer-approval+
Details | Diff
Git diff of new port. (2.51 KB, patch)
2021-09-01 16:57 UTC, Neal Nelson
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Neal Nelson 2021-07-12 13:37:13 UTC
Created attachment 226394 [details]
Git diff of new port.

PyWrap is a C++ binding generator using pybind11, libclang and jinja. The main goal of this project is to automatically generate bindings for OCCT7.3 (opencascade) and beyond without single manual edit of the generated code. Once finished the project will be usable as a general C++ binding generator.

This port is a requirement for an upcoming update to cad/cadquery, which is currently broken. It in turn requires the following new ports to be committed: bug #256925 and bug #256924 before committing this one.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-14 10:38:14 UTC
Review items:

Upstream seem to be using a name (setup.py:name=pywrap) that's already registered with PyPI [1][2], a different project, which is likely to be problematic in various ways.

Probably worth pinging upstream about it if they're not already aware. Since your port is named "cadquery-pywrap", there's slightly less of an issue, unless and until upstream pick a different 'proper name' and we have to rename the port. Better to get clarity on what they will call it as soon as possible. Ideally its "cadquery-pywrap" :)

Upstream pins jinja ( 'jinja2==2.11.3',), modifying just the RUN_DEPENDS is insufficient, and will break at runtime as soon as jinja is not that exact version.

Patch the upstream dependency version spec, and ask upstream to make this a normal >= dependency

All the python (.py) files will need to be precompiled (pyc/pyo) prior to installation using standard 'compileall' methods, as these will be generated at first run time, and on pkg-deinstall be orphaned (as theyre not referenced in the pkg-plist).

However, I would suggest trying to make the standard 'USE_PYTHON=distutils autoplist concurrent' framework bits work, since upstream ships a setup.py, that 'should just work'

If you need help #freebsd-ports and/or #freebsd-python on Libera IRC :)

[1] https://pypi.org/project/pywrap/
[2] https://github.com/tmr232/pywrap
Comment 2 Neal Nelson 2021-07-14 14:05:30 UTC
(In reply to Kubilay Kocak from comment #1)

Yes, the upstream name is pywrap, and yes there is already something else called pywrap. I doubt that it will be changed upstream though as it is currently an internal tool that is used to generate the CadQuery OCP library.

As a standalone tool it's not very interesting at the moment, but it is needed for my upcoming update to cad/py-ocp in order to generate the cad/opencascade interface. The upstream project have it linked into the OCP git repository, but there seems to be no way to do that in our ports system. If there is, then maybe I can dispense with this port altogether.

I just realised that they have added a setup.py. This update to cad/cadquery has been in development for over six months, so things have progressed and I missed this being added. I will update the port to use distutils and autoplist.

As for the fixed Jinja2 dependency in setup.py: it is already different on my system and works fine. I can ask the project about this, but can promise nothing.
Comment 3 Neal Nelson 2021-07-14 14:08:16 UTC
Created attachment 226459 [details]
Git diff of new port.

Here's a new diff using distutils and autoplist.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-20 00:47:44 UTC
Thanks for the update Neal

 * Hard-coded 'usr/local/' needs to be substituted (LOCALBASE) 


 * ${PYTHON_PKGNAMEPREFIX}Jinja2>=2.11.2:devel/py-Jinja2@${PY_FLAVOR}

If the setup.py from the git hash fetched from upstream matches (uses >= as well), thats fine. If the source setup.py still says ==, it *must* be patched. For example the current version of jinja2 in ports is 2.11.2, which does not satisfy the version spec, and will fail at runtime

 * Add 'python' to CATEGORIES

 * Remove ",run" from USES=python. The default is build/run, and given .py files need to be built in order to produce the package, is also a build dependency.

 * The following line looks to need to be made relative, with the hard coded prefix (/usr/local) removed and replaced with the relevent prefix variable for other platforms in that file:

 Config.set_library_file(Path("/usr") / "local" / "llvm90" / 'lib' / 'libclang.so')
Comment 5 Neal Nelson 2021-08-26 15:31:39 UTC
Created attachment 227451 [details]
Git diff of new port.

- Fixed all the hard coded file roots. Sorry about that.
- Updated category and USES.
- Serendipitously upstream has removed the Jinja version requirement as it was nonsense. Unsurprisingly the version in ports works fine. Everything worked fine before though.
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-28 03:51:31 UTC
(In reply to Neal Nelson from comment #5)

Thanks Neal

  - Upstream ships a LICENSE file, add LICENSE_FILE

  - If jinja2 is no longer a requirement, should it be removed from RUN_DEPENDS ?

  - Use %%VARIABLE%% substitution in post-patch, or use SUB_FILES / SUBLIST

  - There's a strong preference for not hardcoding compiler versions. Does pywrap support only LLVM 10 or some other version set? If it's something like >=X, use something like the following: 


  - setup.py declares PyPI clang package as a dependency, whats the nature of this dependency or why is it not included or necessary here? My assumption would be that `import clang` would fail if its missing. Do we need to port it?

  - Packages that install files into non-versioned (shared) locations need to be made concurrent-installation- safe (using USE_PYTHON=concurrent). This package does: entry_points={'console_scripts':
Comment 7 Neal Nelson 2021-08-28 07:25:11 UTC
(In reply to Kubilay Kocak from comment #6)

- I can definitely add the license file, but the license type is as specified in the Makefile. I assume that there is a specific LICENSE_FILE entry that I can add?

- Jinja2 is still a requirement, but the specific version requirement has been removed upstream as it was not needed.

- I'll look in the ports handbook for the various variable substitution methods you mentioned. I have to admit I didn't see a mechanism so just gave it a go.

- Sorry for hardcoding the LLVM version. I'll add a variable.

- As for clang: please see #256925, which adds devel/py-cymbal.

I will provide a patch with the required changes within a few days.
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-29 01:57:21 UTC
(In reply to Neal Nelson from comment #7)

1) Yep, so LICENSE_FILE=${WRKSRC}/path-to/license-file, in this packages case its ${WRKSRC}/LICENSE (iirc)

2) Ack.

3) Your method works, using PLIST_FILES/PLIST_SUB is the preferred 'framework' way.

4) Re clang, unless im missing/misunderstanding something, if cadquery-pywrap does 'import clang' directly (iirc it does), this port should explicitly depend on a port that provides that module, so a py-clang port will be necessary. Presumably this is why pywrap has an 'install_requires = clang' dependency line
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-29 01:57:51 UTC
Forgot, re licenses, see also: https://docs.freebsd.org/en/books/porters-handbook/book/#licenses
Comment 10 Neal Nelson 2021-08-30 16:42:56 UTC
(In reply to Kubilay Kocak from comment #8)

In response to point 4: clang is already part of the llvm ports, so there is no need to install anything else. This may not be true for other OSes, so there is a clang package on PyPI. Patching the path within python is required however as the llvm python libraries are in a non-standard place. The port does this, which is where all the creative substitution comes in.
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-31 01:24:23 UTC
(In reply to Neal Nelson from comment #10)

Is that to say that that the devel/llvm* ports  unconditionally install the clang python bindings in LOCALBASE/lib/pythonX.Y/site-packages so that 'import clang' does not fail?
Comment 12 Neal Nelson 2021-08-31 07:31:07 UTC
(In reply to Kubilay Kocak from comment #11)

It is an option (PYCLANG), which is default. So, it is most likely present, but if the options are tampered with, then possibly not.
Comment 13 Neal Nelson 2021-08-31 07:33:18 UTC
(In reply to Kubilay Kocak from comment #11)

Oh, and no, they don't install into the proper location. They are installed into LOCALBASE/llvmLLVMVER/lib/pythonX.Y/site-packages, which is why I need to add it to the python path.
Comment 14 Kubilay Kocak freebsd_committer freebsd_triage 2021-09-01 01:06:27 UTC
(In reply to Neal Nelson from comment #12)

Thanks Neal

Understood. Given its optional, and not installed in the appropriate place (long term this part of the llvm package should/could probably be sub-packaged), we should create a py-clang port for this port to depend on. If necessary, the llvm ports can depend on it as well to provide the relevent python module
Comment 15 Neal Nelson 2021-09-01 16:40:11 UTC
(In reply to Kubilay Kocak from comment #14)

Submitted #258199.

Update to patch for this port to follow.
Comment 16 Neal Nelson 2021-09-01 16:57:30 UTC
Created attachment 227596 [details]
Git diff of new port.

Here's the latest version of the port, using the new devel/py-clang port and therefore no longer needing any nasty patching. I think that all of the other changes have been made as well.
Comment 17 Kubilay Kocak freebsd_committer freebsd_triage 2021-09-02 00:46:27 UTC
(In reply to Neal Nelson from comment #16)

Good stuff Neal, thank you! :)
Comment 18 Yasuhiro Kimura freebsd_committer 2021-10-28 00:33:47 UTC
Comment 19 commit-hook freebsd_committer 2021-10-28 08:42:16 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=1de9073690687a9fb6c3b34564a71b4f2c560e9d

commit 1de9073690687a9fb6c3b34564a71b4f2c560e9d
Author:     Neal Nelson <ports@nicandneal.net>
AuthorDate: 2021-10-28 06:55:50 +0000
Commit:     Yasuhiro Kimura <yasu@FreeBSD.org>
CommitDate: 2021-10-28 08:40:42 +0000

    devel/py-cadquery-pywrap: Add new port

    PyWrap is a C++ binding generator using pybind11, libclang and
    jinja. The main goal of this project is to automatically generate
    bindings for OCCT7.3 (opencascade) and beyond without single manual
    edit of the generated code. Once finished the project will be usable
    as a general C++ binding generator.

    PR:             257131

 devel/Makefile                           |  1 +
 devel/py-cadquery-pywrap/Makefile (new)  | 39 ++++++++++++++++++++++++++++++++
 devel/py-cadquery-pywrap/distinfo (new)  |  3 +++
 devel/py-cadquery-pywrap/pkg-descr (new) |  6 +++++
 4 files changed, 49 insertions(+)
Comment 20 Yasuhiro Kimura freebsd_committer 2021-10-28 08:47:17 UTC
Committed with minor refinement. Thanks!