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.

Sign up for GitHub

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

Jump to bottom

add sqlitecpp 3.2.1-1 #816

Closed
wants to merge 1 commit into from
Closed

add sqlitecpp 3.2.1-1 #816

wants to merge 1 commit into from

Conversation

UnixY2K
Copy link
Contributor

@UnixY2K UnixY2K commented Dec 10, 2022

adds SQLiteCpp version 3.2.0 to the list of wraps

  • tested environments
    • Windows 11 22H2
      • clang++ 15.0.5 (static compilation only)
      • ninja 1.11.1
      • meson 0.64.1
    • Arch Linux
      • gcc 12.2.0
      • ninja 1.11.0
      • meson 0.64.1

note: if there is any issue regarding windows static build please see: SRombauts/SQLiteCpp#380 or just set CXX and CXX_LD environment variables to clang++ and lld-link

If there is anything else required just let me know

@neheb
Copy link
Collaborator

neheb commented Dec 10, 2022

wrap-git is disallowed here.

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 11, 2022

thanks for the notice,
I will wait until there is a new release of SQLiteCpp so I can update this PR to use wrap-file instead, which should not take much time

@eli-schwartz
Copy link
Member

Please also take a look at the macOS compile failure.

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 11, 2022 •
edited
Loading

Please also take a look at the macOS compile failure.

sure!, I will open an issue on SQLiteCpp in order to check it out

edit: it seems to be related to the support of sqlite3 load extensions disabled in macOS

FAILED: subprojects/sqlitecpp/libsqlitecpp.0.dylib.p/src_Database.cpp.o 
c++ -Isubprojects/sqlitecpp/libsqlitecpp.0.dylib.p -Isubprojects/sqlitecpp -I../subprojects/sqlitecpp -I../subprojects/sqlitecpp/include -fcolor-diagnostics -Wall -Winvalid-pch -Wnon-virtual-dtor -std=c++11 -O0 -g -Wall -Wextra -Wpedantic -Wswitch-enum -Wshadow -Wno-long-long -MD -MQ subprojects/sqlitecpp/libsqlitecpp.0.dylib.p/src_Database.cpp.o -MF subprojects/sqlitecpp/libsqlitecpp.0.dylib.p/src_Database.cpp.o.d -o subprojects/sqlitecpp/libsqlitecpp.0.dylib.p/src_Database.cpp.o -c ../subprojects/sqlitecpp/src/Database.cpp
../subprojects/sqlitecpp/src/Database.cpp:41:33: error: use of undeclared identifier 'SQLITE_OPEN_NOFOLLOW'
const int   OPEN_NOFOLLOW     = SQLITE_OPEN_NOFOLLOW;
                                ^
../subprojects/sqlitecpp/src/Database.cpp:221:11: error: use of undeclared identifier 'sqlite3_load_extension'
    ret = sqlite3_load_extension(getHandle(), apExtensionName, apEntryPointName, 0);
          ^
2 errors generated.

@UnixY2K UnixY2K changed the title add sqlitecpp 3.2.0-1 add sqlitecpp 3.2.1-1 Dec 13, 2022
@UnixY2K UnixY2K mentioned this pull request Dec 13, 2022
Merged
neheb
neheb reviewed Dec 13, 2022
View reviewed changes
subprojects/sqlitecpp.wrap Outdated Show resolved Hide resolved
neheb
neheb reviewed Dec 13, 2022
View reviewed changes
subprojects/packagefiles/sqlitecpp/meson.build Outdated
# for a list of MSVC supported arguments please check:
# https://docs.microsoft.com/en-us/cpp/build/reference/compiler-options-listed-alphabetically?view=msvc-170
if not (host_machine.system() == 'windows' and cxx.get_id() == 'msvc')
sqlitecpp_args += [
Copy link
Collaborator

Choose a reason for hiding this comment

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

could simplify with cc.get_supported_arguments.

Copy link
Member

Choose a reason for hiding this comment

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

The flip side is that hardcoding it based on get_id() is faster because you don't need to run the compiler each time to check which ones are supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, it will be a faster option if the parameters are known in advance, as there are few arguments you can simply use this option, if in the future there were more arguments, perhaps it would be a better option to check if MSVC is used, looking on the bright side we can remove additional verification that is used only for MSVC.

Honestly I don´t know much about the meson implementation and if those values are cached for an specific compiler/setup as it could reduce setup times if they are checked across projects, but I think that is more dependent on implementation than project configuration, maybe a linter should be helpful for this cases(and for new users), as for the moment I think I will set cxx.get_supported_arguments as the meson.build in the project is not a 1-1 copy to the CMakeLists.txt anyways

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@eli-schwartz eli-schwartz Dec 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It would only be cached within a project build so that checking the same option again already knows if it's supported.

(But that should work across subprojects.)

@eli-schwartz
Copy link
Member 

subprojects/SQLiteCpp-3.2.1/meson.build:170:8: ERROR: Unknown function "warming".

That's very much a blocking issue...

@eli-schwartz
Copy link
Member

I thought there was an upstream meson.build file, though?

@eli-schwartz
Copy link
Member

These commits should all be squashed together using git rebase -i upstream/master. Currently they introduce issues in one commit and fix them in another commit.

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 13, 2022

I thought there was an upstream meson.build file, though?

yes, however 3.2.1 was already launched today with a fix for std::filesystem so this time those patches should be made on this release and the next one removed, also it would be good to add a CI on that repo for meson as testing all the environments manually is hard for me

These commits should all be squashed together using git rebase -i upstream/master. Currently they introduce issues in one commit and fix them in another commit.

sure, will make the fixes and then squash them

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 13, 2022

as for the tests on MacOS I did them on a borrowed machine with the following specs:

  • C++ compiler: c++ (clang 14.0.0 "Apple clang version 14.0.0 (clang-1400.0.29.202)")
  • C++ linker: c++ ld64 820.1
  • Ninja: ninja 1.11.1.git.kitware.jobserver-1
  • Meson: meson version 0.64.1
  • Python: Python 3.11.1
    as I don´t have access to a MacOS I do those fixes blindly, but I will try to check if I can get access to another machine.

please let me know if I left another fix missing

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 13, 2022

seems to be failing due to:

sqlitecpp| subprojects/SQLiteCpp-3.2.1/meson.build:164: WARNING: Detected bundled SQLite3 in OSX, but it does not support load extension

is there any issue if I change it to a message()?
as it adds the required flag anyways

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 14, 2022

checked the last error, seems to be an edge case related to SRombauts/SQLiteCpp#394 basically on MacOS 11 in GitHub images SQLITE_OPEN_NOFOLLOW is not defined, will add it as a patch here also as is not a breaking change on the project itself

@UnixY2K
Copy link
Contributor Author

UnixY2K commented Dec 14, 2022 •
edited
Loading

seems that we cannot add those patches(as the CI does not let modify existing project files) on this wrap so maybe this should wait until 3.2.2, removing the need for the patch directory and preserving the fixes on this PR, at the moment will keep it closed and open a new one referencing this one once it is ready, as for the MacOS fixes I opened an issue on the repo in order to fix it(good to know that under the CI the compilation step passed)

@UnixY2K UnixY2K closed this Dec 14, 2022
@UnixY2K UnixY2K mentioned this pull request Aug 28, 2023
Merged
@UnixY2K UnixY2K deleted the sqlitecpp-3.2.0-1 branch January 28, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eli-schwartz eli-schwartz eli-schwartz left review comments

@neheb neheb neheb left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@UnixY2K @neheb @eli-schwartz