Bug 1981466 - stp-devel should depend on minisat2-devel and cryptominisat-devel
Summary: stp-devel should depend on minisat2-devel and cryptominisat-devel
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: stp
Version: 34
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jerry James
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-07-12 15:49 UTC by Lukáš Zaoral
Modified: 2021-07-24 02:16 UTC (History)
2 users (show)

Fixed In Version: stp-2.3.3-16.fc33 stp-2.3.3-16.fc34
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-07-24 01:08:02 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Lukáš Zaoral 2021-07-12 15:49:23 UTC
Description of problem:
STP CMake target defined by the STP CMake module depends directly on /lib64/libminisat.so which is provided by minisat2-devel. This package, however, does not depend on it.

Version-Release number of selected component (if applicable): latest

How reproducible: always

Steps to Reproduce:
1. Create CMakeListst.txt with
```
cmake_minimum_required(VERSION 3.5)
project(test)

find_package(STP CONFIG REQUIRED)
add_executable(test main.c)
target_link_libraries(test stp)
```
2. Create main.c with empty main.
3. cmake .
4. make

Actual results:
[ 50%] Building C object CMakeFiles/test.dir/main.c.o
make[2]: *** No rule to make target '/lib64/libminisat.so', needed by 'test'.  Stop.

Expected results: '/lib64/libminisat.so' is present in the system and everything compiles successfully.

Comment 1 Lukáš Zaoral 2021-07-13 07:54:31 UTC
After some tinkering, I've also found out that the cryptominisat-devel package is required to successfully compile the example above as well. I already had it installed on my machine so I missed this one. Sorry for that.

Comment 2 Jerry James 2021-07-13 16:04:39 UTC
Thanks for the bug report.  But I have to wonder if depending on minisat2-devel and cryptominisat-devel is the correct solution.  The /usr/include/stp/c_interface.h header file makes no direct references to either minisat2 or cryptominisat.  The library file libstp.so.2.3 is already linked with both libminisat and libcryptominisat, so consumers of libstp do not need to link directly to either, unless those consumers themselves make direct use (i.e., not via libstp) of libminisat or libcryptominisat.

In short, I wonder if the correct solution is to remove the INTERFACE_LINK_LIBRARIES declaration from STPTargets.cmake.  What do you think?

Comment 3 Lukáš Zaoral 2021-07-14 12:43:02 UTC
Yes, you are right.  

According to https://github.com/stp/stp/blob/876589d45f656f13cefeb04a2f13005d0fa0c932/lib/CMakeLists.txt#L113, upstream does it so that everything is linked together when a static build of stp is used by other projects.

However, the INTERFACE_LINK_LIBRARIES property is populated regardless of the build type so something like this should fix it:

--- a/lib/CMakeLists.txt
+++ b/lib/CMakeLists.txt
@@ -142,9 +142,15 @@ if (USE_RISS)
         ${stp_link_libs} -lriss-coprocessor)
 endif()

+if (STATICCOMPILE)
 target_link_libraries(stp
     LINK_PUBLIC ${stp_link_libs}
 )
+else()
+target_link_libraries(stp
+    LINK_PRIVATE ${stp_link_libs}
+)
+endif()

 install(TARGETS stp
     EXPORT ${STP_EXPORT_NAME}

Comment 4 Jerry James 2021-07-14 15:18:43 UTC
That does indeed seem to work.  Thanks!  I've added that patch and started a Rawhide build.  Do you need patched builds for F33 or F34?

Comment 5 Lukáš Zaoral 2021-07-15 07:16:25 UTC
Yes, that would be great because I found this problem during packaging of bug 1981739.  Thanks!

Comment 6 Fedora Update System 2021-07-15 15:39:08 UTC
FEDORA-2021-1738ad73db has been submitted as an update to Fedora 34. https://bodhi.fedoraproject.org/updates/FEDORA-2021-1738ad73db

Comment 7 Fedora Update System 2021-07-16 01:57:32 UTC
FEDORA-2021-494152ddfc has been pushed to the Fedora 33 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-494152ddfc`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-494152ddfc

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 8 Fedora Update System 2021-07-16 02:03:59 UTC
FEDORA-2021-1738ad73db has been pushed to the Fedora 34 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-1738ad73db`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-1738ad73db

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 9 Fedora Update System 2021-07-24 01:08:02 UTC
FEDORA-2021-494152ddfc has been pushed to the Fedora 33 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 10 Fedora Update System 2021-07-24 02:16:16 UTC
FEDORA-2021-1738ad73db has been pushed to the Fedora 34 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.