|Summary:||Lessen the reliance on VPATH in WebCore/DerivedSources.make|
|Product:||WebKit||Reporter:||Keith Rollin <krollin>|
|Component:||Tools / Tests||Assignee:||Keith Rollin <krollin>|
|Severity:||Normal||CC:||ap, darin, sam, webkit-bug-importer|
|Version:||WebKit Nightly Build|
Description Keith Rollin 2020-10-13 22:36:49 PDT
Due to the way vpath/VPATH works in `make`, we can get into a situation where necessarily IDL files are not copied to their correct locations. WebKit comes with many IDL files. When building for internal purposes, Apple can add to or modify this set of IDL files. When creating Engineering builds, these additional IDL files are copied to .../WebKitBuild/<configuration>/usr/local/includes. When performing Production builds, these IDL files are included in the SDK. This arrangement can be seen in DerivedSources.make: vpath %.idl $(BUILT_PRODUCTS_DIR)/usr/local/include $(SDKROOT)/usr/local/include In order to get these IDL files into a location where they can be uniformly found and handled, there is a step in DerivedSources.make that copies them to the current directory, which is WebKitBuild/<configuration>/DerivedSources/WebCore: $(ADDITIONAL_BINDING_IDLS) : % : WebKitAdditions/% cp $< . The IDL files specified in ADDITIONAL_BINDING_IDLS are specified as bare files names, meaning that vpath and VPATH are employed to find them. Given the vpath specification previously shown, if the IDL files can be found in $(BUILT_PRODUCTS_DIR)/usr/local/include, they are copied locally. Otherwise, they should be found in $(SDKROOT)/usr/local/include and copied locally. As it turns out, vpath/VPATH resolution is performed not only on the prerequisites of the build rule, but also the target. This means that the files specified on the left side of the `cp` rule above are also searched for. And since the standard IDL files (the ones that are being replaced with the Apple-specific versions) can be found on VPATH as it's defined in DerivedSources.make (specifically, in $(WebCore)/dom), then those files participate in determining if the `cp` rule is executed. Consider the normal build case: repositories are checked out (applying the current modification timestamp to the files). During the build, the files in $(ADDITIONAL_BINDING_IDLS) are copied to either BUILT_PRODUCTS_DIR or to SDKROOT, which again modifies their timestamps. We eventually get to the build rule for the `cp` operation. Because the files in WebKitAdditions (be it in the local build directory or the SDK) are *newer* than the checked-out ones in $(WebCore)/dom due to their having been copied after they were checked-out, then the `cp` command is invoked and then files are copied to the current build directory. However, consider a slightly different build case. In this case, projects are checked out and built one at a time. So the project producing the files in ADDITIONAL_BINDING_IDLS is checked out and then the IDL files are copied to their intermediate location and have their timestamps set to that time. Later, WebCore is checked out, and DerivedSources.make is eventually invoked. Now, the files in $(WebCore)/dom are *newer* than those in WebKitAdditions. `make` determines that the IDL files are up-to-date and does not execute the `cp` commands. The IDL files are not copied, and the build then either fails (because of missing files) or proceeds incorrectly (because the wrong IDL files are found). The solution to this issue is to lessen the reliance on vpath/VPATH in DerivedSources.make. Instead, be more explicit about where files can be found and trim down vpath/VPATH to it's bare minimum. With vpath/VPATH reduced, we remove the issue of files being accidentally discovered that we don't want discovered.
Comment 3 Alexey Proskuryakov 2020-10-14 09:56:27 PDT
Sam, I think that this fixes the root cause of troubles in bug 216831.
Comment 4 Sam Weinig 2020-10-14 10:03:01 PDT
(In reply to Alexey Proskuryakov from comment #3) > Sam, I think that this fixes the root cause of troubles in bug 216831. Awesome.
Comment 5 Darin Adler 2020-10-15 14:04:54 PDT
Comment 6 Keith Rollin 2020-10-15 14:15:22 PDT