Am 28.03.2010 23:13, schrieb Stefan Reinauer: > While I think this patch is great and we definitely need it, there are > some things I'd like to discuss and improve, or back out if possible... That part of the change wasn't one of my brightest moments. Your patch (build.h handling, take 2) is much better.
> On 3/27/10 6:18 PM, repository service wrote: >> -all: $(obj)/config.h coreboot >> +all: $(obj)/config.h $(obj)/build.h coreboot >> endif >> > This will make $(obj)/build.h and coreboot in parallel I think... is > that intended? No. >> OBJS := $(patsubst %,$(obj)/%,$(TARGETS-y)) >> -INCLUDES := -I$(top)/src -I$(top)/src/include -I$(obj) >> -I$(top)/src/arch/$(ARCHDIR-y)/include >> -INCLUDES += -I$(top)/src/devices/oprom/include >> -INCLUDES += -include $(obj)/config.h >> +INCLUDES := -Isrc -Isrc/include -I$(obj) -Isrc/arch/$(ARCHDIR-y)/include >> +INCLUDES += -Isrc/devices/oprom/include >> +# abspath is a workaround for romcc >> +INCLUDES += -include $(abspath $(obj)/config.h) -include $(abspath >> $(obj)/build.h) > Why is build.h added to INCLUDES? This basically renders the dependency > system unusable because all of the C files are depending on build.h > (which they are not) > Only two files should depend on build.h and that's console.o and > romstage.inc The | $(obj)/build.h sort-of fixed this: dependencies after "|" are only used for maintaining build order, not for dependencies. That is, make ensures the file is there, but doesn't care if it's older or newer than the target file. But I think it messes up the time stamp in our scenario. Of course, the .d files messed up this band-aid again. >> @@ -205,7 +203,7 @@ >> >> $(obj)/mainboard/$(MAINBOARDDIR)/romstage.pre.inc: >> $(src)/mainboard/$(MAINBOARDDIR)/romstage.c $(OPTION_TABLE_H) $(obj)/build.h >> printf " CC romstage.inc\n" >> - $(CC) -MMD $(CFLAGS) -include $(obj)/build.h -I$(src) -I. -c -S $< -o $@ >> + $(CC) -MMD $(CFLAGS) -I$(src) -I. -c -S $< -o $@ >> > > Why is build.h dropped here? As it's in CFLAGS (see above) >> Modified: trunk/src/arch/i386/lib/Makefile.inc >> ============================================================================== >> --- trunk/src/arch/i386/lib/Makefile.inc Fri Mar 26 19:31:12 2010 >> (r5303) >> +++ trunk/src/arch/i386/lib/Makefile.inc Sat Mar 27 18:18:39 2010 >> (r5304) >> @@ -8,8 +8,3 @@ >> >> initobj-y += printk_init.o >> initobj-y += cbfs_and_run.o >> - >> -ifdef POST_EVALUATION >> -$(obj)/arch/i386/lib/console.o :: $(obj)/build.h >> -endif >> - >> > I don't think it's OK to drop this dependency, as it needs to be known > before gcc is run on the binary. It's drawn in by the global build.h dependency (see above) I'm sorry about that. Patrick -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot