-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
MySQL 8.3 is released #1012
MySQL 8.3 is released #1012
Conversation
WalkthroughThe project's continuous integration configuration has been updated to support newer software versions and environments. MySQL now includes a test for version 8.3, the build scripts for macOS have been adjusted to use the latest clang compiler, and the Windows build configurations have been shifted from Visual Studio 2019 to Visual Studio 2022. Additionally, a patch has been applied to the MySQL build process to avoid installing large .pdb files on Windows. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
versions/mysql.json
is excluded by:!**/*.json
Files selected for processing (1)
- .github/workflows/test.yml (1 hunks)
Additional comments: 1
.github/workflows/test.yml (1)
- 45-45: The addition of MySQL version "8.3" to the testing matrix is consistent with the PR objectives and the AI-generated summary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .github/build-mysql-darwin.sh (1 hunks)
Additional comments: 2
.github/build-mysql-darwin.sh (2)
- 27-31: The script now sets
CC
andCXX
to use the latest version of clang installed (llvm@15
). Verify that this version of clang is compatible with all other dependencies and the MySQL version being built. Additionally, ensure thatllvm@15
is available on the runner where this script will be executed.- 24-35: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-102]
The remainder of the script remains unchanged and is responsible for building MySQL with the appropriate OpenSSL version and applying necessary patches. The changes to the compiler settings are isolated and do not appear to interfere with the rest of the script's operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/workflows/build-mariadb.yml (1 hunks)
- .github/workflows/build-mysql.yml (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/build-mariadb.yml
Additional comments: 1
.github/workflows/build-mysql.yml (1)
- 87-87: The update of the
runs-on
attribute fromwindows-2019
towindows-2022
is in line with the PR objectives to ensure compatibility with newer environments. Confirm that all dependencies and scripts used in thebuild-windows
job are compatible withwindows-2022
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/build-mariadb-windows.ps1 (1 hunks)
- .github/build-mysql-windows.ps1 (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/build-mysql-windows.ps1
Additional comments: 3
.github/build-mariadb-windows.ps1 (3)
- 17-17: The script now sets up Visual Studio 2022 instead of Visual Studio 2019. This change aligns with the PR objectives to update the build environment.
- 24-24: The script uses a regex pattern to determine the OpenSSL version to use based on the MariaDB version. Ensure that this logic is robust and correctly handles all expected MariaDB version formats.
- 24-24: Ensure that the MariaDB source fetching, Bison setup, and build artifact handling are consistent with the updated build environment and that the paths and commands used are compatible with Visual Studio 2022.
Verification successful
The MariaDB source fetching, Bison setup, and build artifact handling in the
.github/build-mariadb-windows.ps1
script use tools and commands that are independent of the Visual Studio version. Therefore, they should be compatible with Visual Studio 2022.
- MariaDB source fetching: Uses
Invoke-WebRequest
to download andtar
to extract the source, which are not dependent on Visual Studio.- Bison setup: Installs
winflexbison3
via Chocolatey, which is also independent of Visual Studio.- Build artifact handling: Uses
Expand-Archive
to extract the build artifacts, a PowerShell cmdlet not tied to any specific version of Visual Studio.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the paths and commands used in the MariaDB source fetching, Bison setup, and build artifact handling are compatible with Visual Studio 2022. # This script is a placeholder and should be replaced with specific verification steps as needed. echo "Verify MariaDB source fetching compatibility." echo "Verify Bison setup compatibility." echo "Verify build artifact handling compatibility." # Expected output: confirmation that all paths and commands are compatible with Visual Studio 2022.Length of output: 278
Script:
#!/bin/bash # Let's find the build-mariadb-windows.ps1 script and examine the sections related to MariaDB source fetching, Bison setup, and build artifact handling. # We will use `rg` to search for relevant keywords within the script. # Search for MariaDB source fetching related commands echo "Searching for MariaDB source fetching related commands..." rg "git clone" --context 5 .github/build-mariadb-windows.ps1 # Search for Bison setup related commands echo "Searching for Bison setup related commands..." rg "bison" --context 5 .github/build-mariadb-windows.ps1 # Search for build artifact handling related commands echo "Searching for build artifact handling related commands..." rg "archive" --context 5 .github/build-mariadb-windows.ps1Length of output: 2561
.github/build-mariadb-windows.ps1
Outdated
New-Item $RUNNER_TEMP -ItemType Directory -Force | ||
Set-Location "$RUNNER_TEMP" | ||
Remove-Item -Path * -Recurse -Force | ||
|
||
# https://help.appveyor.com/discussions/questions/18777-how-to-use-vcvars64bat-from-powershell | ||
# https://stackoverflow.com/questions/2124753/how-can-i-use-powershell-with-the-visual-studio-command-prompt | ||
cmd.exe /c "call `"C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvarsall.bat`" x64 && set > %temp%\vcvars.txt" | ||
cmd.exe /c "call `"C:\Program Files (x86)\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat`" x64 && set > %temp%\vcvars.txt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path to the Visual Studio 2022 command prompt script is hardcoded. Consider using an environment variable or a parameter to specify the Visual Studio installation path to make the script more flexible and maintainable.
Changing the RUNNER_TEMP
directory to C:\Temp
may lead to conflicts if multiple instances of the script run concurrently. Consider creating a unique subdirectory within C:\Temp
for each build to avoid such conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .github/build-mariadb-windows.ps1 (1 hunks)
- .github/build-mysql-windows.ps1 (1 hunks)
- .github/workflows/build-mysql.yml (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- .github/build-mariadb-windows.ps1
- .github/build-mysql-windows.ps1
- .github/workflows/build-mysql.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- patches/mysql/8.3.0/skip-install-pdb.patch (1 hunks)
Additional comments: 2
patches/mysql/8.3.0/skip-install-pdb.patch (2)
- 41-63: The second part of the patch also removes the installation of .pdb files for .exe and .dll files. This change is consistent with the previous one, but it's important to verify that this does not affect release builds where .pdb files might be necessary for debugging.
Verification successful
The search across the codebase revealed multiple patches for different MySQL versions that consistently remove the installation of .pdb files, indicating that these files are not required for release builds within the GitHub Actions environment. This supports the changes made in the PR for MySQL 8.3.0 and addresses the concern raised in the review comment.
* 70-79: The third part of the patch removes the installation of .pdb files for the Router component. This change should be verified with the maintainers to ensure that Router debugging is not impacted negatively.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if release builds require .pdb files rg --type cmake 'install.*\.pdb' --files-with-matchesLength of output: 204
Script:
#!/bin/bash # Broaden the search for .pdb file installation references rg 'install.*\.pdb'Length of output: 3688
MACRO(INSTALL_DEBUG_SYMBOLS target) | ||
- IF(MSVC) | ||
- GET_TARGET_PROPERTY(type ${target} TYPE) | ||
- IF(NOT INSTALL_LOCATION) | ||
- IF(type MATCHES "STATIC_LIBRARY" | ||
- OR type MATCHES "MODULE_LIBRARY" | ||
- OR type MATCHES "SHARED_LIBRARY") | ||
- SET(INSTALL_LOCATION "lib") | ||
- ELSEIF(type MATCHES "EXECUTABLE") | ||
- SET(INSTALL_LOCATION "bin") | ||
- ELSE() | ||
- MESSAGE(FATAL_ERROR | ||
- "cannot determine type of ${target}. Don't now where to install") | ||
- ENDIF() | ||
- ENDIF() | ||
- | ||
- IF(target STREQUAL "mysqld" OR target STREQUAL "mysqlbackup") | ||
- SET(comp Server) | ||
- ELSE() | ||
- SET(comp Debuginfo) | ||
- ENDIF() | ||
- | ||
- # No .pdb file for static libraries. | ||
- IF(NOT type MATCHES "STATIC_LIBRARY") | ||
- INSTALL(FILES $<TARGET_PDB_FILE:${target}> | ||
- DESTINATION ${INSTALL_LOCATION} COMPONENT ${comp}) | ||
- ENDIF() | ||
- ENDIF() | ||
+# .pdb files are too large to use in GitHub Actions. | ||
+# so skip installing | ||
ENDMACRO() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The patch removes the installation of .pdb files for Windows builds. This is a significant change that assumes .pdb files are not required for all future builds and testing scenarios.
Ensure that this change is clearly documented in the project's README or developer documentation, as it affects debugging capabilities for Windows builds.
Summary by CodeRabbit
New Features
Refactor
Chores