Fix C++ client cannot be built on Windows #10363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Currently there's no CI build process for Windows. It's different from Unix systems that could install a compiler easily. Also I'm not familiar with how to install the necessary softwares using pure commands on Windows. I built Pulsar C++ client on Windows with CMake 3.20 and Visual Studio 2019 Community, though I didn't use the IDE directly but only use CMake to detect the Windows C++ compiler behind the IDE. |
|
I think we can release the C++ client to some existed package manager. I see https://github.com/edenhill/librdkafka uses NuGet for package management. Also I think https://github.com/microsoft/vcpkg is good. |
|
/pulsarbot run-failure-checks |
merlimat
approved these changes
Apr 25, 2021
codelipenghui
approved these changes
Apr 26, 2021
codelipenghui
pushed a commit
that referenced
this pull request
Apr 26, 2021
### Motivation C++ source code cannot be built on Windows. The build commands are: ```powershell > cmake -B _builds -S . ` -DBUILD_PYTHON_WRAPPER=OFF -DBUILD_TESTS=OFF > cmake --build .\_builds\ ``` There're two errors. One is > pulsar-client-cpp\lib\ReaderImpl.h(34): error C2144: syntax error: 'int' should be preceded by ';' > ... It's because `PULSAR_PUBLIC` is `__declspec(dllexport)` on Windows platform and it should be put before the variable type. The other error is > LINK : fatal error LNK1104: cannot open file 'pulsar.lib' pulsar\pulsar-client-cpp\_builds \examples\SampleReaderCApi.vcxproj] > ... It looks like when files under `examples` and `perf` directories were compiled, it tried to link `pulsar.lib` and CMake target `pulsarShared`'s name has a `dll` suffix, like `pulsardll.lib` and `pulsardll.dll`. The suffix is redundant. ### Modifications - Put `PULSAR_PUBLIC` before the variable type. - Keep the `LIB_NAME` as the shared library's name, i.e. remove the `dll` suffix. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change is a trivial rework / code cleanup without any test coverage. (cherry picked from commit 8fed161)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
C++ source code cannot be built on Windows. The build commands are:
There're two errors. One is
It's because
PULSAR_PUBLICis__declspec(dllexport)on Windows platform and it should be put before the variable type.The other error is
It looks like when files under
examplesandperfdirectories were compiled, it tried to linkpulsar.liband CMake targetpulsarShared's name has adllsuffix, likepulsardll.libandpulsardll.dll. The suffix is redundant.Modifications
PULSAR_PUBLICbefore the variable type.LIB_NAMEas the shared library's name, i.e. remove thedllsuffix.Verifying this change
This change is a trivial rework / code cleanup without any test coverage.