Re: [PATCH] coverage: filter out lib{fdt,elf}-temp.o

[Michal Orzel <michal.orzel@xxxxxxx>]

On 22/01/2024 12:05, Anthony PERARD wrote:
> 
> 
> On Mon, Jan 22, 2024 at 10:54:13AM +0000, Anthony PERARD wrote:
>> On Mon, Jan 22, 2024 at 11:04:41AM +0100, Jan Beulich wrote:
>>> On 19.01.2024 16:25, Anthony PERARD wrote:
>>>> On Fri, Jan 19, 2024 at 09:43:30AM +0100, Michal Orzel wrote:
>>>>> Is my understanding correct that by switching from extra-y to targets we 
>>>>> are preventing these objects to
>>>>> appear in non-init-objects (and thus having COV_FLAGS appended) while 
>>>>> retaining the proper if_changed behavior?
>>>>>
>>>>> According to docs/misc/xen-makefiles/makefiles.rst:
>>>>> Any target that utilises if_changed must be listed in $(targets),
>>>>> otherwise the command line check will fail, and the target will
>>>>> always be built.
>>>>
>>>> Indeed, and $(extra-y) is added to $(targets) via
>>>> $(targets-for-builtin).
>>>>
>>>> While switching from $(extra-y) to $(targets) prevents the objects from
>>>> been added to $(non-init-objets), it doesn't matter because "libelf.o"
>>>> is in that variable, so $(COV_FLAGS) is added to $(_c_flags) and its
>>>> value is used in all the prerequisites of "libelf.o" which includes
>>>> "libelf-temp.o" and for example "libelf-dominfo.o". So the only thing
>>>> preventing $(COV_FLAGS) from been added when building "libelf-tools.o"
>>>> for example is that we set `COV_FLAGS:=` for "libelf.o".
>>>
>>> Yet doesn't that (again) mean things should actually work as-is, [...]
>>
>> No, because I've explain how it should work, in the hypothetical world
>> where we have `targets += libelf-temp.o $(libelf-objs)`.
> 
> The problem is that there's currently two "paths" to build libelf-temp.o
> (and even three I think for libelf-tools.o libelf-loader.o
> libelf-dominfo.o):
> 
> Simplified makefile:
> 
>     obj-y := libelf.o
>     extra-y := libelf-temp.o
>     COV_FLAGS := -fcoverage
> 
>     __build: $(extra-y) built_in.o
>     built_in.o: $(obj-y)
>     libelf.o: COV_FLAGS :=
>     libelf.o: libelf-temp.o
> 
> So, make can build "libelf-temp.o" as prerequisite of "__build" the
> default target, or as prerequisite of "libelf.o".
> In the first case, COV_FLAGS would have all the flags, and in the second
> case, COV_FLAGS would be reset, but that second case is too late as make
> is more likely to have already built libelf-temp.o with all the flags.

Thanks for detailed explanation. I will follow your rationale in v2.

~Michal