Use the new autoconf macros

[dridi]

Split in individual commits, details (if any) can be found in commit messages for each change. These changes technically also belong in the 4.1 branch since 4.1.4-beta1 includes the new macros too. The question is, do we support older 4.1 releases? If yes we should keep the old build system in the 4.1 branch.

7 files changed, 63 insertions(+), 131 deletions(-)

Summoning: @scn @gquintard @aondio

[aondio]

I've played around a bit with this PR and it looks good to me.

Little request: travis is failing, can you please update the travis.yml which is still trying to build the vmod against 4.1?

[dridi]

@aondio good catch, but I'm afraid it will have to wait for packages before this can be merged:

https://repo.varnish-cache.org/ubuntu/dists/precise/

[dridi]

I forgot to summon interested parties: @fgsch @nigoroll

[nigoroll]

Oh damn, I'm so sorry, @Dridi, I had forgot about your PR and just did the thing, and now I'm fighting with travis. BIG APOLOGIES AGAIN!

[nigoroll]

ok, due to the exeptional fuckup from my side I've force-pushed back to dc4f97b6b2b93c58fe72fddd50b64e6f51180f68 Will now take the opportunity to use my independent changes for a review of your work, @Dridi

[nigoroll]

Re Travis: The last I had was this in .travis.yml:

diff --git a/.travis.yml b/.travis.yml
index c0be8b7..18b0556 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -2,15 +2,13 @@ language: c

 before_install:
   - sudo apt-get update -q
-  - sudo apt-get install -qq apt-transport-https python-docutils
-  - curl https://repo.varnish-cache.org/debian/GPG-key.txt | sudo apt-key add -
-  - echo "deb https://repo.varnish-cache.org/ubuntu/ precise varnish-4.1" | sudo tee /etc/apt/sources.list.d/varnish-cache.list
-  - sudo apt-get -q update
-  - sudo apt-get install varnish libvarnishapi-dev
+  - sudo apt-get install -qq apt-transport-https python-docutils libjemalloc1
+  - wget -q https://repo.varnish-cache.org/pkg/5.0.0/varnish_5.0.0-1_amd64.deb https://repo.varnish-cache.org/pkg/5.0.0/varnish-dev_5.0.0-1_amd64.deb
+  - sudo dpkg -i varnish_5.0.0-1_amd64.deb varnish-dev_5.0.0-1_amd64.deb

 before_script:
-  - ./autogen.sh
+  - ./bootstrap --prefix=/usr
   - ./configure --prefix=/usr
   - make -j4

but still got:

Error:
Message from VCC-compiler:
/usr/sbin/varnishd: symbol lookup error: /usr/lib/varnish/libvarnish.so: undefined symbol: pcre_free_study

This sounds like the we need a separate build for precise

[nigoroll]

• would suggest to switch to bootstrap and improve the script https://github.com/nigoroll/libvmod-example/blob/my_5.0/autogen.sh https://github.com/nigoroll/libvmod-example/blob/my_5.0/bootstrap
• shouldnt we keep m4_ifndef([VARNISH_VMOD_INCLUDES], AC_MSG_ERROR([Need varnish.m4 -- see README.rst])) in configure.ac?
• you've been a bit to eager with .gitignore Other than that, your banch WFM and LGTM - probably much better than mine :)

[dridi]

Regarding Travis, I have a module using the new macros that builds against 5.0.0 so depending on preferences I can install it from a package too.

This sounds like the we need a separate build for precise

I'm not sure which direction we are taking with packaging, but it seems that we mainly ship dist archives and raw deb/rpm packages (not in apt/yum repositories). And debs don't seem to embed the target in their file names.

See https://repo.varnish-cache.org/pkg/5.0.0/

Also I think the new macros don't work on precise, automake is too old (unless of course you install a more recent version). But it builds fine from a dist tarball.

would suggest to switch to bootstrap and improve the script

The same module has a simpler bootstrap script that doesn't check the automake version because it'll be done at configure time. Also it contains no bashisms that I know of (and just in case, $(...) is not a bashism).

Also, I don't see the point of keeping autogen.sh if we update the README accordingly.

shouldnt we keep [...] in configure.ac?

No. The old and new macros don't mix well together, but both work fine on their own. I didn't remove it, I simply kept AC, AM and LT macros together before dealing with Varnish.

See this check before the use of VARNISH macros.

you've been a bit to eager with .gitignore

Are you sure? I thought I had only removed irrelevant patterns. Don't you have leftovers from a previous branch instead?

Thanks for the review!

[nigoroll]

@Dridi, I have re-checked:

• you were right regarding .gitignore :) thx
• I suspect we will receive complaints from users of old autocrap tools, but personally I am fine with modernizing and documenting the requirements
• adding a varnish build to travis seems like a sensible thing to do while we don't have the right binary build available So I'd appreciate a merge sooner than later in order to use this as a basis for other vmods. Thank you!

[dridi]

I made this PR merge-able after @nigoroll's last comment.