-
Notifications
You must be signed in to change notification settings - Fork 14
[several] Refactor, stage 2, take 2 #185
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
base: main
Are you sure you want to change the base?
Changes from all commits
0aee794
7ab75f2
e15c06e
fbca874
329e230
940023a
21aa935
000af29
fd7a566
789bca1
cd8a4d7
567d185
ff03645
3332008
9e9b36b
787678a
0083f3a
4dc054c
421a59f
4646653
cd8d17f
06b52e2
5746c45
e701367
4405600
70c17e2
c83a24e
95a7d06
a5ba63c
a152dd2
181e20e
588bbb9
75a0234
0838d18
a246ddb
4870fce
a82cac7
8c8e3fb
7db9378
5382e1c
685ed36
e5d2fcb
1f7977e
3179c94
f18a959
a357cd3
24c1d31
08f2868
105c376
31bae16
03170b5
3d2dd8c
0e69492
da8d552
f5ed970
0d7b2e2
96ff88e
919f7d4
6fd5c20
2afe889
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,10 +28,12 @@ set(IRODS_MINIMUM_VERSION "4.90.0") | |
| set(IRODS_MAXIMUM_VERSION "6.0.0") | ||
| find_package(IRODS "${IRODS_MINIMUM_VERSION}...<${IRODS_MAXIMUM_VERSION}" REQUIRED) | ||
|
|
||
| set(IRODS_PLUGIN_VERSION "5.0.1") | ||
| set(IRODS_PLUGIN_VERSION "5.1.0") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NTS: Update milestone.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just making sure: As far as we know, no interfaces have changed and no behavioral changes are considered breaking, right? In other words, no need for a major version with these changes?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new block-by-default failsafe mode could be considered a breaking change, though a configuration tweak would restore the previous behavior.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i have zero concerns with this being a major release of the plugin, for the record. that said, the behavior as i understand it today, is that if the AMQP endpoint was not happy... iRODS just stopped/blocked/appeared broken. So... this could be argued is a bugfix from a certain perspective. I defer on the versioning thing. I think it's fine either way.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on the explanation in #192, I think the failsafe mode is in fact making unintended-yet-desirable behavior into an official (and actually functioning) feature, and a configurable one. So, yes, in some sense it is in fact a bug fix. Anyway, it doesn't necessarily seem worthy of a major version bump after seeing these comments - minor seems fine. |
||
|
|
||
| set(IRODS_PACKAGE_REVISION "0") | ||
|
|
||
| set(IRODS_RULE_ENGINE_PLUGIN_NAME "audit_amqp") | ||
| set(IRODS_PLUGIN_FULL_NAME "irods_rule_engine_plugin-${IRODS_RULE_ENGINE_PLUGIN_NAME}") | ||
| set(PLUGIN irods_rule_engine_plugin-audit_amqp) | ||
|
|
||
| include(IrodsCXXCompiler) | ||
|
|
@@ -46,10 +48,10 @@ include(IrodsExternals) | |
|
|
||
| IRODS_MACRO_CHECK_DEPENDENCY_SET_FULLPATH_ADD_TO_IRODS_PACKAGE_DEPENDENCIES_LIST(QPID_PROTON qpid-proton0.36.0-3) | ||
|
|
||
| string(REPLACE ";" ", " ${PLUGIN}_PACKAGE_DEPENDENCIES_STRING "${IRODS_PACKAGE_DEPENDENCIES_LIST}") | ||
| string(REPLACE ";" ", " ${IRODS_PLUGIN_FULL_NAME}_PACKAGE_DEPENDENCIES_STRING "${IRODS_PACKAGE_DEPENDENCIES_LIST}") | ||
| unset(IRODS_PACKAGE_DEPENDENCIES_LIST) | ||
|
|
||
| project(irods_rule_engine_plugin-audit_amqp | ||
| project("${IRODS_PLUGIN_FULL_NAME}" | ||
| VERSION "${IRODS_PLUGIN_VERSION}" | ||
| LANGUAGES CXX) | ||
|
|
||
|
|
@@ -62,58 +64,111 @@ endif() | |
|
|
||
| include(GNUInstallDirs) | ||
| include(CheckCXXSymbolExists) | ||
| include(ObjectTargetHelpers) | ||
|
|
||
| find_package(Threads REQUIRED) | ||
| find_package(OpenSSL REQUIRED COMPONENTS Crypto SSL) | ||
| find_package(nlohmann_json "3.6.1" REQUIRED) | ||
| find_package(fmt "8.1.1" REQUIRED) | ||
|
|
||
| add_library( | ||
| ${PLUGIN} | ||
| MODULE | ||
| set( | ||
| ${IRODS_PLUGIN_FULL_NAME}_SOURCES | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/src/main.cpp" | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/src/amqp_config.cpp" | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/src/amqp_sender.cpp" | ||
| ) | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/src/audit_config.cpp" | ||
| ) | ||
| set( | ||
| ${PLUGIN}_HEADERS | ||
| ${IRODS_PLUGIN_FULL_NAME}_HEADERS | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/amqp_config.hpp" | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/amqp_sender.hpp" | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/audit_amqp.hpp" | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/audit_config.hpp" | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/audit_b64enc.hpp" | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/amqp_sender.hpp" | ||
| ) | ||
| target_sources(${PLUGIN} PRIVATE ${${PLUGIN}_HEADERS}) | ||
| target_compile_options(${PLUGIN} PRIVATE -Wno-write-strings) | ||
| target_compile_definitions(${PLUGIN} PRIVATE ${IRODS_COMPILE_DEFINITIONS} ${IRODS_COMPILE_DEFINITIONS_PRIVATE} IRODS_ENABLE_SYSLOG) | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/proton_formatters.hpp" | ||
| ) | ||
| set( | ||
| ${IRODS_PLUGIN_FULL_NAME}_HEADERS_GENERATORS | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/audit_amqp_version.hpp.in" | ||
| ) | ||
| set( | ||
| ${IRODS_PLUGIN_FULL_NAME}_HEADERS_GENERATED | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/audit_amqp_version.hpp" | ||
| ) | ||
|
|
||
| set_source_files_properties( | ||
| "${${IRODS_PLUGIN_FULL_NAME}_HEADERS_GENERATORS}" | ||
| PROPERTIES | ||
| HEADER_FILE_ONLY ON | ||
| ) | ||
| set_source_files_properties( | ||
| "${${IRODS_PLUGIN_FULL_NAME}_HEADERS_GENERATED}" | ||
| PROPERTIES | ||
| GENERATED ON | ||
| ) | ||
|
korydraughn marked this conversation as resolved.
|
||
|
|
||
| configure_file( | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include/irods/private/audit_amqp_version.hpp.in" | ||
| "${CMAKE_CURRENT_BINARY_DIR}/include/irods/private/audit_amqp_version.hpp" | ||
| @ONLY | ||
| ) | ||
|
|
||
| add_library( | ||
| ${IRODS_PLUGIN_FULL_NAME}_obj | ||
| OBJECT | ||
| "${${IRODS_PLUGIN_FULL_NAME}_SOURCES}" | ||
| "${${IRODS_PLUGIN_FULL_NAME}_HEADERS}" | ||
| "${${IRODS_PLUGIN_FULL_NAME}_HEADERS_GENERATORS}" | ||
| "${${IRODS_PLUGIN_FULL_NAME}_HEADERS_GENERATED}" | ||
| ) | ||
| target_compile_options(${IRODS_PLUGIN_FULL_NAME}_obj PRIVATE -Wno-write-strings) | ||
| target_compile_definitions(${IRODS_PLUGIN_FULL_NAME}_obj PRIVATE ${IRODS_COMPILE_DEFINITIONS} ${IRODS_COMPILE_DEFINITIONS_PRIVATE} IRODS_ENABLE_SYSLOG) | ||
| target_include_directories( | ||
| ${PLUGIN} | ||
| PRIVATE | ||
| ${IRODS_PLUGIN_FULL_NAME}_obj | ||
| PUBLIC | ||
| $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> | ||
| $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include> | ||
| PRIVATE | ||
| ${IRODS_INCLUDE_DIRS} | ||
| ${IRODS_EXTERNALS_FULLPATH_BOOST}/include | ||
| ${IRODS_EXTERNALS_FULLPATH_QPID_PROTON}/include | ||
| ) | ||
|
|
||
| target_link_libraries( | ||
| ${PLUGIN} | ||
| PRIVATE | ||
| ${IRODS_PLUGIN_FULL_NAME}_obj | ||
| PUBLIC | ||
| irods_server | ||
| irods_common | ||
| nlohmann_json::nlohmann_json | ||
| fmt::fmt | ||
| PRIVATE | ||
| ${IRODS_EXTERNALS_FULLPATH_QPID_PROTON}/lib/libqpid-proton-cpp.so | ||
| ${IRODS_EXTERNALS_FULLPATH_BOOST}/lib/libboost_system.so | ||
| ${IRODS_EXTERNALS_FULLPATH_BOOST}/lib/libboost_url.so | ||
| ${CMAKE_DL_LIBS} | ||
| ) | ||
|
|
||
| if (CMAKE_BUILD_TYPE STREQUAL "Debug") | ||
| target_compile_definitions(${IRODS_PLUGIN_FULL_NAME}_obj PRIVATE IRODS_AUDIT_EXTRA_TRACE) | ||
| endif() | ||
|
|
||
| check_cxx_symbol_exists(__cpp_lib_filesystem "version" HAVE_STD_FILESYSTEM) | ||
| if (NOT HAVE_STD_FILESYSTEM) | ||
| target_link_libraries( | ||
| ${PLUGIN} | ||
| ${IRODS_PLUGIN_FULL_NAME}_obj | ||
| PRIVATE | ||
| ${IRODS_EXTERNALS_FULLPATH_BOOST}/lib/libboost_filesystem.so | ||
| ) | ||
| endif() | ||
|
|
||
| install(TARGETS ${PLUGIN} LIBRARY DESTINATION ${IRODS_PLUGINS_DIRECTORY}/rule_engines) | ||
| add_library(${IRODS_PLUGIN_FULL_NAME} MODULE) | ||
| target_link_objects( | ||
| ${IRODS_PLUGIN_FULL_NAME} | ||
| PRIVATE | ||
| ${IRODS_PLUGIN_FULL_NAME}_obj | ||
| ) | ||
|
|
||
| install(TARGETS ${IRODS_PLUGIN_FULL_NAME} LIBRARY DESTINATION ${IRODS_PLUGINS_DIRECTORY}/rule_engines) | ||
|
|
||
| install( | ||
| FILES | ||
|
|
@@ -163,10 +218,10 @@ set(CPACK_RPM_FILE_NAME RPM-DEFAULT) | |
| set(CPACK_ARCHIVE_COMPONENT_INSTALL OFF) | ||
|
|
||
| set(CPACK_DEBIAN_PACKAGE_NAME ${PLUGIN_PACKAGE_NAME}) | ||
| set(CPACK_DEBIAN_PACKAGE_DEPENDS "${IRODS_PACKAGE_DEPENDENCIES_STRING}, ${${PLUGIN}_PACKAGE_DEPENDENCIES_STRING}, irods-runtime (= ${IRODS_VERSION})") | ||
| set(CPACK_DEBIAN_PACKAGE_DEPENDS "${IRODS_PACKAGE_DEPENDENCIES_STRING}, ${${IRODS_PLUGIN_FULL_NAME}_PACKAGE_DEPENDENCIES_STRING}, irods-runtime (= ${IRODS_VERSION})") | ||
|
|
||
| set(CPACK_RPM_PACKAGE_NAME ${PLUGIN_PACKAGE_NAME}) | ||
| set(CPACK_RPM_PACKAGE_REQUIRES "${IRODS_PACKAGE_DEPENDENCIES_STRING}, ${${PLUGIN}_PACKAGE_DEPENDENCIES_STRING}, irods-runtime = ${IRODS_VERSION}") | ||
| set(CPACK_RPM_PACKAGE_REQUIRES "${IRODS_PACKAGE_DEPENDENCIES_STRING}, ${${IRODS_PLUGIN_FULL_NAME}_PACKAGE_DEPENDENCIES_STRING}, irods-runtime = ${IRODS_VERSION}") | ||
|
|
||
|
|
||
| if (NOT CPACK_GENERATOR) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.