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 <[email protected]> > --- > 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 <[email protected]> Even if we ever decide to use another build system, we need this fix now. Regards, Carl-Daniel _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
