Added a config to export the library as a cmake target #66

hellozee · 2020-12-14T06:35:16Z

This exports the library as a cmake package with sqlpp11::sqlpp-mysql target. Don't know if it is necessary or not, but I needed it to link against my program when installed this library from vcpkg. I have kept it under the sqlpp11 namespace, since package name is pretty long.

src/CMakeLists.txt

CMakeLists.txt

hellozee · 2021-01-16T11:56:16Z

Sorry for being late, 😸

rbock · 2021-01-17T08:31:34Z

Thanks for the update. I appreciate the effort you put into this.

However, this now has hundreds of lines of code for the equivalent of git tag | sort -V | tail -n1.

Your initial statement was:

Don't know if it is necessary or not, but I needed it to link against my program when installed this library from vcpkg.

I have no relevant knowledge of vcpkg and my cmake skills are rather limited. I cannot imagine that anything like this is necessary to make them work together. Please do more research. There has to be a better way.

Thanks!

hellozee · 2021-01-17T13:15:22Z

However, this now has hundreds of lines of code for the equivalent of git tag | sort -V | tail -n1.

Sorry, that was me being lazy, I copied a whole module where I needed just a single function, removed that and replaced that with a small command.

Your initial statement was:

Don't know if it is necessary or not, but I needed it to link against my program when installed this library from vcpkg.

I have no relevant knowledge of vcpkg and my cmake skills are rather limited. I cannot imagine that anything like this is necessary to make them work together. Please do more research. There has to be a better way.

I can't link against the connector since no targets were exported corresponding to it. So unless the libraries and headers exist in the system path, it would be a hassle to use it. That is what I think, though I could be definitely wrong.

CMakeLists.txt

src/CMakeLists.txt

…ilable

hellozee · 2021-01-26T07:44:19Z

Put a check for the TAG variable if not set, I am out of ideas on how to tackle this if there is not .git or git isnt available in the system, :)

rbock · 2021-01-30T17:11:45Z

Thanks!

@Leon0402 and @GCarneiroA My cmake foo is insufficient to figure out if this would conflict with #65?

Leon0402 · 2021-01-30T17:32:29Z

src/CMakeLists.txt

+
+install(
+  FILES
+    "../cmake/sqlpp11-connector-mysqlConfig.cmake"


Why did you copy the config file in the binary directory in line 17, if you now use the path to the cmake directory directly?

Also instead of using "../cmake", you could also use "${PROJECT_SOURCE_DIR}/cmake/", which I personally like more. But that might be preference.

Leon0402 · 2021-01-30T17:34:13Z

src/CMakeLists.txt

+  "${CMAKE_CURRENT_BINARY_DIR}/cmake/sqlpp11-connector-mysqlConfig.cmake"
+  COPYONLY)
+
+set(ConfigPackageLocation lib/cmake/sqlpp11-connector-mysql)


To be compatible with all major distributions / OS, you should rather not hardcode the destinations path here. Instead cmake can figure out the correct directory:

include(GNUInstallDirs) set(ConfigPackageLocation ${CMAKE_INSTALL_LIBDIR}/cmake/sqlpp11-connector-mysql)

Leon0402 · 2021-01-30T17:35:26Z

src/CMakeLists.txt

-		bind_result.cpp
-		detail/connection_handle.cpp)
+add_library(sqlpp-mysql connection.cpp prepared_statement.cpp char_result.cpp
+                        bind_result.cpp detail/connection_handle.cpp)

 install(TARGETS sqlpp-mysql DESTINATION lib)


This can be removed I guess

Leon0402 · 2021-01-30T17:38:34Z

src/CMakeLists.txt

+  VERSION ${TAG}
+  COMPATIBILITY AnyNewerVersion)
+
+export(


This shouldn't be necessary, it is done by install(EXPORT ...) in line 23

Leon0402 · 2021-01-30T17:41:21Z

src/CMakeLists.txt

+  FILES
+    "../cmake/sqlpp11-connector-mysqlConfig.cmake"
+    "${CMAKE_CURRENT_BINARY_DIR}/cmake/sqlpp11-connector-mysqlConfigVersion.cmake"
+  DESTINATION ${ConfigPackageLocation})


Actually nothing here, but I can't comment on code you haven't changed (Why Github?). In the top level cmake file there is the installation command for the header files. You might wanna move them in here, to have everything installation related in one place

Leon0402 · 2021-01-30T17:42:14Z

cmake/sqlpp11-connector-mysqlConfig.cmake

@@ -0,0 +1,6 @@
+list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")


Seems to be not needed

Leon0402 · 2021-01-30T17:57:06Z

Thanks!

@Leon0402 and @GCarneiroA My cmake foo is insufficient to figure out if this would conflict with #65?

It should be alright as far as I can see.

But @GCarneiroA made some good changes over there, which this patch could benefit from. He added target specific commands like target_include_directories with the appropriate visibility (most often PUBLIC here). So everyone that links against this library will have set the include directories, libraries set automatically.
There is special handling for Installation though. The path of the headers will change. Therefore you would usually add some logic to make target_include_directories contain relative paths when building, but pathes to the installation folder when installing. It might look like this:

target_include_directories(mysql-connector 
   INTERFACE
       $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>
       $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
)

The INSTALL_INTERFACE part can be removed, when using INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} on the install(TARGETS ...) command. I use it normally, because you can't do anything wrong here :)

As far as I can see the @GCarneiroA took this into account in his patch, although the INSTALL_INTERFACE part isn't correct I think.

So I think it might be better to merge the other PR first and then rebase here. But I'm not sure if @GCarneiroA is still working at the other PR, it seems stale.

rbock · 2021-12-18T11:08:46Z

The mysql-connector now lives in the sqlpp11 repo.

Closing this before archiving. Please re-open in sqlpp11, if this is still relevant.

Thanks!

rbock reviewed Dec 15, 2020

View reviewed changes

src/CMakeLists.txt Outdated Show resolved Hide resolved

rbock reviewed Dec 18, 2020

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

hellozee requested a review from rbock January 16, 2021 11:55

hellozee added 4 commits January 16, 2021 18:59

Added a config to export the library as a cmake target

0eb165a

Made use of PROJECT_VERSION instead of hardcoding the number

e46adbd

Ran cmake-format over src/CMakeLists.txt

ce4e921

pick the version from the current tag

7a1e6f8

removed the newly introduced modules

8b9626e

rbock reviewed Jan 18, 2021

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

src/CMakeLists.txt Show resolved Hide resolved

sets tag if the directory is not a git repo or git command is not ava…

c054a8c

…ilable

Leon0402 suggested changes Jan 30, 2021

View reviewed changes

kovdan01 mentioned this pull request Jul 29, 2021

Enhance CMake files #72

Merged

rbock closed this Dec 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a config to export the library as a cmake target #66

Added a config to export the library as a cmake target #66

hellozee commented Dec 14, 2020

hellozee commented Jan 16, 2021

rbock commented Jan 17, 2021

hellozee commented Jan 17, 2021 •

edited

Loading

hellozee commented Jan 26, 2021

rbock commented Jan 30, 2021

Leon0402 Jan 30, 2021

Leon0402 Jan 30, 2021

Leon0402 Jan 30, 2021

Leon0402 Jan 30, 2021

Leon0402 Jan 30, 2021

Leon0402 Jan 30, 2021

Leon0402 commented Jan 30, 2021

rbock commented Dec 18, 2021

		@@ -0,0 +1,6 @@
		list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")

Added a config to export the library as a cmake target #66

Added a config to export the library as a cmake target #66

Conversation

hellozee commented Dec 14, 2020

hellozee commented Jan 16, 2021

rbock commented Jan 17, 2021

hellozee commented Jan 17, 2021 • edited Loading

hellozee commented Jan 26, 2021

rbock commented Jan 30, 2021

Leon0402 Jan 30, 2021

Choose a reason for hiding this comment

Leon0402 Jan 30, 2021

Choose a reason for hiding this comment

Leon0402 Jan 30, 2021

Choose a reason for hiding this comment

Leon0402 Jan 30, 2021

Choose a reason for hiding this comment

Leon0402 Jan 30, 2021

Choose a reason for hiding this comment

Leon0402 Jan 30, 2021

Choose a reason for hiding this comment

Leon0402 commented Jan 30, 2021

rbock commented Dec 18, 2021

hellozee commented Jan 17, 2021 •

edited

Loading