[coreboot] [commit] r5304 - in trunk: . src/arch/i386 src/arch/i386/lib src/cpu/x86/smm util/abuild util/cbfstool

[Patrick Georgi]

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