-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
fix broken MySQL 8.1.0 build on macos-14 #1133
Conversation
WalkthroughThis patch updates the MySQL codebase to ensure compatibility with C++20 while retaining support for C++17. It addresses compiler warnings and errors due to stricter C++20 rules, modifies member functions to be Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CMake
participant Compiler
participant Codebase
Developer->>CMake: Configure with C++20
CMake->>Compiler: Check compiler version
Compiler-->>CMake: Version supported
CMake->>Codebase: Prepare for C++20
Codebase->>Compiler: Compile with C++20 features
Compiler-->>Codebase: Compilation successful
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- patches/mysql/8.1.0/prepare-the-codebase-for-c++20.patch (1 hunks)
Additional comments not posted (7)
patches/mysql/8.1.0/prepare-the-codebase-for-c++20.patch (7)
216-231
: Ensure const Correctness in Operator OverloadsMaking the equality operator
const
in theData
class is a crucial update for C++20 compatibility, ensuring that objects can be compared without modifying them. This change is correctly implemented.
296-327
: Refactor Volatile to std::atomic for Thread SafetyReplacing
volatile
withstd::atomic
in the logging component is an excellent improvement for thread safety and correctness. This change aligns with modern C++ practices for managing concurrency.
338-369
: Simplify PFS_engine_table_proxy StructureThe simplification of the
PFS_engine_table_proxy
structure by initializing members tonullptr
is a good practice. It makes the code cleaner and more maintainable.
380-384
: Add const to Operator OverloadsAdding
const
to operator overloads in theCursor
class is a minor but necessary change for const correctness.
393-397
: Refactor Increment FunctionRefactoring the increment function to use a more robust method rather than directly manipulating the counter is a good practice, especially in a multi-threaded environment.
562-572
: Remove Unused MacroRemoving the unused
in_range
macro is a good cleanup. Keeping the codebase free of unused code reduces maintenance overhead and potential confusion.
602-662
: Refactor Execution Context MethodsMoving the implementation of methods like
notify_fork
andshutdown
out of the header file into a more appropriate location helps in reducing compilation dependencies and potentially hiding implementation details.
This reverts commit 6147cfa.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- patches/mysql/8.1.0/equality-operator-needs-to-be-updated.patch (1 hunks)
Additional comments not posted (2)
patches/mysql/8.1.0/equality-operator-needs-to-be-updated.patch (2)
26-26
: Approved change in operator definition.The modification from
const Role_id
toRole_id
in the pair type definition is likely necessary to address the type mismatch issues highlighted in the PR. This change allows for modifications to the second element of the pair, which could be crucial for the equality comparisons.Please verify that this change does not introduce side effects in other parts of the codebase where this operator is used.
39-39
: Consistent change across files.The modification from
const Role_id
toRole_id
in the pair type definition here is consistent with the change made inauth_internal.h
. This consistency is crucial for maintaining the integrity of the operator definitions across the codebase.Please ensure that this change aligns with the expected behavior in all scenarios where this operator is used, particularly in complex equality comparisons.
https://github.com/shogo82148/actions-setup-mysql/actions/runs/10643017496/job/29509993834
Summary by CodeRabbit
New Features
WITH_EXPERIMENTAL_CPP20
) for building with C++20 features.Improvements
volatile
types withstd::atomic
.std::pair
involvingRole_id
types to align with new C++ standards.Bug Fixes
const
.