Skip to content

FEAT: PDB File generation to make Private symbols for DDBC Bindings #71

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bewithgaurav
Copy link
Collaborator

@bewithgaurav bewithgaurav commented Jun 9, 2025

Summary

This pull request introduces changes to improve debugging support for the ddbc_bindings project by enabling PDB (Program Database) file generation and ensuring proper handling of these files during the build process. The changes primarily focus on updating the CMake configuration and build script.

Debugging Support Improvements:

  • Enabled PDB generation for MSVC builds in Release mode: Added compile and link options in CMakeLists.txt to ensure PDB files are generated when building with MSVC in Release mode. (mssql_python/pybind/CMakeLists.txt, mssql_python/pybind/CMakeLists.txtR8-R13)

  • Configured PDB properties for ddbc_bindings target: Set properties in CMakeLists.txt to specify the PDB file name and output directory for the ddbc_bindings target. (mssql_python/pybind/CMakeLists.txt, mssql_python/pybind/CMakeLists.txtR113-R119)

  • Handled PDB file copying in build script: Updated build.bat to check for the existence of the PDB file after the build, copy it to the appropriate directory if found, and log diagnostic messages for success or warnings if the file is missing. (mssql_python/pybind/build.bat, mssql_python/pybind/build.batR147-R155)

Issue Reference

Fixes AB#37472

Solution Implemented

  • Updated CMakeLists and build.bat to generate a PDB file for release

Checklist

  • Tests Passed (if applicable)
  • Code is formatted
  • Docs Updated (if necessary)

Testing Performed

  • Unit Tests
  • Manual Testing
    • Python Version: 3.13.3
    • OS: Windows

@Copilot Copilot AI review requested due to automatic review settings June 9, 2025 11:41
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds PDB file generation for MSVC Release builds and updates the build script to copy PDBs alongside the generated PYD for improved debugging support.

  • Enable /Zi compile and /DEBUG link options for MSVC in Release mode
  • Set PDB name and output directory properties on the ddbc_bindings target
  • Extend build.bat to detect and copy the generated PDB file, logging success or warning

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
build.bat Added logic to copy %PDB_NAME% from output to source directory with diagnostic logs
CMakeLists.txt Enabled PDB generation via compile/link options and set target PDB properties
Comments suppressed due to low confidence (1)

mssql_python/pybind/build.bat:148

  • The batch variable %PYTAG% may not match ${PYTHON_VERSION} used in CMake for naming PDB files. Ensure the same version variable is used so the script looks for the correct file name.
set PDB_NAME=ddbc_bindings.cp%PYTAG%-%WHEEL_ARCH%.pdb

Comment on lines +9 to +11
# Enable PDB generation for all target types
add_compile_options("$<$<CONFIG:Release>:/Zi>")
add_link_options("$<$<CONFIG:Release>:/DEBUG /OPT:REF /OPT:ICF>")
Copy link
Preview

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider scoping these compile and link options to the ddbc_bindings target (using target_compile_options and target_link_options) instead of globally to avoid unintended effects on other targets.

Suggested change
# Enable PDB generation for all target types
add_compile_options("$<$<CONFIG:Release>:/Zi>")
add_link_options("$<$<CONFIG:Release>:/DEBUG /OPT:REF /OPT:ICF>")
# Enable PDB generation for the ddbc_bindings target
target_compile_options(ddbc_bindings PRIVATE "$<$<CONFIG:Release>:/Zi>")
target_link_options(ddbc_bindings PRIVATE "$<$<CONFIG:Release>:/DEBUG /OPT:REF /OPT:ICF>")

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant