Make fails for varnish-6.2

marcinhlybin commented 5 years ago

Bunch of compiler errors while running make. This fork seems to solve this issue in branch 6.2: https://github.com/nigoroll/varnish-modules.git

root@vagrant:~/varnish-modules-0.15.0# ./bootstrap
+ mkdir -p m4
+ aclocal -I m4 -I /usr/share/aclocal
+ libtoolize --copy --force
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
+ autoheader
+ automake --add-missing --copy --foreign
configure.ac:14: installing 'build-aux/compile'
configure.ac:10: installing 'build-aux/missing'
src/Makefile.am: installing 'build-aux/depcomp'
+ autoconf
root@vagrant:~/varnish-modules-0.15.0# ./configure --prefix=/usr
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking how to print strings... printf
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking dependency style of gcc... gcc3
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking how to convert x86_64-pc-linux-gnu file names to x86_64-pc-linux-gnu format... func_convert_file_noop
checking how to convert x86_64-pc-linux-gnu file names to toolchain format... func_convert_file_noop
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for dlltool... no
checking how to associate runtime and link libraries... printf %s\n
checking for ar... ar
checking for archiver @FILE support... @
checking for strip... strip
checking for ranlib... ranlib
checking command to parse /usr/bin/nm -B output from gcc object... ok
checking for sysroot... no
checking for a working dd... /bin/dd
checking how to truncate binary pipes... /bin/dd bs=4096 count=1
checking for mt... mt
checking if mt is a manifest tool... no
checking how to run the C preprocessor... gcc -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking for dlfcn.h... yes
checking for objdir... .libs
checking if gcc supports -fno-rtti -fno-exceptions... no
checking for gcc option to produce PIC... -fPIC -DPIC
checking if gcc PIC flag -fPIC -DPIC works... yes
checking if gcc static flag -static works... yes
checking if gcc supports -c -o file.o... yes
checking if gcc supports -c -o file.o... (cached) yes
checking whether the gcc linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yes
checking whether -lc should be explicitly linked in... no
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking for shl_load... no
checking for shl_load in -ldld... no
checking for dlopen... no
checking for dlopen in -ldl... yes
checking whether a program can dlopen itself... yes
checking whether a statically linked program can dlopen itself... no
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... no
checking for the pthreads library -lpthreads... no
checking whether pthreads work without any flags... no
checking whether pthreads work with -Kthread... no
checking whether pthreads work with -kthread... no
checking for the pthreads library -llthread... no
checking whether pthreads work with -pthread... yes
checking for joinable pthread attribute... PTHREAD_CREATE_JOINABLE
checking if more special flags are required for pthreads... no
checking whether to build with code coverage support... no
checking for rst2man... rst2man
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for pkg-config... (cached) /usr/bin/pkg-config
checking pkg-config is at least version 0.21... yes
checking for VARNISHAPI... yes
checking for explicit_bzero... yes
checking for Varnish... 6.2.0
checking vsha256.h usability... yes
checking vsha256.h presence... yes
checking for vsha256.h... yes
checking cache/cache.h usability... yes
checking cache/cache.h presence... yes
checking for cache/cache.h... yes
checking for a Python interpreter with version >= 3.4... python3.6
checking for python3.6... /usr/bin/python3.6
checking for python3.6 version... 3.6
checking for python3.6 platform... linux
checking for python3.6 script directory... ${prefix}/lib/python3.6/site-packages
checking for python3.6 extension module directory... ${exec_prefix}/lib/python3.6/site-packages
checking for gcc option to accept ISO C99... none needed
checking cache/cache_varnishd.h usability... yes
checking cache/cache_varnishd.h presence... yes
checking for cache/cache_varnishd.h... yes
checking for struct objcore.exp... no
checking for struct objcore.hsh_list... yes
checking for struct director.admin_health... no
checking for req_body_iter_f... no
checking for objiterate_f... yes
checking for enum exp_event_e... no
checking for struct tcp_info... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating src/Makefile
config.status: creating config.h
config.status: config.h is unchanged
config.status: executing depfiles commands
config.status: executing libtool commands
root@vagrant:~/varnish-modules-0.15.0# make
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /root/varnish-modules-0.15.0/build-aux/missing aclocal-1.15 -I m4 -I /usr/share/aclocal
 cd . && /bin/bash /root/varnish-modules-0.15.0/build-aux/missing automake-1.15 --foreign
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /root/varnish-modules-0.15.0/build-aux/missing autoconf
/bin/bash ./config.status --recheck
running CONFIG_SHELL=/bin/bash /bin/bash ./configure --prefix=/usr --no-create --no-recursion
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking how to print strings... printf
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking dependency style of gcc... gcc3
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking how to convert x86_64-pc-linux-gnu file names to x86_64-pc-linux-gnu format... func_convert_file_noop
checking how to convert x86_64-pc-linux-gnu file names to toolchain format... func_convert_file_noop
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for dlltool... no
checking how to associate runtime and link libraries... printf %s\n
checking for ar... ar
checking for archiver @FILE support... @
checking for strip... strip
checking for ranlib... ranlib
checking command to parse /usr/bin/nm -B output from gcc object... ok
checking for sysroot... no
checking for a working dd... /bin/dd
checking how to truncate binary pipes... /bin/dd bs=4096 count=1
checking for mt... mt
checking if mt is a manifest tool... no
checking how to run the C preprocessor... gcc -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking for dlfcn.h... yes
checking for objdir... .libs
checking if gcc supports -fno-rtti -fno-exceptions... no
checking for gcc option to produce PIC... -fPIC -DPIC
checking if gcc PIC flag -fPIC -DPIC works... yes
checking if gcc static flag -static works... yes
checking if gcc supports -c -o file.o... yes
checking if gcc supports -c -o file.o... (cached) yes
checking whether the gcc linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yes
checking whether -lc should be explicitly linked in... no
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking for shl_load... no
checking for shl_load in -ldld... no
checking for dlopen... no
checking for dlopen in -ldl... yes
checking whether a program can dlopen itself... yes
checking whether a statically linked program can dlopen itself... no
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... no
checking for the pthreads library -lpthreads... no
checking whether pthreads work without any flags... no
checking whether pthreads work with -Kthread... no
checking whether pthreads work with -kthread... no
checking for the pthreads library -llthread... no
checking whether pthreads work with -pthread... yes
checking for joinable pthread attribute... PTHREAD_CREATE_JOINABLE
checking if more special flags are required for pthreads... no
checking whether to build with code coverage support... no
checking for rst2man... rst2man
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for pkg-config... (cached) /usr/bin/pkg-config
checking pkg-config is at least version 0.21... yes
checking for VARNISHAPI... yes
checking for explicit_bzero... yes
checking for Varnish... 6.2.0
checking vsha256.h usability... yes
checking vsha256.h presence... yes
checking for vsha256.h... yes
checking cache/cache.h usability... yes
checking cache/cache.h presence... yes
checking for cache/cache.h... yes
checking for a Python interpreter with version >= 3.4... python3.6
checking for python3.6... /usr/bin/python3.6
checking for python3.6 version... 3.6
checking for python3.6 platform... linux
checking for python3.6 script directory... ${prefix}/lib/python3.6/site-packages
checking for python3.6 extension module directory... ${exec_prefix}/lib/python3.6/site-packages
checking for gcc option to accept ISO C99... none needed
checking cache/cache_varnishd.h usability... yes
checking cache/cache_varnishd.h presence... yes
checking for cache/cache_varnishd.h... yes
checking for struct objcore.exp... no
checking for struct objcore.hsh_list... yes
checking for struct director.admin_health... no
checking for req_body_iter_f... no
checking for objiterate_f... yes
checking for enum exp_event_e... no
checking for struct tcp_info... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
 /bin/bash ./config.status
config.status: creating Makefile
config.status: creating src/Makefile
config.status: creating config.h
config.status: config.h is unchanged
config.status: executing depfiles commands
config.status: executing libtool commands
(CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /root/varnish-modules-0.15.0/build-aux/missing autoheader)
rm -f stamp-h1
touch config.h.in
cd . && /bin/bash ./config.status config.h
config.status: creating config.h
config.status: config.h is unchanged
make  all-recursive
make[1]: Entering directory '/root/varnish-modules-0.15.0'
Making all in src
make[2]: Entering directory '/root/varnish-modules-0.15.0/src'
  VMODTOOL vcc_bodyaccess_if.c

NOTICE: Please put $Module description in quotes.

  CC       vmod_bodyaccess.lo
In file included from /usr/include/varnish/cache/cache_varnishd.h:32:0,
                 from vmod_bodyaccess.c:37:
/usr/include/varnish/cache/cache.h:202:1: error: data definition has no type or storage class [-Werror]
 VRBT_HEAD(vrt_priv_tree,vrt_priv);
 ^~~~~~~~~
/usr/include/varnish/cache/cache.h:202:1: error: type defaults to ‘int’ in declaration of ‘VRBT_HEAD’ [-Werror=implicit-int]
/usr/include/varnish/cache/cache.h:202:1: error: parameter names (without types) in function declaration [-Werror]
/usr/include/varnish/cache/cache.h:207:23: error: field ‘privs’ has incomplete type
  struct vrt_priv_tree privs;
                       ^~~~~
vmod_bodyaccess.c: In function ‘bodyaccess_bcat’:
vmod_bodyaccess.c:149:28: error: passing argument 2 of ‘VRB_Iterate’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  l = VRB_Iterate(ctx->req, bodyaccess_bcat_cb, vsb);
                            ^~~~~~~~~~~~~~~~~~
In file included from vmod_bodyaccess.c:37:0:
/usr/include/varnish/cache/cache_varnishd.h:351:9: note: expected ‘int (*)(void *, unsigned int,  const void *, ssize_t) {aka int (*)(void *, unsigned int,  const void *, long int)}’ but argument is of type ‘int (*)(void *, int,  const void *, ssize_t) {aka int (*)(void *, int,  const void *, long int)}’
 ssize_t VRB_Iterate(struct req *, objiterate_f *func, void *priv);
         ^~~~~~~~~~~
vmod_bodyaccess.c: In function ‘vmod_len_req_body’:
vmod_bodyaccess.c:205:20: error: ‘struct req’ has no member named ‘req_bodybytes’; did you mean ‘req_body_status’?
  return (ctx->req->req_bodybytes);
                    ^~~~~~~~~~~~~
                    req_body_status
vmod_bodyaccess.c: In function ‘vmod_log_req_body’:
vmod_bodyaccess.c:287:30: error: passing argument 2 of ‘VRB_Iterate’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  ret = VRB_Iterate(ctx->req, bodyaccess_log_cb, &log_ctx);
                              ^~~~~~~~~~~~~~~~~
In file included from vmod_bodyaccess.c:37:0:
/usr/include/varnish/cache/cache_varnishd.h:351:9: note: expected ‘int (*)(void *, unsigned int,  const void *, ssize_t) {aka int (*)(void *, unsigned int,  const void *, long int)}’ but argument is of type ‘int (*)(void *, int,  const void *, ssize_t) {aka int (*)(void *, int,  const void *, long int)}’
 ssize_t VRB_Iterate(struct req *, objiterate_f *func, void *priv);
         ^~~~~~~~~~~
vmod_bodyaccess.c: In function ‘vmod_len_req_body’:
vmod_bodyaccess.c:206:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors
Makefile:918: recipe for target 'vmod_bodyaccess.lo' failed
make[2]: *** [vmod_bodyaccess.lo] Error 1
make[2]: Leaving directory '/root/varnish-modules-0.15.0/src'
Makefile:541: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/root/varnish-modules-0.15.0'
Makefile:452: recipe for target 'all' failed
make: *** [all] Error 2

root@vagrant:~/varnish-modules-0.15.0# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 18.04.2 LTS
Release:    18.04
Codename:   bionic

nigoroll commented 5 years ago

@ahes you gave the answer yourself. This repository does not support 6.2, but mine does

marcinhlybin commented 5 years ago

@ahes you gave the answer yourself. This repository does not support 6.2, but mine does

Why it does not support 6.2? This one is official.

nigoroll commented 5 years ago

@ahes what would you call "official" with respect to open source software? My repo is the "official" repo for 6.2. Other than that, I can only refer you to the README

marcinhlybin commented 5 years ago

@nigoroll By official I mean owned by Varnish Software and referenced in official Varnish documentation. I need to add if conditional to my VCL to check content of multiple headers (like Set-Cookie). Header vmod does that. On vmod page I can see a link to this repo (no VCC 6.2 though). That is why I refer to this repository as official.

Anyway, I can see that you are from UPLEX but forked repositories tend to be abandoned, not updated, and not merged. I am automating varnish-modules deb package creation and I would prefer to use "official" varnish github repository.

For some reason packagecloud varnish apt repository does not include varnish-modules package which would come in handy. For example Ubuntu does that with Varnish 5.x.

Did you try to PR your changes?

Maybe adding varnish-modules package would be possible? Are you able to make it happen?

nigoroll commented 5 years ago

I have pushed an update to http://varnish-cache.org/vmods/ to contain a link to my repo for the cookie vmod. The other vmods contained in varnish-modules were not referenced there, so I only updated that one reference.

On your other points:

The varnish-cache project is independent of Varnish-Software
I would personally like to ask you to put aside prejudice like tend to be abandoned, not updated, and not merged and check the facts before making a judgement. I am looking after the "newer versions fork" of varnish-modules for a long time and my efforts to get these changes merged are well documented.
packagecloud repos are maintained by Varnish-Software and it is up to them to decide which packages to include. You might want to consider https://pkg.uplex.de/

marcinhlybin commented 5 years ago

Sounds cool. Regarding prejudice, don't worry about that. That's my opinion based on personal observations. YMMV. I am not pointing you nor your repository in particular.

Take care, thanks for replying.

dridi commented 5 years ago

@ahes I understand your worry regarding a switch to a fork with the risk that it would bit-rot. However @nigoroll has tried to force 6.1 support onto us when it got released and has so far kept a good maintenance of his fork as far as I can tell.

Also with my Varnish Software hat on I should warn you against running 6.2 in production as it is uncertain today how much maintenance it will get once the next major release is out. You can get in contact with UPLEX for that kind of support, I assume.

Otherwise we have our own commercial offering, and in addition we picked 6.0 as the latest LTS release.

Our collection of VMODs is relatively low traffic, we certainly fix bugs when we are made aware of them but new features rarely appear. Maybe new VMODs will land in the future, or go away like vmod-softpurge (we upstreamed a vmod-purge directly available in Varnish instead).

While I personally want this repository to support all 6.x releases, going beyond 6.0 is currently not a priority.

dridi commented 5 years ago

I will redirect future discussions on this topic to #138.

marcinhlybin commented 5 years ago

@Dridi Thank you for clarifying.

Perhaps that is not the best time or place to ask but I am wondering why Varnish does not support handling multiple headers natively. I guess it is a common use case to search for a key pattern in Set-Cookie/Cookie headers.

Currently, below conditional will look only in the first header found which is unexpected behavior (at least for me):

if (beresp.http.Set-Cookie ~ "my_cookie_key=") {
  set beresp.uncachable = true;
}

To handle it correctly one would have to use header vmod:

if (header.get(beresp.http.Set-Cookie, "my_cookie_key=")) {
  set beresp.uncachable = true;
}

While experimenting I came across std.collect(). That is interesting because I would expect that it returns a value but instead it modifies a header in place.

It would be really cool if I could just do:

if (std.collect(beresp.http.Set-Cookie, ";") ~ "my_cookie_key=") {
  set beresp.uncachable = true;
}

or even:

set beresp.http.X-Set-Cookie = std.collect(beresp.http.Set-Cookie, ";");

if (beresp.http.X-Set-Cookie ~ "my_cookie_key=") {
  set beresp.uncachable = true;
}

dridi commented 5 years ago

I guess the crux of it right now is that we don't have a nice syntax in pure VCL to handle multiple headers with the same name, and didn't find a satisfying API to ship such a module with Varnish itself.

And it's a violation of the "spec" [1] to send multiple cookie headers in HTTP/1 and it's either mandatory [2] or recommended for h2. But for h2 we collect the cookies prior to entering vcl_recv. Arguably we could do the same for HTTP/1.

This is rather a topic for the Varnish mailing lists, so don't open a github issue on this topic or it is guaranteed to be closed as invalid.

Also if you want to collect cookies the separator [3,4] is "; " with a single trailing SP. HTTP cookies are a mess, make sure to read the RFC with an empty stomach and don't look too closely at header compression in h2 with cookies in mind.

Finally, it may not look obvious from my previous comment, but @nigoroll's past contributions have always been very helpful and future contributions from anyone are reviewed and considered. So thank you @nigoroll and everyone else ;-)

[1] more like a GCD of observed behavior in the wild [2] I'm sure it's mandatory but I'm not checking the RFC right now [3] set-cookie-string = cookie-pair *( ";" SP cookie-av ) [4] cookie-string = cookie-pair *( ";" SP cookie-pair )

varnish / varnish-modules

Make fails for varnish-6.2 #137