search

varnish / varnish-modules

Collection of Varnish Cache modules (vmods) by Varnish Software
Other
182 stars 86 forks source link

Varnish-modules wont build against varnish master. #105

Closed simonvik closed 6 years ago

simonvik commented 6 years ago

bodyaccess depends on VRBIterate and xkey depends on OEV*, ObjUnsubscribeEvents and multiple other functions.

Those functions and defines got moved to cache_varnishd.h in commit https://github.com/varnishcache/varnish-cache/commit/fee476acc085e6563225d35fd8e19f1b10f04139 and that file is not installed via the -dev package.

nigoroll commented 6 years ago

As I need the build to work now, I've just pushed a quick bandaid to https://github.com/nigoroll/varnish-modules

simonvik commented 6 years ago

That solution works but its super-bad that it requires source tree.. If you go with this change then a src-pkg for deb, rpm is needed i think.

dridi commented 6 years ago

This is on my plate for _today_.

dridi commented 6 years ago

Since PHK wanted cache_varnishd.h to land in the "-dev" package or otherwise enforce building against a source tree, I double-checked and figured out we shouldn't need to build against a source tree.

See varnishcache/varnish-cache@90c4b747ced9053353c13d827ef0a0df5ab4445f

simonvik commented 6 years ago

Thanks! Stuff that should *never* be exposed to a VMOD from the cache_varnishd.h isnt so nice but thanks for solving the issue.

dridi commented 6 years ago

At the same time giving access to the request body for example for security scanning was one of the goals of Varnish 4.0 and while it may not have materialized as a blessed VRT facility, vmod-bodyaccess was a first (out of tree) step in that direction.

Again, the introduction of a callback before objects leave storage was specifically added so that vmod-xkey could be open-sourced since its predecessor vmod-hashtwo would only work in Varnish Plus. Edit: in order to maintain consistency since evictions happen outside transactions, so outside VCL.

So yes, maybe cache_varnishd.h says this, but it also contradicts previous decisions so I wouldn't discard the possibility that maybe some of this history was forgotten when the decision was made to move things there and say those are no-no for VMODs.

I certainly hope this patch won't be reverted to bring us back to the Varnish 3 days.