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

Fix CMakeLists.txt, macOS guide and refactor .gitignore #492

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Vdaleke
Copy link
Collaborator

@Vdaleke Vdaleke commented Nov 11, 2024

Increase CMake minimum required version to 3.15 due to incompatibilities
with scikit-build-core
. Update README.md.

Add POLICY checks to avoid errors on policy-unsupported versions.

Change CMP0167 policy of boost searching to OLD due to non-compliance
with "Modern CMake" development patterns. Add "TODO" to fix this later.
Add useful logs to understand what Boost libraries were found by CMake.

Apply sort to .gitignore to improve readability.
Add .DS_Store, which is generated in macOS directories.
Add .cache, which is generated by clangd.

Add python3 to installation guide.
Fix macOS Boost installation guide to work with python module.
Fix default installation directories as root permissions were required.
Change macOS SDK version to compatibility with new version.

@Vdaleke
Copy link
Collaborator Author

Vdaleke commented Nov 11, 2024

By the way, I checked the build with the new SDK and nothing changed. So the need to use the 14 SDK remains the same.

@Vdaleke Vdaleke requested a review from vs9h November 11, 2024 12:47
@Vdaleke Vdaleke marked this pull request as draft November 11, 2024 14:04
@Vdaleke Vdaleke force-pushed the fix/refactor-gitignore-and-readme branch from 6e56f4d to a84b9d6 Compare November 11, 2024 14:08
@Vdaleke Vdaleke changed the title Refactor .gitignore and macOS guide SDK version Refactor .gitignore and fix macOS guide Nov 11, 2024
@Vdaleke Vdaleke marked this pull request as ready for review November 11, 2024 14:10
@Vdaleke Vdaleke marked this pull request as draft November 11, 2024 19:17
@Vdaleke Vdaleke force-pushed the fix/refactor-gitignore-and-readme branch from a84b9d6 to 9af0119 Compare November 16, 2024 17:02
@Vdaleke
Copy link
Collaborator Author

Vdaleke commented Nov 16, 2024

I also comment export DYLD_LIBRARY_PATH. It's architecturally bad solution and not recommended to use.

The problem is that built python module can't find boost dynamic libraries:

>>> import desbordante
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: dlopen(/Users/vdaleke/Documents/desbordante-core/venv/lib/python3.12/site-packages/desbordante.cpython-312-darwin.so, 0x0002): Library not loaded: libboost_container-xgcc14-mt-a64-1_86.dylib
  Referenced from: <5DE266BA-6882-3835-BB89-3E13967FF861> /Users/vdaleke/Documents/desbordante-core/venv/lib/python3.12/site-packages/desbordante.cpython-312-darwin.so
  Reason: tried: 'libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OSlibboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), 'libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/usr/lib/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file, not in dyld cache), '/Users/vdaleke/Documents/desbordante-core/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/vdaleke/Documents/desbordante-core/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/Users/vdaleke/Documents/desbordante-core/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/usr/lib/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file, not in dyld cache)

The reason is that CMake doesn't set LC_LOAD_DYLIB cmd in ABI for boost libraries with correct path:

➜ otool -L /Users/vdaleke/Documents/desbordante-core/venv/lib/python3.12/site-packages/desbordante.cpython-312-darwin.so 
/Users/vdaleke/Documents/desbordante-core/venv/lib/python3.12/site-packages/desbordante.cpython-312-darwin.so:
	libboost_container-xgcc14-mt-a64-1_86.dylib (compatibility version 0.0.0, current version 0.0.0)
	libboost_thread-xgcc14-mt-a64-1_86.dylib (compatibility version 0.0.0, current version 0.0.0)
	libboost_graph-xgcc14-mt-a64-1_86.dylib (compatibility version 0.0.0, current version 0.0.0)
	/opt/homebrew/opt/gcc/lib/gcc/current/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.33.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1345.120.2)

I don't know why it's like this and is it possible to force CMake use full paths or rpath-relative paths. I searched enough and couldn't find a solution. I suggest to stay with this solution just to have a working guide for the moment. In next related PR I suggest to focus on porting project to C++ standard to use clang and homebrew Boost on macOS.

@Vdaleke Vdaleke force-pushed the fix/refactor-gitignore-and-readme branch 3 times, most recently from 911553a to b33efc5 Compare November 16, 2024 20:49
@Vdaleke Vdaleke changed the title Refactor .gitignore and fix macOS guide Fix CMakeLists.txt, macOS guide and refactor .gitignore Nov 16, 2024
CMakeLists.txt Show resolved Hide resolved
@Vdaleke Vdaleke marked this pull request as ready for review November 16, 2024 21:37
README.md Show resolved Hide resolved
@Vdaleke Vdaleke force-pushed the fix/refactor-gitignore-and-readme branch 2 times, most recently from 240077b to 8510a48 Compare November 18, 2024 09:01
Copy link
Collaborator

@vs9h vs9h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks good, I left a few comments and questions.

.gitignore Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@Vdaleke Vdaleke force-pushed the fix/refactor-gitignore-and-readme branch 3 times, most recently from d4cf129 to b7d3310 Compare December 9, 2024 17:51
@Vdaleke Vdaleke requested a review from vs9h December 9, 2024 17:53
Copy link
Collaborator

@vs9h vs9h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I can't verify whether it is correct to add such an export for DYLD_LIBRARY_PATH. I have not encountered this before.

I have no more comments on the changes, overall it looks good.

Add python3 to installation guide.
Fix macOS Boost installation guide to work with python module.
Fix default installation directories as root permissions were required.
Add sudo to ./b2 install command.
Change macOS SDK version to compatibility with new version.
Increase CMake minimum required version to 3.15 due to incompatibilities
with scikit-build-core. Update README.md.

Add POLICY checks to avoid errors on policy-unsupported versions.

Change CMP0167 policy of Boost searching to OLD due to non-compliance
with "Modern CMake" development patterns. Add "TODO" to fix this later.
Add useful logs to understand what Boost libraries were found by CMake.

Refactor .gitignore to improve readability:
Remove legacy unused files and directories;
Add .DS_Store, which is generated in macOS directories;
Add .cache, which is generated by clangd plugin in IDE.
@chernishev chernishev force-pushed the fix/refactor-gitignore-and-readme branch from b7d3310 to 4981cd2 Compare December 18, 2024 18:12
@BUYT-1
Copy link
Collaborator

BUYT-1 commented Dec 21, 2024

After the library is compiled, does it work if DYLD_LIBRARY_PATH is not set?

@Vdaleke
Copy link
Collaborator Author

Vdaleke commented Dec 21, 2024

After the library is compiled, does it work if DYLD_LIBRARY_PATH is not set?

I'm not sure about the meaning of 'does it work', so, as for Desbordante_test it works fine and as for build/target/desbordante.cpython-312-darwin.so which is built by sci-build-core not, the error is like this:

(venv) ➜  target git:(fix/refactor-gitignore-and-readme) ✗ python3                                                           
Python 3.12.5 (v3.12.5:ff3bc82f7c9, Aug  7 2024, 05:32:06) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import desbordante
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: dlopen(/Users/vdaleke/Documents/desbordante-core/build/target/desbordante.cpython-312-darwin.so, 0x0002): Library not loaded: libboost_container-xgcc14-mt-a64-1_86.dylib
  Referenced from: <2B68442A-C7AA-3FFB-A70C-C3B1D6F570A0> /Users/vdaleke/Documents/desbordante-core/build/target/desbordante.cpython-312-darwin.so
  Reason: tried: 'libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OSlibboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), 'libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/usr/lib/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file, not in dyld cache), '/Users/vdaleke/Documents/desbordante-core/build/target/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/vdaleke/Documents/desbordante-core/build/target/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/Users/vdaleke/Documents/desbordante-core/build/target/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file), '/usr/lib/libboost_container-xgcc14-mt-a64-1_86.dylib' (no such file, not in dyld cache)

I absolutely don't know why it works like this and what good ways to fix it. I attach otool -l output for both binaries, where presented cmds for dyld:
Desbordante_test https://pastebin.com/rxwv8xs1
desbordante.cpython-312-darwin.so https://pastebin.com/wq50BcXm

@Vdaleke
Copy link
Collaborator Author

Vdaleke commented Dec 22, 2024

I found some interesting c make variables which may be help to force include @rpath to dynamic libraries paths which is boost, I will try it later

image

https://gitlab.kitware.com/cmake/community/-/wikis/doc/cmake/RPATH-handling#default-rpath-settings

@Petua41
Copy link
Collaborator

Petua41 commented Dec 22, 2024

Unfortunately, I can't verify whether it is correct to add such an export for DYLD_LIBRARY_PATH. I have not encountered this before.

DYLD_LIBRARY_PATH is equivalent of Linux LD_PATH, so yes, it's needed to be exported like this for ld to find shared libraries installed into /usr/local/lib (which is default path where brew installs them. Also, Boost is installed here).

Another way to do this is to set CMake's LDFLAGS.

@chernishev chernishev marked this pull request as draft December 23, 2024 18:23
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.

4 participants