Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update build instructions to install needed shared libraries #775

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gavinp
Copy link

@gavinp gavinp commented Dec 21, 2024

It seems that the default build of cryptominisat5 used shared libraries that weren't installed using these build instructions.

I've added the (scary) minimal stuff I did that makes it work right away.

It seems that the default build of cryptominisat5 used shared libraries that weren't installed using these build instructions.

I've added the (scary) minimal stuff I did that makes it work right away.
@msoos
Copy link
Owner

msoos commented Dec 21, 2024

Hi,

Thanks! That's true, the installation is not included. At the same time, to run the executable from where it's at, it's not necessary to install these libraries. Maybe we should make a separate install instruction set? Installing things by default can make people's systems pretty messy, e.g. if they want o later install a different version of the library, or they install it later at a different place (e.g. /usr/local/lib instead of /usr/lib). So nowadays I am cautious of giving instructions to install things. I have seen colleagues install things to /usr/lib and then have trouble with updated libraries colliding.

What do you think?

Thanks again,

Mate

@gavinp
Copy link
Author

gavinp commented Dec 22, 2024

I think your solution is absolutely the best. Installation is indeed not needed; but the executable built in the quick start instructions presumes installed dynamic libraries; one of them from a program that doesn't even install dynamic libraries! Hence my PR including the super scary "cp" to /usr/lib.

Either way, the quick start instructions are materially wrong, and in a way that can confuse many users.

I'll take a few minutes to poke at them and come up with instructions to statically link or something. Feel free to close this PR if I leave it alone too long, but I should come back with something better.

I'm enjoying looking at the state of open source SAT. I'm curious about the interface between SAT and mixed integer programming solvers.

@msoos
Copy link
Owner

msoos commented Dec 22, 2024

Hi,

Yeah, good point. BTW, it's trivial to build statically! The GitHub actions actually compiles statically:

https://github.com/msoos/cryptominisat/actions/runs/12244747429

Basically, the only difference is to add -DSTATICCOMPILE=ON to the cmake of CryptoMiniSat and you'll be good to go:

cmake  -DSTATICCOMPILE=ON ..
make

Done :) In fact, the reason I use a branch of cadical/cadiback is because by default they don't create both dynamic&static libraries, which would make the above impossible. But anyway, you can also just download it the latest CryptoMiniSat from the Artifacts of GitHub Actions. That will do the trick :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants