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

[Meson] fixes for meson project #380

Merged
merged 13 commits into from
Dec 9, 2022
Merged

Conversation

UnixY2K
Copy link
Contributor

@UnixY2K UnixY2K commented Dec 8, 2022

PR Description

This PR does the following things.

  • .gitignore: ignore .cache directory created by vscode when using clangd
  • Meson specific files
    • increment the project version to 3.2.0
    • increase gtest version to 1.12.1 and use the version that includes main(only applies when gtest is not installed/detected so it can compile out of the box in meson)
    • add missing comentaries for the meson options

Note for Windows: there is a current bug in meson/ninja when using clang see mesonbuild/meson#10022, the workaround is setting up manually the CXX and CXX_LD env vars manually to be clang++ and lld-link if clang is installed

if there is anything else required please let me know

Future aditions on another PR

  • Add a meson setup section in the readme so any new user can set it up easily if not familiar with meson
  • Add dynamic link support in meson/windows targets (already done on a private fork but requires some changes)
  • Put a public a public wrap file in meson wrapdb so any user can install using meson wrap install sqlitecpp, this is out of scope for this repo, but should be done once the dynamic windows support is ready.

If there is anything else missing please let me know

@dougnazar
Copy link
Contributor

I have a local patch here I've been meaning to send to make a meson install match a cmake install. I guess most meson users are just bundling/wrapping rather than installing. Feel free to include in your future PR if you want since you're working on that area.

diff --git a/meson.build b/meson.build
index fb6f658..e1bdc2e 100644
--- a/meson.build
+++ b/meson.build
@@ -146,7 +146,7 @@ libsqlitecpp = library(
     dependencies: sqlitecpp_deps,
     # override the default options
     override_options: sqlitecpp_opts,
-    # install: true,
+    install: true,
     # API version for SQLiteCpp shared library.
     version: '0',)
 if get_option('SQLITECPP_BUILD_TESTS')
@@ -173,6 +173,9 @@ install_headers(
     'include/SQLiteCpp/Transaction.h',
     'include/SQLiteCpp/VariadicBind.h',
     'include/SQLiteCpp/ExecuteMany.h',
+    'include/SQLiteCpp/Savepoint.h',
+    'include/SQLiteCpp/Utils.h',
+    subdir : 'SQLiteCpp',
 )

 sqlitecpp_dep = declare_dependency(

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 8, 2022

I have a local patch here I've been meaning to send to make a meson install match a cmake install. I guess most meson users are just bundling/wrapping rather than installing. Feel free to include in your future PR if you want since you're working on that area.

diff --git a/meson.build b/meson.build
index fb6f658..e1bdc2e 100644
--- a/meson.build
+++ b/meson.build
@@ -146,7 +146,7 @@ libsqlitecpp = library(
     dependencies: sqlitecpp_deps,
     # override the default options
     override_options: sqlitecpp_opts,
-    # install: true,
+    install: true,
     # API version for SQLiteCpp shared library.
     version: '0',)
 if get_option('SQLITECPP_BUILD_TESTS')
@@ -173,6 +173,9 @@ install_headers(
     'include/SQLiteCpp/Transaction.h',
     'include/SQLiteCpp/VariadicBind.h',
     'include/SQLiteCpp/ExecuteMany.h',
+    'include/SQLiteCpp/Savepoint.h',
+    'include/SQLiteCpp/Utils.h',
+    subdir : 'SQLiteCpp',
 )

 sqlitecpp_dep = declare_dependency(

thanks for the notice!
I added the missing savepoint for the install_headers, it was also missing from headers and sources (and added the subdir option which was missing), I would need to test it later on a non windows machine
as for the install option I plan to add it after I finish the updates to the windows required changes(that will be on other PR), once I have that done I will uncomment the option on that PR, but if I forgot feel free to remember me so I can modify it.

edit: also set the subdir to SQliteCpp
as for the Utils.h i did not see it on the CMakeLists.txt file so I don´t know if it should be installed

@dougnazar
Copy link
Contributor

I added the missing savepoint for the install_headers, it was also missing from headers and sources (and added the subdir option which was missing), I would need to test it later on a non windows machine as for the install option I plan to add it after I finish the updates to the windows required changes(that will be on other PR), once I have that done I will uncomment the option on that PR, but if I forgot feel free to remember me so I can modify it.

I didn't even notice it was missing from the compile/test lists. Mea culpa.

Not a problem, no rush.

edit: also set the subdir to SQliteCpp as for the Utils.h i did not see it on the CMakeLists.txt file so I don´t know if it should be installed

CMake just copies all the headers from the directory during install. I'm fairly sure I did side-by-side installs into dummy paths and than compared to see which files were missing.

install(DIRECTORY include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} COMPONENT headers FILES_MATCHING REGEX ".*\\.(hpp|h)$")

@eli-schwartz
Copy link

You can install the whole directory with Meson too:

install_subdir('include/SQliteCpp', install_dir: get_option('includedir'))

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 8, 2022

I think we can use the second option of installing the subdir, so we don´t need to keep track of the files but let users choose if they want to install them

thanks for the replies!

use install_subdir instead of the install_headers in the meson config file
meson.build Show resolved Hide resolved
@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 9, 2022

added the missing example 2 executable

as dynamic linking is not supported on windows we can give a warning
and compile the library as static itself
@SRombauts SRombauts self-assigned this Dec 9, 2022
@SRombauts
Copy link
Owner

Thanks a lot for the outstanding teamwork, reviews, and patches :)

@SRombauts SRombauts merged commit 4fc2eee into SRombauts:master Dec 9, 2022
@SRombauts
Copy link
Owner

I think it's also time to release a new version, probably just a patch 3.2.1, that will come with all the little cleanup & build system update that already accumulated.
What do you think?

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 9, 2022

sure, since Meson support on Windows is already good, we can also set it as the first version in Meson Wrapdb.
I don´t think that the dynamic link support on windows should be an issue since the static compilation workaround is good enough for most workloads

@UnixY2K UnixY2K deleted the windows-support branch December 11, 2022 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants