Skip to content

Commit 52d72ef

Browse files
committed
st: fix CI - clang-tidy performance checks and MSBuild proto ordering
Two PR #600 failures the reflectcpp fix exposed once the build got further: Lint (clang-tidy, performance-* treated as errors) on lib/st/ClientImpl.cc: - the std::move of the trivially-copyable TransactionPolicy constructor argument had no effect; take it by const reference and copy the member. - the four not-yet-implemented create/subscribe stubs take their config by value (the sink the real implementation will move from) but do not consume it yet, so performance-unnecessary-value-param fired. Suppress it with a NOLINTNEXTLINE on each stub rather than churning the by-value signature the follow-up phases need (which would cascade const-ref up the builder chain). Windows (Visual Studio generator) failed compiling ST_OBJECT_LIB with "Cannot open include file: PulsarApi.pb.h": the st sources include the generated proto header, and while linking PROTO_OBJECTS propagates its include directory, the OBJECT->OBJECT link does not reliably order the build under MSBuild, so the st sources compiled before the proto existed. Add an explicit add_dependencies(ST_OBJECT_LIB PROTO_OBJECTS) (and the legacy PULSAR_OBJECT_LIB equivalent) and the generated-dir include. Verified: full local build + 83/83 pulsar-st-tests green; clang-format clean. Signed-off-by: Matteo Merli <mmerli@apache.org>
1 parent fbb1a68 commit 52d72ef

3 files changed

Lines changed: 20 additions & 3 deletions

File tree

lib/CMakeLists.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,17 @@ set_property(TARGET ST_OBJECT_LIB PROPERTY POSITION_INDEPENDENT_CODE 1)
8989
set_target_properties(ST_OBJECT_LIB PROPERTIES CXX_STANDARD 20 CXX_STANDARD_REQUIRED ON)
9090
if (INTEGRATE_VCPKG)
9191
target_link_libraries(ST_OBJECT_LIB PROTO_OBJECTS)
92+
# The st sources #include the generated PulsarApi.pb.h. Linking PROTO_OBJECTS
93+
# propagates its include dir, but the OBJECT->OBJECT link does not reliably
94+
# order the build under the Visual Studio generator, so the st sources can
95+
# compile before the proto is generated. Force the ordering explicitly.
96+
add_dependencies(ST_OBJECT_LIB PROTO_OBJECTS)
97+
else ()
98+
# Legacy (non-vcpkg) build generates the proto as part of PULSAR_OBJECT_LIB;
99+
# ensure it exists before the st sources that include it compile.
100+
add_dependencies(ST_OBJECT_LIB PULSAR_OBJECT_LIB)
92101
endif ()
102+
target_include_directories(ST_OBJECT_LIB PRIVATE ${LIB_AUTOGEN_DIR})
93103
target_include_directories(ST_OBJECT_LIB PUBLIC
94104
"${CMAKE_SOURCE_DIR}"
95105
"${CMAKE_SOURCE_DIR}/include"

lib/st/ClientImpl.cc

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,28 @@ Future<T> notImplementedYet(const char* what) {
3838

3939
} // namespace
4040

41-
ClientImpl::ClientImpl(pulsar::ClientImplPtr classicClient, TransactionPolicy transactionPolicy)
42-
: classic_(std::move(classicClient)), transactionPolicy_(std::move(transactionPolicy)) {}
41+
ClientImpl::ClientImpl(pulsar::ClientImplPtr classicClient, const TransactionPolicy& transactionPolicy)
42+
: classic_(std::move(classicClient)), transactionPolicy_(transactionPolicy) {}
4343

44+
// The producer/consumer/transaction paths land in follow-up phases. Each takes
45+
// its config by value (the sink the real implementation will move from), but as
46+
// a stub it does not consume the config yet — hence the value-param suppressions.
47+
// NOLINTNEXTLINE(performance-unnecessary-value-param)
4448
Future<detail::ProducerCore> ClientImpl::createProducerAsync(ProducerConfig) {
4549
return notImplementedYet<detail::ProducerCore>("createProducer");
4650
}
4751

52+
// NOLINTNEXTLINE(performance-unnecessary-value-param)
4853
Future<detail::StreamConsumerCore> ClientImpl::subscribeStreamAsync(StreamConsumerConfig) {
4954
return notImplementedYet<detail::StreamConsumerCore>("subscribeStream");
5055
}
5156

57+
// NOLINTNEXTLINE(performance-unnecessary-value-param)
5258
Future<detail::QueueConsumerCore> ClientImpl::subscribeQueueAsync(QueueConsumerConfig) {
5359
return notImplementedYet<detail::QueueConsumerCore>("subscribeQueue");
5460
}
5561

62+
// NOLINTNEXTLINE(performance-unnecessary-value-param)
5663
Future<detail::CheckpointConsumerCore> ClientImpl::createCheckpointConsumerAsync(CheckpointConsumerConfig) {
5764
return notImplementedYet<detail::CheckpointConsumerCore>("createCheckpointConsumer");
5865
}

lib/st/ClientImpl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ namespace pulsar::st {
4444
*/
4545
class ClientImpl {
4646
public:
47-
ClientImpl(pulsar::ClientImplPtr classicClient, TransactionPolicy transactionPolicy);
47+
ClientImpl(pulsar::ClientImplPtr classicClient, const TransactionPolicy& transactionPolicy);
4848

4949
Future<detail::ProducerCore> createProducerAsync(ProducerConfig config);
5050
Future<detail::StreamConsumerCore> subscribeStreamAsync(StreamConsumerConfig config);

0 commit comments

Comments
 (0)