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

Fixes for cmake shared library detection. #10705

Merged
merged 1 commit into from Sep 23, 2022
Merged

Conversation

Zopolis4
Copy link
Contributor

These changes were originally going to be part of a series of changes that let dolphin build on ChromeOS, but then it...built. Anyways, these should make the cmake build steps a bit less noisy, now that it isint complaining about not having a Findfmt.cmake and hidapi can see FindLIBUDEV.cmake.

Most of the Find*.cmake files also need a bit of an overhaul, they're wildly inconsistent on capitalisation, indentation, ordering, naming, arguments, etc. I don't feel like I know enough about cmake or the coding styles, but I thought I'd put it out there. We should also have a FindQt.cmake, It'd remove the need for all the CMAKE_PREFIX_PATH nonsense and should make building on esoteric platforms or configurations much easier. Again, I don't know enough about cmake to confidently make one, just putting it out there.

CMakeLists.txt Outdated Show resolved Hide resolved
@Rumi-Larry
Copy link

Do ".cmake" files not need a legal header?

@Zopolis4
Copy link
Contributor Author

Do ".cmake" files not need a legal header?

A bunch of them don't, it's a bit of a mess. I'm not sure, a couple of them that we presumably sourced from elsewhere don't.

@shuffle2
Copy link
Contributor

shuffle2 commented Jun 3, 2022

👍no idea if the cmake changes are "good" but i've been seeing these warnings for a while and would be nice if they went away.

@Zopolis4
Copy link
Contributor Author

Zopolis4 commented Aug 5, 2022

I've removed the findfmt.cmake due to conflicts with target importing.

Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

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

Based on https://cmake.org/cmake/help/latest/command/find_package.html#search-modes it is case-sensitive for Find<PackageName>.cmake (implied by the existence of <lowercasePackageName>-config.cmake and <PackageName>Config.cmake below), and the variable name is <PackageName>_FOUND (for which we use LIBUDEV_FOUND and not Libudev_FOUND), so this seems reasonable to me. Untested.

@JMC47 JMC47 merged commit 08f78b1 into dolphin-emu:master Sep 23, 2022
11 checks passed
@Zopolis4 Zopolis4 deleted the cmake branch September 23, 2022 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants