Skip to content

Conversation

@bbannier
Copy link
Member

auxil/spicy or more correctly its dependency justrx emitted its test targets into the build and we reconfigured them. Since we will not build these targets going forward and they will actually be unknown remove their configuration.

auxil/spicy or more correctly its dependency justrx emitted its test
targets into the build and we reconfigured them. Since we will not build
these targets going forward and they will actually be unknown remove
their configuration.
hilti-rt-objects
hiltic
jrx-objects
retest
Copy link
Contributor

@awelzel awelzel May 27, 2025

Choose a reason for hiding this comment

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

Fly-by food-for-thought: Would we have a good place in the zeek repos to place this? Meaning the whole file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Using this in the for loop below could work as well in case Zeek would ever set SPICY_ENABLE_TESTS to on, but might hide problems as well (?)

if ( TARGET ... )

Copy link
Member Author

@bbannier bbannier May 27, 2025

Choose a reason for hiding this comment

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

All this file does is tweak the build of Spicy targets when it is built as part of Zeek, so there is zero potential for reuse. I'd just put this on top of the line doing add_subdirectory(auxil/spicy) in zeek/zeek 🤷.

Even when building with Spicy tests this should still work. One could see more errors, but the build would finish (unless building with -Werror, but okay). I don't think we need to accommodate for that scenario here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated, if the intention for this repo was "CMake functionality reusable across the ecosystem", it has accumulated a lot of stuff which should just be in zeek/zeek.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just put this on top of the line doing add_subdirectory(auxil/spicy) in zeek/zeek

👍

Unrelated, if the intention for this repo was "CMake functionality reusable across the ecosystem", it has accumulated a lot of stuff which should just be in zeek/zeek.

Yeah, that's the angle I was looking at this change right now, too.

@bbannier bbannier merged commit 26f313a into master May 27, 2025
1 check passed
@bbannier bbannier deleted the topic/bbannier/spicy-test-targets branch May 27, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants