Skip to content

Fix generated proto exports in Windows shared builds - #5346

Open
PharahAmari wants to merge 1 commit into
google:mainfrom
PharahAmari:fix/windows-proto-shared-exports
Open

Fix generated proto exports in Windows shared builds#5346
PharahAmari wants to merge 1 commit into
google:mainfrom
PharahAmari:fix/windows-proto-shared-exports

Conversation

@PharahAmari

Copy link
Copy Markdown

Problem

When generated proto object libraries are compiled as part of a shared MSVC build, OR_TOOLS_COMPILE_DEFINITIONS can provide the consumer-side dllimport definition while the target also adds its private dllexport definition. Both definitions then appear on the compiler command line. If dllimport wins by command-line ordering, MSVC rejects definitions of generated static data members with C2491.

Change

Filter the current generated proto target's consumer-side import macro out of the common compile definitions before applying the target's explicit interface/private import/export definitions.

Non-MSVC and static builds are unchanged.

Validation

  • Configured and built a native Windows shared OR-Tools Python wheel with MSVC.
  • Verified generated proto compilation contains only the target's dllexport definition.
  • Successfully linked ortools.dll and all Python extension modules.
  • Imported MathOpt protobuf modules and solved a CP-SAT smoke model from the resulting wheel.

@google-cla

google-cla Bot commented Sep 7, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@PharahAmari
PharahAmari force-pushed the fix/windows-proto-shared-exports branch from aeaf4d5 to 6937b24 Compare September 7, 2026 09:03
@Mizux Mizux added this to the v10.0 Beta milestone Sep 7, 2026
@Mizux Mizux self-assigned this Sep 7, 2026
@Mizux Mizux added OS: Windows Windows OS Build: CMake CMake based build issue Bug labels Sep 7, 2026
Comment thread cmake/cpp.cmake
Comment on lines 53 to 55
list(APPEND OR_TOOLS_COMPILE_DEFINITIONS "OR_ORTOOLS_PROTO_DLL=__declspec(dllimport)")
else()
list(APPEND OR_TOOLS_COMPILE_DEFINITIONS "OR_ORTOOLS_PROTO_DLL=")

@Mizux Mizux Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why not removing theses lines (53 to 55) from cpp.cmake instead of filter out later ?

or-tools/cmake/cpp.cmake

Lines 53 to 55 in 5d0f4ac

list(APPEND OR_TOOLS_COMPILE_DEFINITIONS "OR_ORTOOLS_PROTO_DLL=__declspec(dllimport)")
else()
list(APPEND OR_TOOLS_COMPILE_DEFINITIONS "OR_ORTOOLS_PROTO_DLL=")

knowing we already have:

or-tools/cmake/cpp.cmake

Lines 567 to 572 in 5d0f4ac

if(MSVC AND BUILD_SHARED_LIBS)
target_compile_definitions(${PROTO_NAME}_proto INTERFACE "OR_${PROTO_UPPER_NAME}_PROTO_DLL=__declspec(dllimport)")
target_compile_definitions(${PROTO_NAME}_proto PRIVATE "OR_${PROTO_UPPER_NAME}_PROTO_DLL=__declspec(dllexport)")
else()
target_compile_definitions(${PROTO_NAME}_proto PUBLIC "OR_${PROTO_UPPER_NAME}_PROTO_DLL=")
endif()

and

or-tools/cmake/cpp.cmake

Lines 602 to 604 in 5d0f4ac

generate_proto_library(
NAME ortools
FILES ${OR_TOOLS_PROTO_FILES})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Build: CMake CMake based build issue OS: Windows Windows OS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants