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

Introduce modern way to link with pthread library #1227

Merged
merged 8 commits into from
Sep 7, 2023

Conversation

fspindle
Copy link
Contributor

@fspindle fspindle commented Sep 4, 2023

These changes are related to Conda packaging where the following issues was encountered when visp conda package is used as a third-party. In VISPModules.cmake there remain the full absolute path of the m and pthread libraries used during the package generation. When installing the conda package on a linux target, conda is unable to replace these locations with the corresponding ones on the target.
The fix consists in:

  • for lib m to export the library as private (Fixed in PR # 1226 commit e2ed222)
  • for lib pthread to use modern cmake (commit 9de4530 and d6e5a2f)
  • for openmp that has a dependency to pthread to consider the specific case of pthread as above (commit 9209898)
  • as a side effect, ms2_32.lib that was part of VISP_LINKER_LIBS that contains now only private libraries, was removed from VISP_LINKER_LIBS and considered as a classical 3rd party linked to visp_core module (see commit 2c074db)

See issue #1228

…build the embedded pthread library on windows
…conda package usage

Because there is an explicit link to libpthread location that breaks visp conda package usage on linux
we cannot use OpenMP_CXX_LIBRARIES that contains /usr/lib/gcc/x86_64-linux-gnu/11/libgomp.so;/usr/lib/x86_64-linux-gnu/libpthread.a
by adding:
list(APPEND opt_libs ${OpenMP_CXX_LIBRARIES})
That's why we need to use the modern way to consider pthread dependency
VISP_LINKER_LIBS are used as private
- there is the need to make ws2_32.lib as a 3rd party public library
- Consider now ws2_32.lib as a public  3rd party that is linked to core
  module
- Rename deprecated LINK_PUBLIC and LINK_PRIVATE to PUBLIC and PRIVATE respectively
@fspindle
Copy link
Contributor Author

fspindle commented Sep 6, 2023

@olivier-roussel Moving VISP_LINKER_LIBS as private introduced a side effect on Windows where ws2_32.lib was not exported as 3rd party. Fixed in commit 2c074db

Without this change, vpNetwork class is not usable in a project on windows with ViSP as a 3rd party

@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (8f39d87) 54.34% compared to head (769f884) 54.34%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1227   +/-   ##
=======================================
  Coverage   54.34%   54.34%           
=======================================
  Files         405      405           
  Lines       49827    49827           
=======================================
+ Hits        27077    27078    +1     
+ Misses      22750    22749    -1     
Files Changed Coverage Δ
modules/core/include/visp3/core/vpImageCircle.h 0.00% <ø> (ø)

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fspindle
Copy link
Contributor Author

fspindle commented Sep 7, 2023

Closes #1228

@fspindle fspindle merged commit 07da184 into lagadic:master Sep 7, 2023
58 checks passed
@fspindle fspindle deleted the fix_pthread_conda branch July 3, 2024 07:14
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.

1 participant