Copilot commented on code in PR #50406: URL: https://github.com/apache/arrow/pull/50406#discussion_r3545107643
########## ci/scripts/python_build.bat: ########## @@ -59,8 +59,14 @@ set ARROW_WITH_LZ4=ON set ARROW_WITH_SNAPPY=ON set ARROW_WITH_ZLIB=ON set ARROW_WITH_ZSTD=ON -set CMAKE_BUILD_TYPE=Release +set CMAKE_BUILD_TYPE=RelWithDebInfo set CMAKE_GENERATOR=Ninja +@rem Set CMAKE_CXX_FLAGS_RELWITHDEBINFO and CMAKE_C_FLAGS_RELWITHDEBINFO to +@rem override default /DNDEBUG to be dropped so assertions are maintained. +@rem A debug build would require linking against python3xx_d.lib (debug). +@rem See details of discussion on PR GH-50406 +set CMAKE_CXX_FLAGS_RELWITHDEBINFO=/O2 /Ob1 +set CMAKE_C_FLAGS_RELWITHDEBINFO=/O2 /Ob1 Review Comment: Overriding `CMAKE_*_FLAGS_RELWITHDEBINFO` to `/O2 /Ob1` drops MSVC’s default debug-info flag for RelWithDebInfo (typically `/Zi`). That undermines the stated goal of keeping debug information while enabling assertions (by removing `/DNDEBUG`). Consider keeping `/Zi` (or `/Z7`) while omitting `/DNDEBUG`. ########## ci/scripts/python_build.bat: ########## @@ -59,8 +59,14 @@ set ARROW_WITH_LZ4=ON set ARROW_WITH_SNAPPY=ON set ARROW_WITH_ZLIB=ON set ARROW_WITH_ZSTD=ON -set CMAKE_BUILD_TYPE=Release +set CMAKE_BUILD_TYPE=RelWithDebInfo set CMAKE_GENERATOR=Ninja +@rem Set CMAKE_CXX_FLAGS_RELWITHDEBINFO and CMAKE_C_FLAGS_RELWITHDEBINFO to +@rem override default /DNDEBUG to be dropped so assertions are maintained. +@rem A debug build would require linking against python3xx_d.lib (debug). +@rem See details of discussion on PR GH-50406 Review Comment: The comment references "PR GH-50406", but this PR is GH-47583 and the linked discussion in the PR metadata points to GH-47567. This looks like a stale/incorrect reference and may confuse future readers. -- 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]
