search

varnishcache / libvmod-example

Starting point for vmod development for Varnish Cache
The Unlicense
13 stars 12 forks source link

out with autotools, in with cmake #9

Open gquintard opened 3 years ago

gquintard commented 3 years ago

it's the week-end, let's try something new!

I do realize that vcdk is the preferred method now, but having a lean repo just one git clone away is still very appealing to me.

This is mainly a test run for me to get accustomed to cmake, but I really liked what I saw and thought I should share. I opted against a dual autotools/cmake to show how much cruft we could remove, but I'm not against it.

@Dridi, alternatively, that can be used to add a plugin to vcdk, feel free to run with it if you prefer that approach

fwsGonzo commented 3 years ago

Hey! This looks great. I think a few things are still missing here, though. For example, it would have been nice to have a helper script with some functions that abstract away some of the boilerplate, like the creation of a VMOD shared library.

Additionally, tests need a few more properties, so they might also be better off as a function. For example, tests can have timeouts, skip return-codes, working directory and you can even mark a test as will-fail. Varnishtest currently uses the return code 77 for skipped tests.

Tests will have the name test01 which doesn't explain much, and it also prevents a bigger build system. Here we could do something like vmod_example_test_prints_hello from tests/prints_hello.vtc.

On that note I would also replace CMAKE_SOURCE_DIR with CMAKE_CURRENT_SOURCE_DIR, and I think that should be enough. I'm not sure if changing the binary dir also makes sense, but I guess that's just one of those things you will know at some point if you try to use this inside a bigger build system.

It is not currently possible to add VSCs.

Make sure that all the nice-to-have compiler flags are present: -fno-omit-frame-pointer so that the backtraces have better chances of working out-of-the-box. I think autotools adds _GNU_SOURCE, but I'm pretty sure that CMake doesn't!

Regardless, looks great :) It definitely covers the basic VMOD example very well!

gquintard commented 3 years ago

Thanks @fwsGonzo , that is awesome feedback, let's have a list to make sure we have everything:

That's quite the list, but feel free to pile on if you think some are missing. And of course, since you are the cmake expert here, feel free to add some commits in there (I can move the branch to this repo too if that makes it easier for you)

gquintard commented 3 years ago

@fwsGonzo , could you have a look at 40e68d5 and see if that sort of matches your expectations please?

fwsGonzo commented 3 years ago

I'll have a look when I find some time. It looks great. enable_testing() is a bit of a weirdo in that it only works when placed at the root level. So I'm not sure where to put it here. Maybe it's fine inside the function as vmod.cmake gets included, and will count as root? In a bigger build system the root CMakeLists takes care of that too.

gquintard commented 3 years ago

@fwsGonzo , poke :-)

fwsGonzo commented 2 years ago

Hey, thanks for the reminder! I've been finishing and moving into my new house the last few weeks and it's been very time consuming. I'll be having a go at this next week for sure!

fwsGonzo commented 2 years ago

From my PR at https://github.com/gquintard/libvmod-example/pull/1:

There are 2 things that may have to be done to make this the ultimate vmod.cmake:

  1. Detect that you are inside a larger build system and not use the system libvarnishapi. In that case you should use the libvarnishapi of the build system. This will allow things like fuzzing and sanitizing vmods and Varnish at the same time, per build folder. Very handy.
  2. Detect Varnish Plus and simply set VARNISH_PLUS in CMake as well as adding a -DVARNISH_PLUS to VMODs.

Something like that.