varnish / libvmod-example

An example vmod for Varnish Cache
https://varnish-cache.org/vmods/
53 stars 74 forks source link

Major cleanup of the build system #30

Closed dridi closed 8 years ago

dridi commented 8 years ago

Following the commit c7087eb that broke the distcheck it was supposed to fix, many changes:

Fixes #25 #26 #27

daghf commented 8 years ago

@lkarsten please take a look at this

dridi commented 8 years ago

@gquintard aka autotools authority: ping

gquintard commented 8 years ago

If I'm not mistaken, "-Wall -Werror -Wextra" is wrong for CPPFLAGS.

$(VCC_FILES) $(DOC_FILES): vmod_example.vcc is a bit wrong, that recipe may be executed multiples times, but as long as the files are created atomically, we're good. Using vmod_example.vcc instead of vcc adds one extra occurrence of the name though.

Did you check that "make check" builds the vmod before testing it?

dridi commented 8 years ago

If I'm not mistaken, "-Wall -Werror -Wextra" is wrong for CPPFLAGS.

Where shoud I put them?

$(VCC_FILES) $(DOC_FILES): vmod_example.vcc is a bit wrong, that recipe may be executed multiples times, but as long as the files are created atomically, we're good.

It can happen with parallel builds, but how can I express that a single source will build several target otherwise?

That being said, I've tried to break this numerous times but the files seem to be indeed created atomically, I couldn't fail a parallel build despite this situation.

Using vmod_example.vcc instead of vcc adds one extra occurrence of the name though.

I name it vmod.vcc for my own VMODs, but I wanted to focus on the build for this PR so I didn't touch this part.

Did you check that "make check" builds the vmod before testing it?

I did, I have a bunch of tests to make sure I didn't break anything else, and this is one of them.

gquintard commented 8 years ago

Where shoud I put them?

*_CFLAGS, IMHO.

It can happen with parallel builds, but how can I express that a single source will build several target otherwise?

Usually, you'd use a pattern like: file1 file2 file3: file4 file4: generator.py

Kind of like what was done with the vcc target

But, it really doesn't matter here is parallel build can't break the build.

I name it vmod.vcc for my own VMODs, but I wanted to focus on the build for this PR so I didn't touch this part.

My bad, I misread that part!

dridi commented 8 years ago

Usually, you'd use a pattern like: file1 file2 file3: file4 file4: generator.py

Very hacky workaround :)

Kind of like what was done with the vcc target

The vcc target was my mistake, it leads to #27 because the target wouldn't map to a file, be it phony or not.

dridi commented 8 years ago

This seems to do the job without breaking the DRY attempt:

-$(VCC_FILES) $(DOC_FILES): vmod_example.vcc
+$(VCC_FILES) $(DOC_FILES): .vcc
+
+.vcc: vmod_example.vcc
        @echo vmodtool: @VMODTOOL@ $<
        @@VMODTOOL@ $<
+       @touch $@
dridi commented 8 years ago

If I'm not mistaken, "-Wall -Werror -Wextra" is wrong for CPPFLAGS.

Also forgot to mention that I only introduced the -Wextra flag to tighten the build a bit further. Looking at the automake manual it seems to belong in configure.ac in AM_INIT_AUTOMAKE.

I'll do more testing and push commits.

dridi commented 8 years ago

@gquintard: ping (see the new batch of commits above)

gquintard commented 8 years ago

Looking good.

the --with-rstman=true is a bit weird, but hey, it's autotools!

I think bmake doesn't support the $< variable. But autotoosl are a GNU build system, so, I wouldn't worry about that either.

instead of systematically adding '@' in front of your command, you can use "$(AM_V_lt)", that will allow to show the command with make V=1

dridi commented 8 years ago

Rebased against current master because of conflicts with #31