Fix installation of cryptominisat and pycryptosat

[dimpase]

pycryptosat is meant to be installed as a part of cryptominisat,
but it does not happen.

This is because it's installing the Python module in SAGE_ROOT/local/
From the log:

-- Found python interpreter, libs and header files
-- Building python interface
-- Python CFLAGS:  '-Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -ffile-prefix-map=/build/python3.9-RNBry6/python3.9-3.9.2=.  -fstack-protector-strong -Wformat -Werror=format-security  -g -fwrapv -O2   '
-- Python LDFLAGS: '-lcrypt -lpthread -ldl  -lutil -lm'
-- Python LINKFORSHARED flags: '-Xlinker -export-dynamic -Wl,-O1 -Wl,-Bsymbolic-functions'
-- Python module will be installed to : '/home/dimpase/work/sage/local'
-- Configuring done

See
https://groups.google.com/d/msgid/sage-release/cc134257-34d8-4c26-8719-24a994dc42dan%40googlegroups.com

./sage --pip install pycryptosat

can be used as a workaround

CC: @mkoeppe @slel @vbraun

Component: packages: optional

Keywords: cryptominisat

Author: Dima Pasechnik

Branch: cdd19b0

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/33162

[dimpase]

comment:1

in cmake config of cryptominisat on sees

if(DEFINED ENV{PYCRYPTOSAT_INSTALL_PATH})
    set(PY_INSTALL_PREFIX "--prefix=$ENV{PYCRYPTOSAT_INSTALL_PATH}")
elseif(DEFINED ENV{VIRTUAL_ENV})
    set(PY_INSTALL_PREFIX "")
else()
    set(PY_INSTALL_PREFIX "--prefix=${CMAKE_INSTALL_PREFIX}")
endif()

so one can explicitly set the path for the module. But to what value?

[slel]

comment:2

See also

• Upgrade cryptominisat to 5.8.0, fix build on Cygwin #25374: Upgrade cryptominisat to fix cryptominisat build on Cygwin

[slel]

Changed keywords from none to cryptominisat

[mkoeppe]

comment:4

In principle, it would be set to ${SAGE_VENV}.
But it is not a correct solution, as explained in #25374 comment:39

[dimpase]

comment:5

Replying to @mkoeppe:

In principle, it would be set to ${SAGE_VENV}.
But it is not a correct solution, as explained in #25374 comment:39

I don't see why it's not OK. Passing PYCRYPTOSAT_INSTALL_PATH=$SAGE_VENV
would make sure the python package installs in the correct venv, while the rest goes where it should.

[mkoeppe]

comment:6

Because of our installation records in SAGE_LOCAL/var/lib/sage/installed/ and SAGE_VENV/var/lib/sage/installed.

[dimpase]

comment:7

Replying to @mkoeppe:

Because of our installation records in SAGE_LOCAL/var/lib/sage/installed/ and SAGE_VENV/var/lib/sage/installed.

can we allow Python modules in SAGE_LOCAL, too? As a temporary fix, at least.

[mkoeppe]

comment:8

no, they won't be in the load path.

[mkoeppe]

comment:9

Solution is as I said in #25374 comment:48

[dimpase]

Commit: cdd19b0

[dimpase]

Author: Dima Pasechnik

[dimpase]

comment:10

here is an easier fix.

Made it blocker, as this fixes an otherwise totally broken optional package.

---

New commits:

cdd19b0

correctly install pycryptosat

[dimpase]

Branch: u/dimpase/packages/cryptominisat-fix

[mkoeppe]

comment:11

This is not enough because it will break when you switch to a different Python.

[dimpase]

comment:12

cryptominisat depends on
$(PYTHON), thus such a switch will trigger a rebuild.

[mkoeppe]

comment:13

Nope

[dimpase]

comment:14

then I don't understand what "switch" means.

[mkoeppe]

comment:15

When a system python is in use, $(PYTHON) is the path to $(SAGE_VENV)/pyvenv.cfg. This file is created on creation of the venv for the configured python and is never updated.

Switching between pythons = Re-running configure, with or without --with-python=...., which selects different system python versions.

[dimpase]

comment:16

and how does one trigger rebuild of a Pyhton package in such a case?

[mkoeppe]

comment:17

Installation records for Python packages are kept in $(SAGE_VENV)/var/lib/sage/installed.
Different Python version -> Different SAGE_VENV -> different installation records.

[mkoeppe]

Reviewer: Matthias Koeppe

[dimpase]

comment:35

Replying to @mkoeppe:

The installation of the Python package yes, but not the library and not the installation record!

the library still goes to SAGE_LOCAL.

Imho all the sdh_ - scripts correctly set installation records of Sage.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

e3d8e91

spin off installation of cryptominisat's Python

[sagetrac-git]

Changed commit from cdd19b0 to e3d8e91

[dimpase]

comment:37

Is this what you kept in mind? Turns out one just needs to run cmake to get pycryptosat set up, no need to build the library twice.

[mkoeppe]

comment:38

Replying to @dimpase:

Imho all the sdh_ - scripts correctly set installation records of Sage.

The installation records are set by sage-spkg, not by sdh_....
One record for one package.

[mkoeppe]

comment:39

Replying to @dimpase:

Is this what you kept in mind? Turns out one just needs to run cmake to get pycryptosat set up, no need to build the library twice.

Almost. Still need build/pkgs/pycryptosat/install-requires.txt so that the installation record goes into SAGE_VENV, not in SAGE_LOCAL

[mkoeppe]

comment:40

And build/pkgs/pycryptosat/checksums.ini should be a symlink, not a copy, of cryptominisat's.
(Like it is done for gcc/gfortran)

[mkoeppe]

comment:41

Are the pycryptosat distros correct? Do the "cryptominisat" distro packages really install the Python bindings?

[mkoeppe]

comment:42

Also a good moment to add upstream_url in checksums.ini

[mkoeppe]

comment:43

Also note that the positively reviewed version of this ticket is already on Volker's branch, so best to reset to that and continue on a follow-up ticket

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

[sagetrac-git]

Changed commit from e3d8e91 to cdd19b0

[dimpase]

comment:45

ok, this is the old fix

[dimpase]

comment:46

followup on #33183

[dimpase]

comment:47

Replying to @mkoeppe:

Are the pycryptosat distros correct? Do the "cryptominisat" distro packages really install the Python bindings?

I don't know - as spkg-configure is not yet there, it can be left as such for the time being.

[vbraun]

Changed branch from u/dimpase/packages/cryptominisat-fix to cdd19b0

[seblabbe]

comment:49

It seems something is missing (maybe a dependency) as it is now broken on my machine, see https://groups.google.com/g/sage-release/c/1YV0dNtEZz4/m/Tm7RSWINAgAJ

Is there a ticket for it already? I think I have seen a report of the failure earlier this week by someone else, I don't remember where.

[seblabbe]

Changed commit from cdd19b0 to none

[dimpase]

comment:50

missing python toolchain in deps of cryptominisat.

see #33183