server icon indicating copy to clipboard operation
server copied to clipboard

WIP: MDEV-20544: Update bundled Groonga and Mroonga

Open otegami opened this issue 5 months ago • 12 comments

This is still in work in progress. I opened this PR just for using CI. I'll update the PR description after this is completed.

  • [x] The Jira issue number for this PR is: MDEV-20544

Description

TODO: fill description here

Release Notes

TODO: What should the release notes say about this change? Include any changed system variables, status variables or behaviour. Optionally list any https://mariadb.com/kb/ pages that need changing.

How can this PR be tested?

TODO: modify the automated test suite to verify that the PR causes MariaDB to behave as intended. Consult the documentation on "Writing good test cases".

If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.

Basing the PR against the correct MariaDB version

  • [ ] This is a new feature or a refactoring, and the PR is based against the main branch.
  • [ ] This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • [ ] I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • [ ] For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

otegami avatar Jul 11 '25 02:07 otegami

so list of all CI elements for this PR is https://buildbot.mariadb.org/#/grid?branch=refs%2Fpull%2F4185%2Fhead

note Fedora 40 and Ubuntu 20.04 are getting moved as EOL.

deb-autobake requires debian/* changes for install files.

windows - stricter compiler.

appears to be some dependencies - can you provide a list.

grooverdan avatar Jul 11 '25 07:07 grooverdan

Thanks for the pointers! We’ll tackle these one by one.

appears to be some dependencies - can you provide a list.

In the meantime, here are the packages I believe we’ll need:

Groonga (Ubuntu Build-Depends)

  • libedit-dev
  • libevent-dev
  • liblz4-dev
  • libmecab-dev
  • libmsgpack-dev
  • libsimdjson-dev
  • libssl-dev
  • libstemmer-dev
  • libthrift-dev
  • libxxhash-dev
  • libzmq3-dev
  • libzstd-dev
  • rapidjson-dev
  • zlib1g-dev

ref: https://github.com/groonga/groonga/blob/7b50b0cd0ca6815fb430a63e945e7220693dd3d7/packages/debian/control#L9-L26 ref: https://github.com/groonga/groonga/blob/main/packages/apt/debian-bookworm/Dockerfile#L24-L47

Mroonga (Build-Depends with MariaDB 11.4)

  • groonga-devel
  • groonga-normalizer-mysql-devel
  • MariaDB-client(RPM)
  • MariaDB-common(RPM)
  • MariaDB-server(RPM)
  • MariaDB-shared(RPM)

ref: https://github.com/mroonga/mroonga/blob/main/packages/mariadb-11.4-mroonga/yum/mariadb-11.4-mroonga.spec.in#L32-L37

otegami avatar Jul 11 '25 08:07 otegami

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 14 '25 00:07 CLAassistant

🆙 for submodules! 🫠

ParadoxV5 avatar Jul 14 '25 05:07 ParadoxV5

@grooverdan

About https://github.com/mroonga/mroonga/issues/943#issuecomment-3055161056

Thanks again for the guidance. The Mroonga team has been working hard on this, but unfortunately we’re unlikely to finish everything by July 24 due to two major blockers at the moment. We’ll keep pushing on and will update you as soon as we have progress.

  1. Windows CI (buildbot/amd64-windows)

The Windows builder is failing with platform‑specific errors. We’re also treating all warnings (e.g. C4244) as errors, and there are too many to resolve before the release date. For example, ii.cpp in Groonga alone generated enormous errors, and we estimate around a thousand such locations across the codebase.

ref: https://buildbot.mariadb.org/#/builders/234/builds/40629 ref: https://github.com/abetomo/groonga/actions/runs/16311415283/job/46067698311#step:13:2714

  1. Compile‑only CI (buildbot/amd64-compile-only-nopart-debug)

We’re encountering ISO C90 "mixed declarations and code" errors in dependencies like H3 and llama.cpp's ggml. H3's CMakeLists forces C99 features (see https://github.com/uber/h3/blob/2257ace292aeaa1d21ac06b157ca916c012f5add/CMakeLists.txt#L362), and llama.cpp errors appear as follows.

error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
    const uint32_t bits = fp32_to_bits(base);

While disabling these modules would clear these errors because they aren't really necessary for Mroonga functions, there are likely many more hidden issues in other modules, and we currently do not have enough time to fix them all by July 24.

ref: https://buildbot.mariadb.org/#/builders/904/builds/111

otegami avatar Jul 16 '25 07:07 otegami

  1. Windows CI (buildbot/amd64-windows)

We can disable some warning compile options in the first step. We can work on suppressing warnings later.

  1. Compile‑only CI (buildbot/amd64-compile-only-nopart-debug)

We can disable the declaration-after-statement warning by adding -Wno-declaration-after-statement.

kou avatar Jul 17 '25 00:07 kou

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a23f57b83..fe0ba122d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -370,7 +370,7 @@ if(GRN_C_COMPILER_GNU_LIKE)
   check_build_flag("-Wwrite-strings")
   check_build_flag("-Wsign-compare")
   check_build_flag("-Wmissing-field-initializers")
-  check_cflag("-Wno-declaration-after-statement")
+  check_build_flag("-Wno-declaration-after-statement")
   check_cxxflag("-fexceptions")
   check_cxxflag("-fimplicit-templates")
   check_build_flag("-Wno-implicit-fallthrough")

kou avatar Jul 17 '25 00:07 kou

Or:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a23f57b83..e54b9bc2e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1573,6 +1573,8 @@ if(NOT "${GRN_WITH_H3}" STREQUAL "no")
         # https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4307
         string(APPEND CMAKE_C_FLAGS " /wd4307")
         string(APPEND CMAKE_CXX_FLAGS " /wd4307")
+      elseif(GRN_C_COMPILER_GNU_LIKE)
+        string(APPEND CMAKE_C_FLAGS " -Wno-declaration-after-statement")
       endif()
       set(BUILD_BENCHMARKS OFF)
       set(BUILD_FILTERS OFF)
@@ -1675,6 +1677,8 @@ if(NOT "${GRN_WITH_LLAMA_CPP}" STREQUAL "no")
         # https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996
         string(APPEND CMAKE_C_FLAGS " /wd4996")
         string(APPEND CMAKE_CXX_FLAGS " /wd4996")
+      elseif(GRN_C_COMPILER_GNU_LIKE)
+        string(APPEND CMAKE_C_FLAGS " -Wno-declaration-after-statement")
       endif()
       set(BUILD_SHARED_LIBS ON)
       set(BUILD_STATIC_LIBS OFF)

kou avatar Jul 17 '25 00:07 kou

We’re encountering ISO C90 "mixed declarations and code" errors

Does Mroonga/Groonga still use C90?

ParadoxV5 avatar Jul 17 '25 01:07 ParadoxV5

No.

MariaDB adds -Wdeclaration-after-statement: https://github.com/MariaDB/server/blob/ce7ab467dbdfb5fc83a22bd53d923f7d0d1c6a3e/cmake/maintainer.cmake#L25

It's related.

(I think that MariaDB can remove the warning.)

kou avatar Jul 17 '25 01:07 kou

(I think that MariaDB can remove the warning.)

Maybe MariaDB had it there to enforce a coding style?

ParadoxV5 avatar Jul 17 '25 01:07 ParadoxV5

Oh, I missed it.

It was added on 2023-02-14: https://github.com/MariaDB/server/commit/f74bb51b30df03cf21aca040901089ed27821762

It'll be still valid now.

kou avatar Jul 17 '25 01:07 kou