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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
