V3.0.2 merge upgrade deps add new distros #5033
Conversation
This reverts commit b66df68.
Signed-off-by: Wazir Ahmed <[email protected]>
Signed-off-by: Wazir Ahmed <[email protected]>
Signed-off-by: Wazir Ahmed <[email protected]>
Signed-off-by: Wazir Ahmed <[email protected]>
…-merge-upgrade_deps-add_new_distros
mirostauder
added
enhancement
Packages
dependencies
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @mirostauder, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request integrates two major updates for ProxySQL 3.0.2: a comprehensive upgrade of existing third-party dependencies and the addition of new ones, alongside expanding build and packaging support to include several newer Linux distributions. These changes aim to keep the project current with its underlying libraries and broaden its compatibility across modern operating environments.
Highlights
- Dependency Upgrades: Numerous third-party libraries, including
curl,postgresql,prometheus-cpp,libconfig,libmicrohttpd,clickhouse-cpp,sqlite3,jemalloc, andmariadb-connector-c, have been updated to their latest versions to ensure currency and leverage new features or fixes. - New Dependency Integration: The
nlohmann/jsonandabseil-cpplibraries have been added as new project dependencies, requiring significant updates to the build system and source code to correctly include and link them. - Expanded Distribution Support: The build and packaging infrastructure has been extended to officially support newer Linux distributions, specifically AlmaLinux 10, CentOS 10, Debian 13, and Fedora 42, including their clang and debug build variants.
- Build System Refinements: The project's Makefiles have undergone extensive modifications to accommodate the new dependencies' build processes (e.g.,
cmakeforlibconfig), update include and library paths, and apply necessary compatibility patches across various libraries and operating systems.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request updates dependencies and adds support for new distributions. The changes are extensive, especially in the Makefiles. The integration of new dependencies like abseil and nlohmann/json is well-executed across the codebase. I have identified some areas for improvement in the Makefiles and code, including extraction directory, installation prefix, unnecessary template instantiations, and code formatting.
| libinjection/libinjection/src/libinjection.a: | ||
| cd libinjection && rm -rf libinjection-*/ || true | ||
| cd libinjection && tar -zxf libinjection-3.10.0.tar.gz | ||
| cd libinjection && tar -zxf libinjection-*.tar.gz |
There was a problem hiding this comment.
The tar command extracts the archive without specifying a target directory. This could lead to files being extracted in the current directory, potentially mixing with existing files. It's safer to extract to a subdirectory to keep things organized. Consider adding -C libinjection to the tar command to extract the contents into the libinjection directory.
| cd curl/curl && CPPFLAGS="-I$(SSL_IDIR)" LDFLAGS="$(LIB_SSL_PATH) $(LIB_CRYPTO_PATH)" ./configure --prefix=$(abspath ./curl/curl/local) --disable-debug --disable-ftp --disable-ldap --disable-ldaps --disable-rtsp --disable-proxy --disable-dict --disable-telnet --disable-tftp --disable-pop3 --disable-imap --disable-smb --disable-smtp --disable-gopher --disable-manual --disable-ipv6 --disable-sspi --disable-ntlm-wb --disable-tls-srp --without-nghttp2 --without-libidn2 --without-libssh2 --without-brotli --without-librtmp --without-libpsl --without-zstd --with-ssl --enable-shared=yes | ||
| cd curl/curl && CFLAGS=-fPIC CC=${CC} CXX=${CXX} ${MAKE} | ||
| cd curl/curl && ${MAKE} install |
There was a problem hiding this comment.
The configure script is executed without specifying an installation prefix. This can lead to files being installed in system directories, potentially causing conflicts with other software. It's recommended to use --prefix=$(abspath ./curl/curl/local) to install the library in a local directory.
| cd clickhouse-cpp/clickhouse-cpp && cmake . \ | ||
| -DCMAKE_BUILD_TYPE=RelWithDebInfo \ | ||
| -DCMAKE_PREFIX_PATH=$(abspath abseil/abseil-cpp/local) |
There was a problem hiding this comment.
The cmake command is executed without specifying CMAKE_INSTALL_PREFIX. This can lead to files being installed in system directories, potentially causing conflicts with other software. It's recommended to use -DCMAKE_INSTALL_PREFIX=$(abspath abseil/abseil-cpp/local) to install the library in a local directory.
| @@ -58,7 +58,7 @@ enum PgSQL_Param_Name { | |||
| PG_SSLROOTCERT, // Specifies the name of a file containing SSL certificate authority (CA) certificate(s) | |||
| PG_SSLCRL, // Specifies the file name of the SSL server certificate revocation list (CRL) | |||
| PG_SSLCRLDIR, // Specifies the directory name of the SSL server certificate revocation list (CRL) | |||
| PG_SSLSNI, // Sets the TLS extension �Server Name Indication� (SNI) on SSL-enabled connections | |||
| PG_SSLSNI, // Sets the TLS extension �Server Name Indication� (SNI) on SSL-enabled connections | |||
There was a problem hiding this comment.
Typo: 'Server Name Indication' should be 'Server Name Indication'.
| CURL_DIR := $(DEPS_PATH)/curl/curl | ||
| IDIRS += -L$(CURL_DIR)/include | ||
| LDIRS += -L$(CURL_DIR)/lib/.libs | ||
| IDIRS += -L$(CURL_IDIR) |
There was a problem hiding this comment.
It's better to use ${AR} -crs instead of ar rcs to create archive files. The c flag creates the archive if it doesn't exist, the r flag replaces existing files, and the s flag creates an index. Using ${AR} ensures that the correct archiver is used, especially in cross-compilation environments.
|
merge of
V3.0.2 add new distros #5017
and
V3.0.2 upgrade deps #4991