Skip to content
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

Add plugin/CMakeLists.txt #37

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 2 additions & 26 deletions features/plugin/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ cmake_minimum_required(VERSION 3.15.0 FATAL_ERROR)

project(ZeekPlugin@NAME@)

# Establish version numbers in config.h
# Establish version numbers
file(STRINGS "${CMAKE_CURRENT_SOURCE_DIR}/VERSION" VERSION LIMIT_COUNT 1)

string(REGEX REPLACE "[.-]" " " version_numbers ${VERSION})
Expand All @@ -13,34 +13,10 @@ list(GET version_numbers 0 VERSION_MAJOR)
list(GET version_numbers 1 VERSION_MINOR)
list(GET version_numbers 2 VERSION_PATCH)

configure_file("${CMAKE_CURRENT_SOURCE_DIR}/plugin/src/config.h.in"
"${CMAKE_CURRENT_BINARY_DIR}/config.h" @ONLY)

# Process any package-specific customizations
include(plugin.cmake OPTIONAL)

# Our plugin source and scripts are in a subdirectory
set(BRO_PLUGIN_BASE "${CMAKE_CURRENT_SOURCE_DIR}/plugin")

# Workaround to make header files in plugin sources available to BiFs.
include_directories("${BRO_PLUGIN_BASE}/src")

include(ZeekPlugin)

zeek_plugin_begin(@NS@ @NAME@)

file(GLOB cc_files RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}" "plugin/src/*.cc")
foreach(file ${cc_files})
zeek_plugin_cc(${file})
endforeach ()

file(GLOB bif_files RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}" "plugin/src/*.bif")
foreach(file ${bif_files})
zeek_plugin_bif(${file})
endforeach ()

zeek_plugin_dist_files(README CHANGES COPYING VERSION)
zeek_plugin_end()
add_subdirectory(plugin)

if ("${PROJECT_SOURCE_DIR}" STREQUAL "${CMAKE_SOURCE_DIR}")
# Allows building rpm/deb packages via "make package" in build dir.
Expand Down
30 changes: 30 additions & 0 deletions features/plugin/plugin/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/src/config.h.in"
"${CMAKE_CURRENT_BINARY_DIR}/config.h" @ONLY)

include(ZeekPlugin)

zeek_plugin_begin(@NS@ @NAME@)

file(GLOB cc_files RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}" "src/*.cc")
foreach(file ${cc_files})
zeek_plugin_cc(${file})
endforeach ()

file(GLOB bif_files RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}" "src/*.bif")
foreach(file ${bif_files})
zeek_plugin_bif(${file})
endforeach ()

# Copy the dist files to package with the tarball into the top-level
# build directory to fulfill zeek-plugin-create-package.sh assumptions
# about them being located in the parent directory.
set(dist_files README CHANGES COPYING VERSION)
foreach(file ${dist_files})
if ( EXISTS ../${file} )
file(COPY ../${file} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/../)
endif()
endforeach ()
Comment on lines +18 to +26
Copy link
Member

@bbannier bbannier Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I mentioned offline, not a fan of the magic modifications to the "file names" since this makes it pretty hard to modify for users, what if they want to e.g., include a generated file in the tarball? I'd prefer just directly referencing files with relative paths and requiring them to exist; in addition to the technical implementation comment we also need a user-targeted comment explaining what this variable is for -- for anything else we use magic globs 🤢.

Probably worse, I just noticed that packaging already copies some files around, https://github.com/zeek/cmake/blob/2df3b8e82a843b7b8187963d259d32a9fb42b873/ConfigurePackaging.cmake#L133-L140, i.e., we would already create a README.txt and COPYING.txt in the build directory. This could potentially clash with what users set up here (e.g., if they named their README as README.txt). I do not think there is a good automagic way to fix this, can we augment zeek-plugin-create-package.sh so it can understand explicit dist files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we augment zeek-plugin-create-package.sh so it can understand explicit dist files?

Not sure I fully understand how to do this, but agree the file copying isn't great - it just attempts to please zeek-plugin-create-package.sh. Plainly using .../VERSION currently fails as the create package script then copies it not into dist, but dist/../VERSION. Hmm, we could possibly just strip the leading ../'s before determining the destination directory within ./dist, that could actually be "okay".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also just have the semantics that the files end up in the same place they were relative to the CMakeLists.txt with project here, e.g., from a subdirectory ../README (which is the same as <project>/README) gets copied <dist>/README. I left a similar comment on zeek/cmake#116.


zeek_plugin_dist_files(${dist_files})

zeek_plugin_end()
10 changes: 10 additions & 0 deletions tests/Baseline/features.plugin-init-preload/output
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
=== without test/scripts
plugin/scripts/__load__.zeek
plugin/scripts/__preload__.zeek
=== with test/scripts
plugin/scripts/__load__.zeek
plugin/scripts/__preload__.zeek
scripts/__load__.zeek
scripts/main.zeek
Hello world!
1 change: 1 addition & 0 deletions tests/Baseline/features.plugin-license-github_ci/output
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ test/COPYING
test/plugin
test/plugin.cmake
test/plugin/.gitignore
test/plugin/CMakeLists.txt
test/plugin/scripts
test/plugin/scripts/types.zeek
test/plugin/scripts/__load__.zeek
Expand Down
24 changes: 24 additions & 0 deletions tests/features/plugin-init-preload
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# A test to verify that plugin/scripts/ is used as the plugin's script
# directory rather than the top-level scripts/ directory.
#
# Regression test for #35.
#
# @TEST-REQUIRES: make --version
# @TEST-REQUIRES: cmake --help | grep -q '^* Unix Makefiles'
#
# @TEST-EXEC: bash %INPUT
#
# @TEST-EXEC: cd test && ./configure 1>&2
# @TEST-EXEC: make -C test/build/ -j`nproc` 1>&2
# @TEST-EXEC: echo "=== without test/scripts" >>output
# @TEST-EXEC: ZEEK_PLUGIN_PATH=./test/build zeek </dev/null >>output
# @TEST-EXEC: echo "=== with test/scripts" >>output
# @TEST-EXEC: ZEEK_PLUGIN_PATH=./test/build zeek ./test/scripts >>output
# @TEST-EXEC: btest-diff output

${SCRIPTS}/zkg create --packagedir=test --features plugin --user-var name=Name --user-var namespace=Namespace

echo 'event zeek_init() &priority=40 { print "plugin/scripts/__load__.zeek"; }' >> test/plugin/scripts/__load__.zeek
echo 'event zeek_init() &priority=30 { print "plugin/scripts/__preload__.zeek"; }' >> test/plugin/scripts/__preload__.zeek
echo 'event zeek_init() &priority=20 { print "scripts/__load__.zeek"; }' >> test/scripts/__load__.zeek
echo 'event zeek_init() &priority=10 { print "scripts/main.zeek"; }' >> test/scripts/main.zeek