-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[c++] Define and expose PULSAR_VERSION macro #12769
Conversation
@@ -20,7 +20,7 @@ | |||
file(GLOB PULSAR_SOURCES *.cc *.h lz4/*.cc lz4/*.h checksum/*.cc checksum/*.h stats/*.cc stats/*.h c/*.cc c/*.h auth/*.cc auth/*.h auth/athenz/*.cc auth/athenz/*.h) | |||
|
|||
execute_process(COMMAND python ${CMAKE_SOURCE_DIR}/../src/get-project-version.py OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE PV) | |||
set (CMAKE_CXX_FLAGS " ${CMAKE_CXX_FLAGS} -D_PULSAR_VERSION_=\\\"${PV}\\\"") | |||
set (CMAKE_CXX_FLAGS " ${CMAKE_CXX_FLAGS} -D_PULSAR_VERSION_INTERNAL_=\\\"${PV}\\\"") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed this existing macro to avoid confusion with the macro added this time.
#define LIB_VERSION_INTERNAL_H_ | ||
|
||
#ifndef _PULSAR_VERSION_INTERNAL_ | ||
#define _PULSAR_VERSION_INTERNAL_ "1.17" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.17 is a concrete (and very old) version. Should we instead use something link unknown
or undefined
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@merlimat Changed the default value to unknown
.
* Define and expose PULSAR_VERSION macro * Change default value of _PULSAR_VERSION_INTERNAL_ to unknown
Add Ping @315157973 since you're the release manager of Pulsar 2.8.2. |
* Define and expose PULSAR_VERSION macro * Change default value of _PULSAR_VERSION_INTERNAL_ to unknown
…3324) ### Motivation When I build C++ tests on my local env, the following error happened. ``` tests/VersionTest.cc:19:10: fatal error: 'pulsar/Version.h' file not found #include <pulsar/Version.h> ``` It's because I specified another directory as CMake directory. ```bash mkdir _builds && cd _builds && cmake .. ``` After #12769, the `Version.h` is generated under `${CMAKE_BINARY_DIR}/include/pulsar` directory but it's not included in `CMakeLists.txt`. CI works well because it's built in the default CMake directory so that `CMAKE_BINARY_DIR` is the same with `CMAKE_SOURCE_DIR`, which is included. ### Modifications Add the `${CMAKE_BINARY_DIR}/include` to `included_directories`.
* Define and expose PULSAR_VERSION macro * Change default value of _PULSAR_VERSION_INTERNAL_ to unknown
…ache#13324) ### Motivation When I build C++ tests on my local env, the following error happened. ``` tests/VersionTest.cc:19:10: fatal error: 'pulsar/Version.h' file not found #include <pulsar/Version.h> ``` It's because I specified another directory as CMake directory. ```bash mkdir _builds && cd _builds && cmake .. ``` After apache#12769, the `Version.h` is generated under `${CMAKE_BINARY_DIR}/include/pulsar` directory but it's not included in `CMakeLists.txt`. CI works well because it's built in the default CMake directory so that `CMAKE_BINARY_DIR` is the same with `CMAKE_SOURCE_DIR`, which is included. ### Modifications Add the `${CMAKE_BINARY_DIR}/include` to `included_directories`.
* Define and expose PULSAR_VERSION macro * Change default value of _PULSAR_VERSION_INTERNAL_ to unknown (cherry picked from commit 2dcf7e9)
…3324) ### Motivation When I build C++ tests on my local env, the following error happened. ``` tests/VersionTest.cc:19:10: fatal error: 'pulsar/Version.h' file not found #include <pulsar/Version.h> ``` It's because I specified another directory as CMake directory. ```bash mkdir _builds && cd _builds && cmake .. ``` After #12769, the `Version.h` is generated under `${CMAKE_BINARY_DIR}/include/pulsar` directory but it's not included in `CMakeLists.txt`. CI works well because it's built in the default CMake directory so that `CMAKE_BINARY_DIR` is the same with `CMAKE_SOURCE_DIR`, which is included. ### Modifications Add the `${CMAKE_BINARY_DIR}/include` to `included_directories`. (cherry picked from commit ca37e67)
…3324) ### Motivation When I build C++ tests on my local env, the following error happened. ``` tests/VersionTest.cc:19:10: fatal error: 'pulsar/Version.h' file not found #include <pulsar/Version.h> ``` It's because I specified another directory as CMake directory. ```bash mkdir _builds && cd _builds && cmake .. ``` After #12769, the `Version.h` is generated under `${CMAKE_BINARY_DIR}/include/pulsar` directory but it's not included in `CMakeLists.txt`. CI works well because it's built in the default CMake directory so that `CMAKE_BINARY_DIR` is the same with `CMAKE_SOURCE_DIR`, which is included. ### Modifications Add the `${CMAKE_BINARY_DIR}/include` to `included_directories`. (cherry picked from commit ca37e67)
Motivation
Pulsar Node.js client library is a wrapping of the C++ client library. If we add a new feature to the Node.js client that uses a recently implemented C function, users will be forced to upgrade the C++ client, even if they don't use that feature. Otherwise, a compile error will occur when installing the Node.js client.
Modifications
To solve the above issue, define and expose a macro named
PULSAR_VERSION
in the C++ client. The value ofPULSAR_VERSION
is an integer determined by the following calculation:For example, if the version of Pulsar is 2.8.1,
PULSAR_VERSION
is2008001
.On the Node.js client side, we will no longer have to force users to upgrade the C++ client by using this macro as follows:
Verifying this change
Does this pull request potentially affect one of the following parts:
Documentation
no-need-doc