Bug 199929

Summary: [NEW PORT] math/cryptominisat
Product: Ports & Packages Reporter: Gleb Popov <arrowd>
Component: Individual Port(s)Assignee: Jan Beich <jbeich>
Status: Closed FIXED    
Severity: Affects Only Me CC: jbeich
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Patch with new port
none
PYTHON as separate ports, v0 none

Description Gleb Popov freebsd_committer 2015-05-04 14:39:10 UTC
Created attachment 156338 [details]
Patch with new port

CryptoMiniSat is a modern, multi-threaded, feature-rich,
simplifying SAT solver, featuring over 100 configurable 
parameters to tune to specific need, collection of statistical
data to MySQL database + javascript-based visualization of it
and clean C++ and python interfaces.
Comment 1 Jan Beich freebsd_committer 2015-05-05 13:41:22 UTC
Comment on attachment 156338 [details]
Patch with new port

$ portlint -AC
WARN: /usr/ports/math/cryptominisat/pkg-plist: [8]: installing shared libraries, please define USE_LDCONFIG as appropriate
FATAL: /usr/ports/math/cryptominisat/pkg-plist: the last line has to be terminated by \n.

>+++ math/cryptominisat/Makefile	2015-05-02 22:20:57.733228258 +0300
[...]
>+.include <bsd.port.options.mk>
>+
>+.if ${PORT_OPTIONS:MPYTHON}

New ports should use option helpers if possible.

https://www.freebsd.org/doc/en/books/porters-handbook/makefile-options.html#options-helpers

>+USES+=		python:build

USES=python with no version specified is for ports that support both
python2 and python3. CMakeLists.txt seems to disagree:

  find_package (PythonInterp 2.7)
  find_package (PythonLibs 2.7)

>+PLIST_SUB+=	EGGFILE=pycryptosat-${PORTVERSION}-py${PYTHON_VER}.egg-info
>+.else
>+CMAKE_ARGS+=	-DPYTHON_EXECUTABLE=0
>+.endif

>++++ python/CMakeLists.txt
>+@@ -12,7 +12,7 @@ add_custom_command(OUTPUT ${OUTPUT}/time
>+ 
>+ add_custom_target(pytarget ALL DEPENDS ${OUTPUT}/timestamp)
>+ 
>+-install(CODE "execute_process(COMMAND ${PYTHON_EXECUTABLE} ${SETUP_PY} install --record files.txt)")
>++install(CODE "execute_process(COMMAND ${PYTHON_EXECUTABLE} ${SETUP_PY} install -c -O1 --prefix=${CMAKE_INSTALL_PREFIX} --root=${FREEBSD_STAGEDIR} --record files.txt)")

Why not create a separate USE_PYTHON=distutils port that depends on this one?

>+++ math/cryptominisat/pkg-plist	2015-05-02 16:26:25.333693829 +0300
>@@ -0,0 +1,10 @@
[...]
>+lib/libcryptominisat4.so

Maybe set SOVERSION, so the library path can be cached by ldconfig(8)
and used not only via RPATH.
Comment 2 Jan Beich freebsd_committer 2015-05-05 14:07:52 UTC
Also fails to build on FreeBSD 8.4R and 9.3R:

  cc1plus: error: unrecognized command line option "-std=c++11"
Comment 3 Gleb Popov freebsd_committer 2015-05-05 15:44:28 UTC
(In reply to Jan Beich from comment #1)
>WARN: /usr/ports/math/cryptominisat/pkg-plist: [8]: installing shared libraries, please define USE_LDCONFIG as appropriate
>Maybe set SOVERSION, so the library path can be cached by ldconfig(8)
and used not only via RPATH.
Yup, but what's the proper way to do this? Make a symlink at post-install?

>USES=python with no version specified is for ports that support both
python2 and python3. CMakeLists.txt seems to disagree:

As with find_package(Boost 1.46) case, this version number is a minimum version required. Since cryptominisat author doesn't check for actual version of python that have been found, i thought it should work with any python that is 2.7+.

> Why not create a separate USE_PYTHON=distutils port that depends on this one?
I don't see why it's required. The setup.py script is still configured by CMake, so, from my understanding, i would have to run either cmake or setup.py manually.

Regarding other problems, i'll fix them and resubmit the port.
Thanks for review.
Comment 4 Jan Beich freebsd_committer 2015-05-05 20:14:33 UTC
Created attachment 156406 [details]
PYTHON as separate ports, v0

(In reply to 6yearold from comment #3)
> (In reply to Jan Beich from comment #1)
>>WARN: /usr/ports/math/cryptominisat/pkg-plist: [8]: installing shared
>>libraries, please define USE_LDCONFIG as appropriate
>>Maybe set SOVERSION, so the library path can be cached by ldconfig(8)
>>and used not only via RPATH.
> Yup, but what's the proper way to do this? Make a symlink at post-install?

After testing it works fine with just USE_LDCONFIG. Broken ldconfig -r
output can be ignored as it doesn't affect LIB_DEPENDS implementation
for a few years. If you still want SOVERSION try:

  set_target_properties(libcryptominisat4 PROPERTIES
  ...
  SOVERSION 0
  )

>>USES=python with no version specified is for ports that support both
>>python2 and python3. CMakeLists.txt seems to disagree:
>
> As with find_package(Boost 1.46) case, this version number is a minimum version
> required. Since cryptominisat author doesn't check for actual version of python
> that have been found, i thought it should work with any python that is 2.7+.

Try building with:

  # in make.conf
  DEFAULT_VERSIONS = python=3.4

which would either not find suitable python version or fail as

  pycryptosat.cpp:68:10: error: use of undeclared identifier 'PyInt_Check'
      if (!IS_INT(lit))  {
           ^
  pycryptosat.cpp:21:21: note: expanded from macro 'IS_INT'
  #define IS_INT(x)  (PyInt_Check(x) || PyLong_Check(x))
                      ^
  pycryptosat.cpp:366:11: error: no member named 'ob_type' in 'Solver'
      self->ob_type->tp_free((PyObject*)self);
      ~~~~  ^
  pycryptosat.cpp:409:5: error: cannot initialize a member subobject of type 'Py_ssize_t' (aka 'long') with
        an lvalue of type 'const char [19]'
      "pycryptosat.Solver",             /*tp_name*/
      ^~~~~~~~~~~~~~~~~~~~
  pycryptosat.cpp:412:5: error: cannot initialize a member subobject of type 'printfunc' (aka 'int
        (*)(PyObject *, FILE *, int)') with an rvalue of type 'destructor' (aka 'void (*)(PyObject *)'):
        different number of parameters (3 vs 1)
      (destructor)Solver_dealloc, /*tp_dealloc*/
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
  pycryptosat.cpp:427:5: error: cannot initialize a member subobject of type 'const char *' with an rvalue
        of type 'unsigned long'
      Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/local/include/python3.4/object.h:647:29: note: expanded from macro 'Py_TPFLAGS_DEFAULT'
  #define Py_TPFLAGS_DEFAULT  ( \
                              ^
  pycryptosat.cpp:428:5: error: cannot initialize a member subobject of type 'traverseproc' (aka 'int
        (*)(PyObject *, visitproc, void *)') with an lvalue of type 'const char [15]'
      "Solver objects",           /* tp_doc */
      ^~~~~~~~~~~~~~~~
  pycryptosat.cpp:435:5: error: cannot initialize a member subobject of type 'struct PyMemberDef *' with an
        lvalue of type 'PyMethodDef [4]'
      Solver_methods,             /* tp_methods */
      ^~~~~~~~~~~~~~
  pycryptosat.cpp:436:5: error: cannot initialize a member subobject of type 'struct PyGetSetDef *' with an
        lvalue of type 'PyMemberDef [1]'
      Solver_members,            /* tp_members */
      ^~~~~~~~~~~~~~
  pycryptosat.cpp:443:5: error: cannot initialize a member subobject of type 'allocfunc' (aka 'PyObject
        *(*)(struct _typeobject *, Py_ssize_t)') with an rvalue of type 'initproc' (aka 'int (*)(PyObject *,
        PyObject *, PyObject *)'): different number of parameters (2 vs 3)
      (initproc)Solver_init,      /* tp_init */
      ^~~~~~~~~~~~~~~~~~~~~
  pycryptosat.cpp:445:5: error: cannot initialize a member subobject of type 'freefunc'
        (aka 'void (*)(void *)') with an lvalue of type
        'PyObject *(PyTypeObject *, PyObject *, PyObject *)': different number of parameters (1 vs 3)
      Solver_new,                 /* tp_new */
      ^~~~~~~~~~
  pycryptosat.cpp:458:9: error: non-void function 'initpycryptosat' should return a value [-Wreturn-type]
          return;
          ^
  pycryptosat.cpp:460:9: error: use of undeclared identifier 'Py_InitModule3'
      m = Py_InitModule3("pycryptosat", module_methods,
          ^

Also, USES+=python:build doesn't account for libpython dependency.

  $ readelf -d .../pycryptosat.so | fgrep NEEDED
   0x0000000000000001 (NEEDED)             Shared library: [libcryptominisat4.so]
   0x0000000000000001 (NEEDED)             Shared library: [libpython2.7.so.1]
   0x0000000000000001 (NEEDED)             Shared library: [libc++.so.1]
   0x0000000000000001 (NEEDED)             Shared library: [libcxxrt.so.1]
   0x0000000000000001 (NEEDED)             Shared library: [libm.so.5]
   0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
   0x0000000000000001 (NEEDED)             Shared library: [libc.so.7]

>> Why not create a separate USE_PYTHON=distutils port that depends on this one?
> I don't see why it's required. The setup.py script is still configured by
> CMake, so, from my understanding, i would have to run either cmake or setup.py
> manually.

It was merely a suggestion (not requirement) to:
- let the ports framework set the build environment for you
- provide python bindings for package users

I'm attaching an example with all of the mentioned issues addressed.
Hopefully, slave deduplication doesn't make the approach hard
to understand.
Comment 5 Gleb Popov freebsd_committer 2015-05-05 21:04:14 UTC
(In reply to Jan Beich from comment #4)
>After testing it works fine with just USE_LDCONFIG. Broken ldconfig -r
output can be ignored as it doesn't affect LIB_DEPENDS implementation
for a few years.

Linking with libcryptominisat4.so requires SOVERSION too, so it's better to keep it.

>I'm attaching an example with all of the mentioned issues addressed.
Ok, thanks. Since you've done all work for me, then you could commit it?
Comment 6 commit-hook freebsd_committer 2015-05-05 21:54:55 UTC
A commit references this bug:

Author: jbeich
Date: Tue May  5 21:54:33 UTC 2015
New revision: 385495
URL: https://svnweb.freebsd.org/changeset/ports/385495

Log:
  math/cryptominisat, math/py-cryptominisat: add new ports

  CryptoMiniSat is a modern, multi-threaded, feature-rich, simplifying SAT
  solver, featuring over 100 configurable parameters to tune to specific
  need, collection of statistical data to MySQL database + javascript-based
  visualization of it and clean C++ and python interfaces.

  WWW: http://www.msoos.org/cryptominisat4/

  PR:		199929
  Submitted by:	6yearold@gmail.com (based on)

Changes:
  head/math/Makefile
  head/math/cryptominisat/
  head/math/cryptominisat/Makefile
  head/math/cryptominisat/distinfo
  head/math/cryptominisat/files/
  head/math/cryptominisat/files/patch-cryptominisat4_CMakeLists.txt
  head/math/cryptominisat/files/patch-cryptominisat4_bva.cpp
  head/math/cryptominisat/pkg-descr
  head/math/cryptominisat/pkg-plist
  head/math/py-cryptominisat/
  head/math/py-cryptominisat/Makefile
  head/math/py-cryptominisat/files/
  head/math/py-cryptominisat/files/patch-pycryptosat.cpp
Comment 7 Jan Beich freebsd_committer 2015-05-05 21:58:56 UTC
Thanks. Committed. Hopefully the port makes it into the next periodic package build.