Copilot commented on code in PR #1884:
URL: https://github.com/apache/cloudberry/pull/1884#discussion_r3718333707
##########
configure.ac:
##########
@@ -932,7 +933,40 @@ PGAC_ARG_BOOL(enable, pax, no,
[enable PAX support],
[AC_DEFINE(USE_PAX_STORAGE, 1,
[Define to 1 to support PAX])])
-AC_MSG_RESULT([checking whether to build with PAX support ... $enable_pax])
+if test "$enable_pax" = yes; then
+ # Check protobuf >= 3.5.0
+ PKG_CHECK_MODULES([PROTOBUF], [protobuf >= 3.5.0],
+ [AC_DEFINE([HAVE_PROTOBUF], [1], [Define if protobuf is available])],
+ [AC_MSG_ERROR([protobuf >= 3.5.0 is required for PAX support])]
+ )
+
+ # Check libzstd >= 1.4.0
+ PKG_CHECK_MODULES([ZSTD], [libzstd >= 1.4.0],
+ [AC_DEFINE([HAVE_ZSTD], [1], [Define if zstd is available])],
+ [AC_MSG_ERROR([libzstd >= 1.4.0 is required for PAX support])]
+ )
+
+ # Check cmake >= 3.11.0 using AX_COMPARE_VERSION
+ AC_PATH_PROG([CMAKE], [cmake], [no])
+ if test "$CMAKE" = "no"; then
+ AC_MSG_ERROR([cmake is required for PAX support but was not found in PATH])
+ fi
+
+ AC_MSG_CHECKING([CMake version])
+ CMAKE_VERSION=`$CMAKE --version | head -n1 | sed
's/[^0-9]*\([0-9][0-9.]*\).*/\1/'`
+ AC_MSG_RESULT([$CMAKE_VERSION])
Review Comment:
The CMake version extraction uses bracket character classes in a sed regex,
but in Autoconf `configure.ac` those `[...]` get eaten by m4 quoting. The
generated `configure` ends up with a broken sed expression, which can make the
PAX cmake>=3.11.0 check mis-parse the version (often to an empty string). Use a
bracket-free pattern (or properly quoted `[[...]]`) so `configure` is generated
correctly.
##########
configure:
##########
@@ -9131,8 +9238,259 @@ else
fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: checking whether to build
with PAX support ... $enable_pax" >&5
-$as_echo "checking whether to build with PAX support ... $enable_pax" >&6; }
+if test "$enable_pax" = yes; then
+ # Check protobuf >= 3.5.0
+
+pkg_failed=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for protobuf >= 3.5.0" >&5
+$as_echo_n "checking for protobuf >= 3.5.0... " >&6; }
+
+if test -n "$PROTOBUF_CFLAGS"; then
+ pkg_cv_PROTOBUF_CFLAGS="$PROTOBUF_CFLAGS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists
--print-errors \"protobuf >= 3.5.0\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "protobuf >= 3.5.0") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_PROTOBUF_CFLAGS=`$PKG_CONFIG --cflags "protobuf >= 3.5.0" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+if test -n "$PROTOBUF_LIBS"; then
+ pkg_cv_PROTOBUF_LIBS="$PROTOBUF_LIBS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists
--print-errors \"protobuf >= 3.5.0\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "protobuf >= 3.5.0") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_PROTOBUF_LIBS=`$PKG_CONFIG --libs "protobuf >= 3.5.0" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+
+
+
+if test $pkg_failed = yes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
+ _pkg_short_errors_supported=yes
+else
+ _pkg_short_errors_supported=no
+fi
+ if test $_pkg_short_errors_supported = yes; then
+ PROTOBUF_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors
--cflags --libs "protobuf >= 3.5.0" 2>&1`
+ else
+ PROTOBUF_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs
"protobuf >= 3.5.0" 2>&1`
+ fi
+ # Put the nasty error message in config.log where it belongs
+ echo "$PROTOBUF_PKG_ERRORS" >&5
+
+ as_fn_error $? "protobuf >= 3.5.0 is required for PAX support"
"$LINENO" 5
+
+elif test $pkg_failed = untried; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+ as_fn_error $? "protobuf >= 3.5.0 is required for PAX support"
"$LINENO" 5
+
+else
+ PROTOBUF_CFLAGS=$pkg_cv_PROTOBUF_CFLAGS
+ PROTOBUF_LIBS=$pkg_cv_PROTOBUF_LIBS
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+$as_echo "#define HAVE_PROTOBUF 1" >>confdefs.h
+
+fi
+
+ # Check libzstd >= 1.4.0
+
+pkg_failed=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libzstd >= 1.4.0" >&5
+$as_echo_n "checking for libzstd >= 1.4.0... " >&6; }
+
+if test -n "$ZSTD_CFLAGS"; then
+ pkg_cv_ZSTD_CFLAGS="$ZSTD_CFLAGS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists
--print-errors \"libzstd >= 1.4.0\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "libzstd >= 1.4.0") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_ZSTD_CFLAGS=`$PKG_CONFIG --cflags "libzstd >= 1.4.0" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+if test -n "$ZSTD_LIBS"; then
+ pkg_cv_ZSTD_LIBS="$ZSTD_LIBS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists
--print-errors \"libzstd >= 1.4.0\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "libzstd >= 1.4.0") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_ZSTD_LIBS=`$PKG_CONFIG --libs "libzstd >= 1.4.0" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+
+
+
+if test $pkg_failed = yes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
+ _pkg_short_errors_supported=yes
+else
+ _pkg_short_errors_supported=no
+fi
+ if test $_pkg_short_errors_supported = yes; then
+ ZSTD_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors
--cflags --libs "libzstd >= 1.4.0" 2>&1`
+ else
+ ZSTD_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs
"libzstd >= 1.4.0" 2>&1`
+ fi
+ # Put the nasty error message in config.log where it belongs
+ echo "$ZSTD_PKG_ERRORS" >&5
+
+ as_fn_error $? "libzstd >= 1.4.0 is required for PAX support" "$LINENO"
5
+
+elif test $pkg_failed = untried; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+ as_fn_error $? "libzstd >= 1.4.0 is required for PAX support" "$LINENO"
5
+
+else
+ ZSTD_CFLAGS=$pkg_cv_ZSTD_CFLAGS
+ ZSTD_LIBS=$pkg_cv_ZSTD_LIBS
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+$as_echo "#define HAVE_ZSTD 1" >>confdefs.h
+
+fi
+
+ # Check cmake >= 3.11.0 using AX_COMPARE_VERSION
+ # Extract the first word of "cmake", so it can be a program name with args.
+set dummy cmake; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_CMAKE+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $CMAKE in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_CMAKE="$CMAKE" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_CMAKE="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext"
>&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ test -z "$ac_cv_path_CMAKE" && ac_cv_path_CMAKE="no"
+ ;;
+esac
+fi
+CMAKE=$ac_cv_path_CMAKE
+if test -n "$CMAKE"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $CMAKE" >&5
+$as_echo "$CMAKE" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+ if test "$CMAKE" = "no"; then
+ as_fn_error $? "cmake is required for PAX support but was not found in
PATH" "$LINENO" 5
+ fi
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking CMake version" >&5
+$as_echo_n "checking CMake version... " >&6; }
+ CMAKE_VERSION=`$CMAKE --version | head -n1 | sed 's/^0-9*\(0-90-9.*\).*/\1/'`
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $CMAKE_VERSION" >&5
+$as_echo "$CMAKE_VERSION" >&6; }
Review Comment:
This line is the generated output of the Autoconf snippet in configure.ac,
but the sed regex is malformed (likely due to m4 quoting of `[...]`). As
written, it will not reliably extract the CMake version and can cause the PAX
cmake>=3.11.0 check to fail incorrectly.
##########
gpAux/client/install/src/windows/CreatePackage.bat:
##########
@@ -11,8 +11,8 @@ type nul > %GPDB_INSTALL_PATH%\bin\gppylib\__init__.py
copy ..\..\..\..\..\gpMgmt\bin\gppylib\gpversion.py
%GPDB_INSTALL_PATH%\bin\gppylib\
perl -pi.bak -e "s,\$Revision\$,%VERSION%," %GPDB_INSTALL_PATH%\bin\gpload.py
copy ..\..\..\..\..\gpMgmt\bin\gpload.bat %GPDB_INSTALL_PATH%\bin
-for %%f in (..\..\..\..\..\gpMgmt\bin\pythonSrc\ext\PyYAML-*.tar.gz) do tar
-xf %%f
-for /D %%d in (PyYAML-*) do copy %%d\lib\yaml\*
%GPDB_INSTALL_PATH%\lib\python\yaml
+REM Install PyYAML using pip instead of extracting from tarball
+pip3 install --target=%GPDB_INSTALL_PATH%\lib\python PyYAML==5.4.1
Review Comment:
`pip3` is not consistently available as a command on Windows build hosts
(often it's `pip` or invoked via `py -m pip`). Using `pip3` here can make
Windows packaging fail even when Python is installed.
##########
README.md:
##########
@@ -1,3 +1,22 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
Review Comment:
The PR description states the fix is contained in `src/backend/cdb/cdbtm.c`,
`src/include/cdb/cdbtm.h`, and the isolation2 schedule, but this PR also
changes a large number of additional files (build scripts, dependency handling,
licenses/NOTICE, docs, GitHub templates, etc.). Please either update the PR
description to reflect the expanded scope or split the unrelated changes into
separate PR(s) to keep review and risk manageable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
