Re: [flashrom] [PATCH] Makefile: fix and simplify test program compilations

[Carl-Daniel Hailfinger]

Am 12.08.2011 21:38 schrieb Stefan Tauner:
> this was totally broken due to the time behavior of make's shell
> function's temporal behavior.
> quote from the gnu make documentation
> http://www.gnu.org/s/hello/manual/make/Shell-Function.html):
> "The commands run by calls to the shell function are run when the
> function calls are expanded"
> we have used the shell function to echo the test programs to a file.
> the file name used was equal for all tests and was overwritten for
> each test. the result was that all tests (in a single target?) used
> the last test program because the echoing of the test programs was
> done before all test compilations(!)

Thanks for opening that bug report. Apparently the gnu make
documentation was incorrect.
> also the branching for testing ifeq ($(CONFIG_FT2232_SPI), yes) was
> unnecessary.

Branching never worked for me inside a target, but I also never tried it
your way.
> in my approach here i am using verbatim variables (allows to define
> even complex test programs in the makefile without jumping through
> hoops) that get exported to environment variables (via "export",
> reference afterwards with "$$<varname>").
>
> i have also added the missing redirection of stderr to the compiler test.

Good.
> alternatively one could just use different file names of course, but
> i think my approach is superior due to the separation and
> readability of the tested code and the fact that the temporal
> behavior is MUCH clearer now (else this bug would have not crept in
> in the first place :P)
>
> if line count is an issue one could of course compress the test
> programs to something unreadable again :)

Haha.
> side note: we use $(CC) all over the place before we even check if
> it is there. e.g.:
> override ARCH = $(strip $(shell LC_ALL=C $(CC) -E arch.h|grep -v '^#'))
>
> this creates unwanted output like:
> make -j1 CC=eiugthes
> /bin/sh: eiugthes: not found
> /bin/sh: eiugthes: not found
> /bin/sh: eiugthes: not found
> /bin/sh: eiugthes: not found
> Checking for a C compiler... not found.

Hm yes.
> make: *** [compiler] Error 1
> Signed-off-by: Stefan Tauner
>
> Makefile |   91 ++++++++++++++++++++++++++++++++++++++++----------------------
>  1 files changed, 59 insertions(+), 32 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c72d10a..672a0df 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -531,11 +531,62 @@ distclean: clean
>  strip: $(PROGRAM)$(EXEC_SUFFIX)
>    $(STRIP) $(STRIP_ARGS) $(PROGRAM)$(EXEC_SUFFIX)
>
> +# below are the definitions of test programs as verbatim variables, which get
> +# exported to environment variables and are referenced with $$<varname> later
> +define COMPILER_TEST
> +int main(int argc, char **argv)
> +{
> (void) argafc;

Typo?
> +define UTSNAME_TEST
> +#include <sys/utsname.h>
> +struct utsname osinfo;
> +int main(int argc, char **argv)
> +{
> (void) argc;
> (void) argv;
> uname (&osinfo);
> return 0;
>
> +}
> +endef
> +export UTSNAME_TEST

Hm... the alternative would be to have each of those C programs in a
separate file in a config/ directory or somesuch. I don't have a strong
preference either way.

Given that you think verbatim variables are superior, please move them
directly above the targets which use them.
> compiler: featuresavailable
>    @printf "Checking for a C compiler... "
> @$(shell ( echo "int main(int argc, char **argv)"; \
>    echo "{ (void) argc; (void) argv; return 0; }"; ) > .test.c )
> @$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .test.c -o .test$(EXEC_SUFFIX) >/dev/null &&	\
> @echo "$$COMPILER_TEST" > .test.c
> @$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .test.c -o .test$(EXEC_SUFFIX) >/dev/null 2>&1 &&	\
> echo "found." || ( echo "not found."; \
> rm -f .test.c .test$(EXEC_SUFFIX); exit 1)
> @rm -f .test.c .test$(EXEC_SUFFIX)
>
> @@ -548,12 +599,7 @@ compiler: featuresavailable
>  ifeq ($(CHECK_LIBPCI), yes)
>  pciutils: compiler
>    @printf "Checking for libpci headers... "
> @# Avoid a failing test due to libpci header symbol shadowing breakage
> @$(shell ( echo "#define index shadow_workaround_index"; \
>    echo "#include <pci/pci.h>";		   \
>    echo "struct pci_access *pacc;";	   \
>    echo "int main(int argc, char **argv)"; \
>    echo "{ (void) argc; (void) argv; pacc = pci_alloc(); return 0; }"; ) > .test.c )
> @echo "$$LIBPCI_TEST" > .test.c
> @$(CC) -c $(CPPFLAGS) $(CFLAGS) .test.c -o .test.o >/dev/null 2>&1 &&		\
> echo "found." || ( echo "not found."; echo;			\
> echo "Please install libpci headers (package pciutils-devel).";	\
>
> @@ -588,41 +634,22 @@ featuresavailable:
>    @false
>  endif
>
> -ifeq ($(CONFIG_FT2232_SPI), yes)
>  features: compiler
>    @echo "FEATURES := yes" > .features.tmp
> +ifeq ($(CONFIG_FT2232_SPI), yes)

I'm really surprised that ifeq inside a target works. IIRC I tried this
some time ago on my older machine and it did not work. However, if this
works reliably, the code is indeed a lot more readable.

Acked-by: Carl-Daniel Hailfinger

Even if we ever decide to use another build system, we need this fix now.

Regards,
Carl-Daniel