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

Merged

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

@p-senichenkov
Copy link
Collaborator

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
@Vdaleke
Copy link
Collaborator Author

Vdaleke commented Jan 3, 2025

I made some research and suggest to stay like this for several reasons.

  1. I noticed different behaviour of CMake with myself-built Boost and Homebrew Boost. With Homebrew boost there are full DYLD_PATHs for Boost libs and, as I wrote above, for gcc-built boost it's not. I looked through CMake FindBoost module and some homebrew files and can't get the answer. I asked about this on CMake discourse and hope for some answers.

  2. Due to last point I don't know how to set RPATH correctly if it's so unstable except the way of adding "" and "/usr/local/ there, but this is strange solution as for me.

  3. We have PR where it's possible to build with Apple Clang and Homebrew Boost and this is the easiest solution.

  4. We need these CMake fixes and there is no reason to spend more time on this.

@Vdaleke Vdaleke marked this pull request as ready for review January 3, 2025 02:13
@Vdaleke
Copy link
Collaborator Author

Vdaleke commented Jan 3, 2025

As an addition to the previous comment, I had an idea to install boost built with gcc by Homebrew, I recently found out that this is can be possible with 'brew install boost -s' option which means 'build from source', and I have some success in it, but it was stopped by impossibility of setting MacOSX14.sdk instead of broken MacOSX15.sdk. I created an issue to brew repo and waiting for reaction.

@vs9h
Copy link
Collaborator

vs9h commented Jan 5, 2025

I made some research and suggest to stay like this for several reasons.

  1. I noticed different behaviour of CMake with myself-built Boost and Homebrew Boost. With Homebrew boost there are full DYLD_PATHs for Boost libs and, as I wrote above, for gcc-built boost it's not. I looked through CMake FindBoost module and some homebrew files and can't get the answer. I asked about this on CMake discourse and hope for some answers.
  2. Due to last point I don't know how to set RPATH correctly if it's so unstable except the way of adding "" and "/usr/local/ there, but this is strange solution as for me.
  3. We have PR where it's possible to build with Apple Clang and Homebrew Boost and this is the easiest solution.
  4. We need these CMake fixes and there is no reason to spend more time on this.

In general, I agree that the current solution should be sufficient and we can stop here.

Regarding the first point, I don't know what the problem is.

Regarding rpath, it seems to me that this solution may be even worse than the current one. I don't think anyone does this in existing projects. We can do this only if we are sure that the installation occurs along single specific path. But we probably don't have such guarantees.

@chernishev chernishev merged commit 3fa1249 into Desbordante:main Jan 5, 2025
20 checks passed
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.

5 participants