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

CMake: Use imported target for fmt in tests #11510

Merged
merged 1 commit into from Jan 30, 2023

Conversation

TellowKrinkle
Copy link
Contributor

This properly adds the header include paths when using system fmt

This properly adds the header include paths when using system fmt
@Pokechu22
Copy link
Contributor

I'm not really that experienced with cmake; what's the difference between the two of these? (I do see that everything else uses fmt::fmt already, and I probably started with that but then changed fmt when I ran into problems and never changed it back afterwards.)

@TellowKrinkle
Copy link
Contributor Author

CMake loves its backwards compatibility, so lots of things just silently use various fallbacks instead of being errors.

If target_link_libraries(target PUBLIC library) can't find a target named library declared in cmake, it falls back to adding -llibrary (unless the library name contains ::, in which case the fallback is disabled and it actually errors). In this case, the bundled fmt creates a fmt target, which it then aliases to fmt::fmt. find_package(fmt) on the other hand writes a bunch of variables for old-style cmake, and makes a fmt::fmt imported target, but does not make a fmt target.

This means that when you link fmt, you get the target if you use the submodule, but just -lfmt for the package, which may or may not work depending on whether the fmt headers are already in an include directory required by some other package.

Most modern cmake packages will create such a :: alias when used as a submodule, and make a matching imported target from their find_package script, so that's generally the way to go for any dependency that you want to get from either a submodule or the system.

@AdmiralCurtiss AdmiralCurtiss merged commit fa1fec7 into dolphin-emu:master Jan 30, 2023
14 checks passed
@TellowKrinkle TellowKrinkle deleted the FmtImport branch January 30, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants