Skip to content

Support building with MSVC #23

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

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Oct 27, 2022

Fixes #7

Motivation

The Python client cannot be built with MSVC.

Modifications

CMakeLists.txt:

  1. Boost.Python cannot be found on Windows. The component of Boost
    cannot be python3. It should be a specific version like
    python310. Therefore, find all possible components from 3.10 to 3.7
    until the first component is available.
  2. For MSVC, link to pulsarWithDeps.lib when LINK_STATIC is ON and
    set the MSVC_RUNTIME_LIBRARY target property to MultiThreaded.
  3. Change the suffix from .so to .pyd because Python on Windows
    recognizes *.pyd as the dynamic library.
  4. Link to Python library with MSVC, otherwise the symbos cannot be
    found when linking boost-python.

README: tell users how to build Python client on Windows.

Add a workflow to build Python wheels of versions 3.7 to 3.10 on Windows
and verify the build will succeed.

NOTE

With this PR, Windows users still need to put the related
boost_python*.dll under the system path. In future, the boost-python
dependency will be removed. See
#24.

@BewareMyPower BewareMyPower self-assigned this Oct 27, 2022
@BewareMyPower BewareMyPower marked this pull request as draft October 27, 2022 08:53
@BewareMyPower BewareMyPower force-pushed the bewaremypower/win-build branch from e19006e to 0b4de27 Compare October 27, 2022 09:08
@BewareMyPower BewareMyPower marked this pull request as ready for review October 27, 2022 09:08
@BewareMyPower
Copy link
Contributor Author

The Boost.Python dependency is still linked dynamically even if LINK_STATIC is ON. I have struggled for some time and opened an issue here: microsoft/vcpkg#27491. Not sure if it's a bug of Vcpkg or Boost.Python itself.

@BewareMyPower BewareMyPower marked this pull request as draft October 28, 2022 09:11
@BewareMyPower BewareMyPower force-pushed the bewaremypower/win-build branch from 9e868e3 to 3ac4f3e Compare November 1, 2022 06:45
@BewareMyPower BewareMyPower added this to the 3.0.0 milestone Nov 1, 2022
@BewareMyPower BewareMyPower marked this pull request as ready for review November 1, 2022 06:48
@BewareMyPower BewareMyPower marked this pull request as draft November 1, 2022 06:51
@BewareMyPower BewareMyPower force-pushed the bewaremypower/win-build branch from 3ac4f3e to a2c04af Compare November 1, 2022 09:26
Fixes apache#7

The Python client cannot be built with MSVC.

CMakeLists.txt:
1. Boost.Python cannot be found on Windows. The component of Boost
   cannot be `python3`. It should be a specific version like
   `python310`. Therefore, find all possible components from 3.10 to 3.7
   until the first component is available.
2. For MSVC, link to `pulsarWithDeps.lib` when `LINK_STATIC` is `ON` and
   set the `MSVC_RUNTIME_LIBRARY` target property to `MultiThreaded`.
3. Change the suffix from `.so` to `.pyd` because Python on Windows
   recognizes `*.pyd` as the dynamic library.
4. Link to Python library with MSVC, otherwise the symbos cannot be
   found when linking `boost-python`.

README: tell users how to build Python client on Windows.

Add a workflow to build Python wheels of versions 3.7 to 3.10 on Windows
and verify the build will succeed.

With this PR, Windows users still need to put the related
`boost_python*.dll` under the system path. In future, the `boost-python`
dependency will be removed. See
apache#24.
@BewareMyPower BewareMyPower force-pushed the bewaremypower/win-build branch from a2c04af to 3475de8 Compare November 1, 2022 10:50
@BewareMyPower BewareMyPower marked this pull request as ready for review November 1, 2022 12:39
@BewareMyPower
Copy link
Contributor Author

@merlimat @Demogorgon314 @RobertIndie Now this Pr is ready to review, PTAL.

I didn't add the packaging workflow. The only dependency is the boost python DLL. I think we only need to upload the .whl and the DLL as the artifacts.

@BewareMyPower
Copy link
Contributor Author

microsoft/vcpkg#27491 (comment) can link Boost.Python statically, but it requires other libraries are multi-threaded dynamically-linked. Since pulsarWithAllDeps.lib is still multi-threaded statically-linked, there will be a conflict. The solution might be a little complicated, which requires some changes to build the Pulsar C++ client. So this PR still links Boost.Python dynamically.

@merlimat merlimat merged commit 325f852 into apache:main Nov 2, 2022
@merlimat
Copy link
Contributor

merlimat commented Nov 2, 2022

@BewareMyPower perhaps the DLL can be included in the wheel file, in similar way as we’re doing in Linux, where “auditwheel” command is fixing the wheel by adding the dependencies.

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

Successfully merging this pull request may close these issues.

Windows builds
3 participants