[edk2-devel] [PATCH 0/4] Arm, ArmPlatform, Crypto, Embedded: list internal headers in [Sources]
Laszlo Ersek
lersek at redhat.com
Tue Jul 23 17:23:16 UTC 2019
On 07/23/19 15:25, Leif Lindholm wrote:
> On Tue, Jul 23, 2019 at 01:02:42PM +0000, Gao, Liming wrote:
>>>>> I am just not pleased with the issue
>>>>> bringing this to the fore is caused by the new caching feature using a
>>>>> different mechanism for tracking header file dependencies than the
>>>>> primary build process.
>>>>
>>>> Ugh... that's a lot of statements compressed into a single sentence. Can
>>>> you please break it down for me? (Yes, I remember the mailing list
>>>> reference you posted earlier, that discussion was too divergent for me.)
>>>
>>> The inclusion of .h files in .inf is not necessary for determining
>>> build-time dependencies on the Makefile level.
>>
>> Right. In fact, build tool will parse source file to find their dependent header files,
>> and list those header files as the dependency in Makefile.
>
> If Visual Studio does not have functionality similar to the GCC
> family's -M flag, then yeah, I can see why the tool needs to take care
> of this itself. Although it would be good if we could find a way to
> not fully maintain our own code for this.
>
>>> Thus, the warnings come out of a different and unrelated level of the
>>> build system, related to the recent build cache features. Which means
>>> we're checking header file build dependencies through two different
>>> mechanisms at two different points of the build.
>>
>> This is related to build feature --hash. When --hash option is enabled, build will calculate
>> the hash value of source files specified in INF file. If hash value is not changed, build tool
>> will not parse source files, and not regenerate Makefile again. So, --hash option
>> is the incremental way in the build parse phase. If some header files are not
>> specified in INF file, it will cause hash value inaccurate.
>
> Right - so we actually have two levels of dependency scanning in the
> build tool itself? One for the .inf file contents, and one for the
> source file contents.
>
>> Then, Makefile may not
>> be generated to match the real source code, and cause the incremental build failure.
>> So, the missing header files in INF file may bring the incremental build issue when --hash option.
>> If you don't enable --hash option, there is no problem even if the missing header files exist.
>
> Right, but we still see the warning messages without using --hash.
>
> Thank you very much for the summary - it clarifies the situation greatly.
+1
> Would it be feasible to update the --hash functionality to make use of
> the include dependencies extracted from the source files? (Clearly, we
> know when the source files change, so we would also know when we would
> need to re-run the dependency search.)
>
> If not, I think we should make the explicit listing of .h files
> in .inf mandatory, triggering a build failure when not the case.
I believe I made the exact same request / suggestion, when Christian
initially posted the patches. The counter-argument was (back then
anyway) that this would break a plethora of platforms. So the idea was
to annoy people with warnings just enough to clean up those INF files,
and then turn the warnings into errors.
If I remember correctly, at least.
Given that the (partly generated) OpenSSL INF files still produce
warnings, I assume that this cleanup is likely incomplete still, in
other (out of tree) platforms as well.
> If it is, then I think we should make it explicitly banned to list .h
> files in .inf. (If there is no other dependency, such as doxygen, also
> making use of .inf listings of .h files.)
This looks a bit too recursive for me:
- rely on hashes saved from a previous run to avoid parsing the #include
directives,
- use extracted #include dependencies to determine what files to hash.
The possibility of a stale cache concerns me. (*Incremental* dependency
extraction with gcc's -MMD concerns me the same, BTW.)
> And this may well be a topic requiring much longer discussion. While
> undecided, I would definitely prefer if we could disable the warning
> when not actually building with --hash.
That makes sense.
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44266): https://edk2.groups.io/g/devel/message/44266
Mute This Topic: https://groups.io/mt/32529014/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list