varnish / varnish-modules

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

Nigorolls masterport, rebased #103

Closed dmatetelki closed 6 years ago

dmatetelki commented 6 years ago

Compiled against VC-5.2.1

dridi commented 6 years ago

So we're dropping 4.1 support?

dmatetelki commented 6 years ago

@Dridi I thought the idea here too is to branch off 4.1.9 and have master as compatible with vc-master.

I don't see (as my vc knowledge is very limited) how we can make v-modules (and all the vmods) compile against both vc-4.1.9 and vc-master.

dridi commented 6 years ago

See here:

https://github.com/varnish/varnish-modules#administrativa

Expressed non goal:

Support older releases of Varnish Cache.

Either we have one branch supporting both, or we drop support for 4.1 altogether. Not to mention that if we branch off we'll need to make sure version numbers for each branch don't conflict with each other and I think we don't want to enter this territory.

I don't see (as my vc knowledge is very limited) how we can make v-modules (and all the vmods) compile against both vc-4.1.9 and vc-master.

Currently it compiles against both 4.1 and 5.x releases with the help of some processing, so there's no reason not to be able to do the same for mere #include order. So the question is, do we support 4.1 and add the #ifdef dance or do we drop support for 4.1 and 5.x altogether?

See my comments in #83 going along the same lines:

https://github.com/varnish/varnish-modules/pull/83#issuecomment-333810522 https://github.com/varnish/varnish-modules/pull/83#issuecomment-334818318 https://github.com/varnish/varnish-modules/pull/83#issuecomment-338655151 https://github.com/varnish/varnish-modules/pull/83#issuecomment-342132057

dmatetelki commented 6 years ago

@Dridi : I'll drop the "retire softpurge" and start to use your include order workaround.

dmatetelki commented 6 years ago

@Dridi : i followed your dark bidding, but the workaround:

#include <cache/cache.h>

#ifndef VRT_H_INCLUDED
#  include <vrt.h>
#endif

#ifndef VDEF_H_INCLUDED
#  include <vdef.h>
#endif

#include <vcl.h>

Is not enough for the 4.1.9 compatibility.