[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 <stefan.tauner at student.tuwien.ac.at>
> ---
>  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 <c-d.hailfinger.devel.2006 at gmx.net>

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

Regards,
Carl-Daniel