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

provide sqlite2cpp as well #1786

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vpoinot
Copy link

@vpoinot vpoinot commented Nov 15, 2024

The script sqlite2cpp works better than ddl2cpp when it comes to sqlite3 schemas. Provide it as well.

@jpakkane
Copy link
Member

You need to add the package to releases.json.

@vpoinot
Copy link
Author

vpoinot commented Nov 19, 2024

@jpakkane
Copy link
Member

It is not. You need to add a new entry in the version list as well. "0.64-2"

@vpoinot
Copy link
Author

vpoinot commented Nov 21, 2024

It is not. You need to add a new entry in the version list as well. "0.64-2"

Done: https://github.com/mesonbuild/wrapdb/pull/1786/files#diff-f71e67b61c52ecd942c6d915f8ff111cb57052fe82992eda958b87a9c62ed633R3473

Let me know if something else is needed.

@vpoinot
Copy link
Author

vpoinot commented Nov 22, 2024

The sanity checks fail on macos, because of some missing dependency (mariadb). Is this something expected?

@jpakkane
Copy link
Member

Maybe brew has changed their packaging somehow? I don't use macOS, so not really sure how to debug that. If nothing else then the flag should be removed from ci_config.json.

@eli-schwartz
Copy link
Member

wrapdb/ci_config.json

Lines 1076 to 1085 in 4972deb

"sqlitecpp": {
"_comment": "required to not use the default sqlite3 in MacOS to avoid compilation errors/tests failures, also add pkgconfig path to find sqlite3(keg-only)",
"build_options": [
"sqlitecpp:SQLITECPP_BUILD_TESTS=true",
"sqlitecpp:SQLITECPP_BUILD_EXAMPLES=true",
"pkg_config_path=/usr/local/opt/sqlite/lib/pkgconfig"
],
"brew_packages": [
"sqlite"
]

If mariadb is mandatory for the wrapdb CI e.g. in order to run all tests, then please install it in the ci config. It's possible this wasn't necessary in the past because GitHub chose to install it by default -- they sometimes remove packages that most people don't use in order to decrease VM sizes for their hosted runners. IIRC they have done one of those cleanups recently.

Copy link
Member

@eli-schwartz eli-schwartz left a comment

Choose a reason for hiding this comment

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

Please use git commit --fixup=sha1 && git rebase -i --autosquash when fixing issues introduced in a previous commit in the same PR.

@vpoinot
Copy link
Author

vpoinot commented Dec 7, 2024

Please use git commit --fixup=sha1 && git rebase -i --autosquash when fixing issues introduced in a previous commit in the same PR.

I have run git commit --fixup=63562a5ee0e9cbc86e99e2c76d24caf0b1f98e80 && git rebase -i --autosquash, but I am not sure that it did anything...

@jpakkane
Copy link
Member

jpakkane commented Dec 7, 2024

Rebasing against current master should make the error go away.

@vpoinot
Copy link
Author

vpoinot commented Dec 8, 2024

Rebasing against current master should make the error go away.

Nope, my branch is already up-to-date.

@jpakkane
Copy link
Member

jpakkane commented Dec 8, 2024

If I click on the branch link that page says "This branch is 5 commits ahead of, 27 commits behind mesonbuild/wrapdb:master."

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

Successfully merging this pull request may close these issues.

3 participants