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 build static vs. shared libs #588

Closed
KerstinKeller opened this issue Jul 21, 2017 · 5 comments
Closed

CMake build static vs. shared libs #588

KerstinKeller opened this issue Jul 21, 2017 · 5 comments

Comments

@KerstinKeller
Copy link

I have seen that the CMake build is now supported and it exports its targets with a config, which is great.
However, I am a little confused as on how you're handling static vs dynamic libs.

From my understanding, with a CMake build, you specify BUILD_SHARED_LIBS=ON or BUILD_SHARED_LIBS=OFF and your library will be build accordingly. However, your configuration allows to build both, but exports them with different names (tinyxml2 vs tinyxml2_static).

I am also developing a library, which uses tinyxml2, but in my CMakeLists.txt files, I don't want to care about if someone has installed the static or non-static version of tinyxml2.
I would have to do something like

find_package(tinyxml2 REQUIRED)
if (TARGET tinyxml2)
target_link_libraries(my_lib PRIVATE tinyxml2)
else()
target_link_libraries(my_lib PRIVATE tinyxml2_static)
endif()

So basically this is not a bug or anything, but I am just wondering if you're willing to reconsider this design.

On a sidenote, many libraries chose to export their libraries to a namespace, e.g.
export(EXPORT tinyxml2 NAMESPACE tinyxml2) to later be referenced as tinyxml2::tinyxml2. I guess as tinyxml2 only has one component, it is not strictly necessary, but it could be helpful in seeing that you're referencing an installed library.

@leethomason
Copy link
Owner

I should probably be more involved in all the build options than I am, and I hope someone else can contribute an answer. But it's always my hope that including source will work.

@jasjuang
Copy link
Contributor

@KerstinKeller Using namespace for export is definitely the modern approach but it might break all current builds. Do you plan to submit a PR for this?

@leethomason I can do this sometime later if you don't mind require everyone who use tinyxml2 to change target link to tinyxml2::tinyxml2 from target link to tinyxml2 in their downstream CMakeLists.

@johnb003
Copy link
Contributor

johnb003 commented Jun 21, 2018

You can have both actually.

add_library(tinyxml2::tinyxml2 ALIAS tinyxml2)

FWIW: Yes! Please add the namespace. This is currently non-idiomatic. Also here's some reasons:

  • With namespace targets like Foo::Foo, if it's missing the config will give you an error rather than just silently adding -lFoo
  • The namespace is a nice way to tell when a library is imported vs part of the project, as most people do follow that standard for imported targets.

@lsolanka
Copy link
Contributor

Would this pull request originally intended for Hunter package manager be of interest? hunter-packages#2

It essentially does what you are suggesting (+introduces CMakePackageConfigHelpers to generate the config file). I just didn't have time to submit it here as I was working on other stuff.

It could pretty much be applied out of the box with minimal modifications.

Of course downstream projects would no longer be able to use tinyxml2_static.

@jasjuang
Copy link
Contributor

@lsolanka The PR hunter-packages#2 looks great to me too.

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

No branches or pull requests

5 participants