Bug 197151 - WHLSLPrepare.cpp always recompiles, even if nothing was changed
Summary: WHLSLPrepare.cpp always recompiles, even if nothing was changed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on: 197159
Blocks:
  Show dependency treegraph
 
Reported: 2019-04-21 17:16 PDT by Darin Adler
Modified: 2019-05-13 07:00 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.33 KB, patch)
2019-04-21 17:17 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (4.34 KB, patch)
2019-04-21 17:20 PDT, Darin Adler
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2019-04-21 17:16:06 PDT
WHLSLPrepare.cpp always recompiles, even if nothing was changed
Comment 1 Darin Adler 2019-04-21 17:17:59 PDT Comment hidden (obsolete)
Comment 2 Darin Adler 2019-04-21 17:20:19 PDT
Created attachment 367921 [details]
Patch
Comment 3 Darin Adler 2019-04-21 17:23:23 PDT
One small thing I don’t understand is why generate-xcfilelists did not remove the reference to WHLSLStandardLibrary.cpp even after I removed all other references to that filename from the source tree.
Comment 4 Darin Adler 2019-04-21 17:24:07 PDT
Committed r244495: <https://trac.webkit.org/changeset/244495>
Comment 5 Radar WebKit Bug Importer 2019-04-21 17:25:24 PDT
<rdar://problem/50083499>
Comment 6 Shawn Roberts 2019-04-22 08:41:49 PDT
Changes in https://trac.webkit.org/changeset/244495 are causing build failures on Mac and iOS Release and Debug queues.

https://build.webkit.org/builders/Apple%20Mojave%20Release%20%28Build%29/builds/5103/steps/compile-webkit/logs/stdio

perl JavaScriptCorePrivateHeaders/xxd.pl XMLViewer_js ./XMLViewer.min.js XMLViewerJS.h
Can't modify anonymous list ([]) in scalar assignment at JavaScriptCorePrivateHeaders/xxd.pl line 3, near "};"
Execution of JavaScriptCorePrivateHeaders/xxd.pl aborted due to compilation errors.
make: *** [XMLViewerJS.h] Error 255
perl JavaScriptCorePrivateHeaders/xxd.pl CommandLineAPIModuleSource_js ./CommandLineAPIModuleSource.min.js CommandLineAPIModuleSource.h
Can't modify anonymous list ([]) in scalar assignment at JavaScriptCorePrivateHeaders/xxd.pl line 3, near "};"
Execution of JavaScriptCorePrivateHeaders/xxd.pl aborted due to compilation errors.
make: *** [CommandLineAPIModuleSource.h] Error 255
Comment 7 WebKit Commit Bot 2019-04-22 08:47:11 PDT
Re-opened since this is blocked by bug 197159
Comment 8 Darin Adler 2019-04-22 12:34:11 PDT
I have no idea how this change caused those problems. Does anyone know?
Comment 9 Darin Adler 2019-04-22 13:56:53 PDT
Myles, at some point when you are free after doing higher priority work, would you be willing to help me with this?
Comment 10 Darin Adler 2019-04-23 09:20:15 PDT
I am still getting this error in xxd.pl in my build here on my Mac even though my source tree does not have the changes from this patch; no time to investigate and fix it right now. Are we sure about cause/effect here?
Comment 11 Darin Adler 2019-04-23 09:20:57 PDT
Oh wait, maybe I do have the changes; I’ll be able to debug this locally now. No worries, no rush.
Comment 12 Darin Adler 2019-05-05 14:57:17 PDT
I can no longer reproduce this error in xxd.pl so I don’t know how to make progress on this. Can someone help me reproduce the problem? I’d love to fix this makefile mistake because it’s really annoying to do extra compiling every time I build!
Comment 13 Darin Adler 2019-05-12 10:41:49 PDT
This bug is really annoying me — I’d love to land the change some day, but I need help reproducing the build failure that meant Shawn had to roll it out last time. Especially mysterious given the changes don’t seem to be ones that could affect those other invocations of xxd.pl
Comment 14 Keith Rollin 2019-05-12 15:19:48 PDT
(In reply to Darin Adler from comment #3)
> One small thing I don’t understand is why generate-xcfilelists did not
> remove the reference to WHLSLStandardLibrary.cpp even after I removed all
> other references to that filename from the source tree.

xcfilelists need to represent all platforms and configurations, and must be a super-set of the files used for all those build combinations. However, when updating the xcfilelists during build time, we can only update the xcfilelists with respect to that current build. This means that the xcfilelist contents generated during the build might be a subset of what's required for all build combinations. Since it's a subset, there might be files in the resulting xcfilelist that we could characterize as "left over". And there's no way to know if those files are obsolete and can be removed, or if they are needed by a different build combination. Therefore, during the incremental xcfilelist updates, we can't safely delete any files.

There's some discussion of this at <https://confluence.sd.apple.com/pages/viewpage.action?pageId=783873680> under Known Issues.
Comment 15 Keith Rollin 2019-05-12 20:50:53 PDT
Comment on attachment 367921 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367921&action=review

> Source/WebCore/DerivedSources.make:1639
> +	$(PERL) $(JavaScriptCore_SCRIPTS_DIR)/xxd.pl WHLSLStandardLibrary $(WebCore)/Modules/webgpu/WHLSL/WHLSLStandardLibrary.txt $<

Is this an equivalent replacement? You're replacing "WHLSLStandardLibrary.h" with "$<". But $< is the first pre-requisite, which is the xxd.pl tool. I think you want $@ instead of $<.
Comment 16 Darin Adler 2019-05-13 06:57:14 PDT
Comment on attachment 367921 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367921&action=review

>> Source/WebCore/DerivedSources.make:1639
>> +	$(PERL) $(JavaScriptCore_SCRIPTS_DIR)/xxd.pl WHLSLStandardLibrary $(WebCore)/Modules/webgpu/WHLSL/WHLSLStandardLibrary.txt $<
> 
> Is this an equivalent replacement? You're replacing "WHLSLStandardLibrary.h" with "$<". But $< is the first pre-requisite, which is the xxd.pl tool. I think you want $@ instead of $<.

Yes, I want $@, that’s the bug. I was overwriting the tool!
Comment 17 Darin Adler 2019-05-13 06:57:40 PDT
Thank you Keith!
Comment 18 Darin Adler 2019-05-13 07:00:04 PDT
Committed r245233: <https://trac.webkit.org/changeset/245233>