Conversation
…on build failed 3.choice ossp as uuid default value
WalkthroughAdds a new contrib extension gb18030_2022 with Meson build, install, and tests; updates contrib/meson.build to include it. Extends ivorysql_ora sources and installs a preload SQL. Introduces IvorySQL version variables and updates PG_VERSION_STR. Changes default uuid option to ossp. Adds build-order deps for generated catalog headers and updates CI Meson version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant M as Meson
participant Ext as gb18030_2022
participant Gen as perl/gensql.pl
participant CC as Compiler
participant OS as Installer
participant T as Test runner
Note over M,Ext: Configure
M->>Ext: define shared_module gb18030_2022 (utf8_and_gb18030_2022.c)
alt Windows
M->>OS: invoke rc_lib_gen to make version resource
end
Note over M,Gen: Build-time codegen
M->>Gen: run gensql.pl -> utf8_and_gb18030_2022.bc
Gen-->>M: produce .bc
M->>CC: compile shared module
CC-->>M: gb18030_2022(.so/.dll)
Note over M,OS: Install
M->>OS: install control/sql files and compiled artifacts
Note over M,T: Test registration
M->>T: register tests (gb18030_2022_and_utf8, copy)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contrib/gb18030_2022/meson.build (1)
21-26: Remove unnecessary run_target loop.The foreach loop creating touch targets for both 'utf8_and_gb18030_2022.bc' and 'gb18030_2022.bc' files is unnecessary. The custom_target above already handles the creation of 'utf8_and_gb18030_2022.bc', and the second file name doesn't seem to be used elsewhere.
Consider removing this loop entirely or clarify why both filenames are needed:
-foreach file : ['utf8_and_gb18030_2022.bc', 'gb18030_2022.bc'] - run_target( - file, - command: ['touch', meson.current_build_dir() / file] - ) -endforeach
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
contrib/gb18030_2022/meson.build(1 hunks)contrib/ivorysql_ora/meson.build(2 hunks)contrib/meson.build(1 hunks)meson.build(4 hunks)meson_options.txt(1 hunks)src/include/catalog/meson.build(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: meson_build
contrib/ivorysql_ora/meson.build
[error] 1584-1584: meson.build:1584:4: ERROR: Expected 1 arguments, got 2. Command: meson setup build -Dcassert=true -Dbuildtype=debug
contrib/meson.build
[error] 1584-1584: meson.build:1584:4: ERROR: Expected 1 arguments, got 2. Command: meson setup build -Dcassert=true -Dbuildtype=debug
src/include/catalog/meson.build
[error] 1584-1584: meson.build:1584:4: ERROR: Expected 1 arguments, got 2. Command: meson setup build -Dcassert=true -Dbuildtype=debug
contrib/gb18030_2022/meson.build
[error] 1584-1584: meson.build:1584:4: ERROR: Expected 1 arguments, got 2. Command: meson setup build -Dcassert=true -Dbuildtype=debug
meson.build
[error] 1584-1584: meson.build:1584:4: ERROR: Expected 1 arguments, got 2. Command: meson setup build -Dcassert=true -Dbuildtype=debug
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: oracle_pg_regression (ubuntu-latest)
- GitHub Check: contrib_regression
- GitHub Check: pg_regression (ubuntu-latest)
- GitHub Check: oracle_regression (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (10)
meson_options.txt (1)
162-164: LGTM! Default change aligns with PR objectives.The change from 'none' to 'ossp' as the default UUID implementation aligns with the PR objective to set OSSP as the default choice for uuid. This is a reasonable default as OSSP-UUID is widely available and provides good functionality.
src/include/catalog/meson.build (1)
170-170: Excellent fix for build ordering dependencies.The addition of explicit build dependencies (
depends: [generated_catalog_headers]anddepends: [ara_generated_catalog_headers]) properly establishes the build order sequence:
generated_catalog_headers→ara_generated_catalog_headers→ora_generated_catalog_headersThis addresses the build ordering issues mentioned in the PR objectives and ensures that Oracle-related headers are generated in the correct sequence.
Also applies to: 192-192
contrib/meson.build (1)
77-77: LGTM! Correct subdirectory inclusion.The addition of
subdir('gb18030_2022')in the appropriate alphabetical position integrates the new gb18030_2022 module into the contrib build system correctly.meson.build (4)
30-31: LGTM! Clean IvorySQL version setup.The addition of the
ivorysql_versionvariable provides a clean way to manage IvorySQL-specific versioning alongside PostgreSQL versioning.
158-158: LGTM! Proper configuration data setup.The
PACKAGE_IVORYSQL_VERSIONconfiguration properly exports the IvorySQL version for use throughout the build system.
494-494: LGTM! Correct directory prefix recognition.Adding 'IvorySQL' to
dir_prefix_contains_pgensures that IvorySQL installations are recognized as PostgreSQL-compatible for directory layout purposes.
2979-2983: Fix format string argument mismatch causing build failure.The
PG_VERSION_STRformat string has 7 placeholders (@0@ through @6@) but only 6 arguments are provided, which is causing the Meson build error "Expected 1 arguments, got 2".Apply this diff to fix the argument count:
cdata.set_quoted('PG_VERSION_STR', - 'PostgreSQL (IvorySQL @6@) @0@ on @1@-@2@, compiled by @3@-@4@, @5@-bit'.format( + 'PostgreSQL (IvorySQL @0@) @1@ on @2@-@3@, compiled by @4@-@5@, @6@-bit'.format( + cdata.get('PACKAGE_IVORYSQL_VERSION'), pg_version, host_machine.cpu_family(), host_system, - cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8, cdata.get('PACKAGE_IVORYSQL_VERSION') + cc.get_id(), cc.version(), cdata.get('SIZEOF_VOID_P') * 8 ) )Likely an incorrect or invalid review comment.
contrib/ivorysql_ora/meson.build (2)
28-28: Verified — source file exists.
contrib/ivorysql_ora/src/xml_functions/ora_xml_functions.c present (53,749 bytes, mtime Sep 18 08:20).
56-59: preload_ora_misc.sql verified — no action required.
contrib/ivorysql_ora/preload_ora_misc.sql exists and contains the expected SQL statements.contrib/gb18030_2022/meson.build (1)
13-19: Confirm correct gensql.pl pathcontrib/gb18030_2022/gensql.pl is missing; only contrib/ivorysql_ora/gensql.pl was found. The proposed change to ../contrib/gb18030_2022/gensql.pl would point to a non‑existent file. Either add contrib/gb18030_2022/gensql.pl, or keep/update meson.build to reference the intended existing script and document why it's shared.
Location: contrib/gb18030_2022/meson.build (lines 13–19)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/meson_build.yml (3)
21-26: Avoid installing both uuid-dev and libossp-uuid-dev.These provide different UUID libraries; installing both can lead to link ambiguity and undercut the PR goal of defaulting to OSSP. Prefer only libossp-uuid-dev if OSSP is the default in Meson options.
If OSSP is the default, remove
uuid-devhere to prevent accidental linkage against util-linuxlibuuid.
17-17: Bump checkout action to v4.actions/checkout v4 is the current major; use it for security and runner parity.
Apply:
- - uses: actions/checkout@v3 + - uses: actions/checkout@v4
18-18: Fix step label typo.“dependancy” -> “dependency”.
Apply:
- - name: dependancy - linux + - name: dependency - linux
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/meson_build.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: pg_regression (ubuntu-latest)
- GitHub Check: oracle_regression (ubuntu-latest)
- GitHub Check: meson_build (ubuntu-latest)
- GitHub Check: contrib_regression
- GitHub Check: oracle_pg_regression (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (1)
.github/workflows/meson_build.yml (1)
44-44: Re-evaluate forcing-j1.If Meson now serializes genbki.pl correctly (as per this PR), restore parallel builds for CI speed. Otherwise keep
-j1.You can try a CI run with default Ninja parallelism to confirm the genbki.pl fix holds under load.
| curl -L "https://github.com/mesonbuild/meson/releases/download/1.0.1/meson-1.0.1.tar.gz" -o meson-1.0.1.tar.gz | ||
| curl -L "https://github.com/ninja-build/ninja/releases/download/v1.10.1/ninja-linux.zip" -o ninja-linux.zip | ||
| unzip -o ninja-linux.zip | ||
| sudo cp ninja /usr/bin/ | ||
| tar xzf meson-0.57.2.tar.gz | ||
| cd meson-0.57.2 | ||
| tar xzf meson-1.0.1.tar.gz | ||
| cd meson-1.0.1 | ||
| sudo python3 setup.py install |
There was a problem hiding this comment.
Replace tarball + setup.py install with pip; install Ninja via apt (simpler, safer).
Manual curl + setup.py install is brittle and deprecated in Python packaging. Use pip for Meson and apt for Ninja to reduce supply‑chain risk and simplify PATH issues.
Apply this diff:
- curl -L "https://github.com/mesonbuild/meson/releases/download/1.0.1/meson-1.0.1.tar.gz" -o meson-1.0.1.tar.gz
- curl -L "https://github.com/ninja-build/ninja/releases/download/v1.10.1/ninja-linux.zip" -o ninja-linux.zip
- unzip -o ninja-linux.zip
- sudo cp ninja /usr/bin/
- tar xzf meson-1.0.1.tar.gz
- cd meson-1.0.1
- sudo python3 setup.py install
+ sudo apt-get install -y ninja-build python3-pip
+ python3 -m pip install --upgrade pip
+ python3 -m pip install "meson==1.0.1"
+ meson --version
+ ninja --version📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl -L "https://github.com/mesonbuild/meson/releases/download/1.0.1/meson-1.0.1.tar.gz" -o meson-1.0.1.tar.gz | |
| curl -L "https://github.com/ninja-build/ninja/releases/download/v1.10.1/ninja-linux.zip" -o ninja-linux.zip | |
| unzip -o ninja-linux.zip | |
| sudo cp ninja /usr/bin/ | |
| tar xzf meson-0.57.2.tar.gz | |
| cd meson-0.57.2 | |
| tar xzf meson-1.0.1.tar.gz | |
| cd meson-1.0.1 | |
| sudo python3 setup.py install | |
| sudo apt-get install -y ninja-build python3-pip | |
| python3 -m pip install --upgrade pip | |
| python3 -m pip install "meson==1.0.1" | |
| meson --version | |
| ninja --version |
🤖 Prompt for AI Agents
.github/workflows/meson_build.yml around lines 27 to 33: the workflow currently
downloads Meson tarball and runs sudo python3 setup.py install and fetches Ninja
zip/unzips and copies binary; replace these brittle steps by using pip to
install Meson and the distro package for Ninja: remove the curl/tar/unzip/cp and
replace with a step that runs sudo python3 -m pip install "meson==1.0.1" (or
python3 -m pip install --upgrade pip then pip) and install Ninja via the package
manager (e.g., sudo apt-get update && sudo apt-get install -y ninja-build) so
installation uses supported packaging and avoids manual binary handling.
🧩 Analysis chain
Confirm Meson 1.0.1 is within your supported range.
If you need a broader window for future Meson releases, consider a semver range via pip (e.g., meson>=1.0,<1.5) after confirming repo constraints.
Run this to check declared Meson requirements and UUID default in the repo:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Meson minimum in project():"
rg -nP --glob '!**/subprojects/**' "project\([^\)]*meson_version\s*:\s*'[^']+'\)" -n -C0
echo
echo "UUID option default:"
rg -nP "option\(\s*'uuid'.*default\s*:\s*'[^']+'\s*\)" -n meson_options.txt || trueLength of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "cwd: $(pwd)"
echo "Top-level files:"
ls -la | sed -n '1,200p'
echo
echo "Find meson.build files:"
find . -type f -iname 'meson.build' -print || true
echo
echo "Find meson options files (meson_options.txt / meson.options):"
find . -type f \( -iname 'meson_options.txt' -o -iname 'meson.options' \) -print || true
echo
echo "Searching meson.build files for project(... meson_version : '...') and showing first 200 lines:"
while IFS= read -r f; do
echo "---- $f ----"
rg -nP "project\([^)]*meson_version\s*:\s*'[^']+'\)" -n -C0 "$f" || true
sed -n '1,200p' "$f"
done < <(find . -type f -iname 'meson.build' -print)
echo
echo "Searching meson options files for option('uuid' ... default : '...') and showing first 200 lines:"
while IFS= read -r f; do
echo "---- $f ----"
rg -nP "option\(\s*'uuid'.*default\s*:\s*'[^']+'\s*\)" -n "$f" || true
sed -n '1,200p' "$f"
done < <(find . -type f \( -iname 'meson_options.txt' -o -iname 'meson.options' \) -print)Length of output: 16210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "cwd: $(pwd)"
echo
echo "rg version:"
rg --version || true
echo
echo "Searching for 'meson_version' occurrences:"
rg -n --hidden --no-ignore "meson_version" || true
echo
echo "Searching for 'project(' with 'meson_version' (multiline):"
rg -n --hidden --no-ignore -U "project\([^)]*meson_version" || true
echo
echo "Show top-level meson.build (first 200 lines):"
if [ -f meson.build ]; then sed -n '1,200p' meson.build; else echo "meson.build not found"; fi
echo
echo "Show meson_options.txt (first 200 lines):"
if [ -f meson_options.txt ]; then sed -n '1,200p' meson_options.txt; else echo "meson_options.txt not found"; fiLength of output: 13153
Pin CI Meson to the project's supported range (<0.56); don't install Meson 1.0.1.
Top-level meson.build declares meson_version: '>=0.54' and contains a comment explicitly preferring <0.56; meson_options.txt sets the 'uuid' default to 'ossp'. Update .github/workflows/meson_build.yml to install a Meson release within that window (for example pip install "meson>=0.54,<0.56" or pin to a tested 0.55.x) or intentionally bump the project's meson_version/comments if you want to move to 1.x. Ensure CI provides the OSSP UUID library when using the 'ossp' default.
🤖 Prompt for AI Agents
.github/workflows/meson_build.yml lines 27-33: the workflow currently downloads
and installs Meson 1.0.1 which is outside the project's declared supported range
(meson_version '>=0.54' with a comment preferring <0.56); remove the hardcoded
Meson 1.0.1 install and instead install a Meson release in the supported window
(for example use pip install "meson>=0.54,<0.56" or pin to a tested 0.55.x), and
ensure the CI also installs/provides the OSSP uuid library (libossp-uuid or
equivalent package) since meson_options.txt defaults to 'ossp' UUID.
Summary by CodeRabbit
New Features
Changes
Chores