10000 Tweak cmake by ByteHamster · Pull Request #78 · jermp/pthash · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Tweak cmake #78

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

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Tweak cmake #78

merged 1 commit into from
Dec 6, 2024

Conversation

ByteHamster
Copy link
Contributor
  • When setting the include path to the main folder, projects depending on pthash would have stuff like README.md in their include paths. Also, using absolute paths (instead of include directories) causes problems when using pthash as a dependency, especially when running into diamond dependencies.
  • Explicitly set compiler warnings only when compiling PTHash itself. Otherwise these get propagated to projects depending on it.
  • With modern cmake, there is no need to specify "-pthread". Cmake does that for us if necessary and supported by the OS.

- When setting the include path to the main folder, projects depending
  on pthash would have stuff like `README.md` in their include paths.
  Also, using absolute paths (instead of include directories) causes
  problems when using pthash as a dependency, especially when running
  into diamond dependencies.
- Explicitly set compiler warnings only when compiling PTHash itself.
  Otherwise these get propagated to projects depending on it.
- With modern cmake, there is no need to specify "-pthread". Cmake does
  that for us if necessary and supported by the OS.
@jermp
Copy link
Owner
jermp commented Dec 6, 2024

Nice simplifications, thanks!

@jermp jermp merged commit 99e7c67 into jermp:dev Dec 6, 2024
9 checks passed
@ByteHamster ByteHamster deleted the cmake-tweaks branch December 6, 2024 09:42
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
0