[Webkit-unassigned] [Bug 237329] [XCBuild] Emit a discovered dependency file from offlineasm

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=237329

--- Comment #12 from Elliott Williams <emw at apple.com> ---
(In reply to Keith Miller from comment #10)
> Comment on attachment 453560 [details]
> Integrate offlineasm using build rules and emit a depfile r5
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=453560&action=review
> 
> > Source/JavaScriptCore/offlineasm/parser.rb:846
> >                  raise "File not found: #{fileName}" if not fileExists and not isOptional
> 
> Do we want to add this file to the `.deps` if it doesn't exist? Does that
> work? What happens if someone adds that file later? e.g. someone adds the
> `Internal` directory after doing an OS build.

This is a good idea! AFAICT, having missing discovered dependencies does not affect incremental builds (i.e. you can still null build) but does trigger a rerun when they become available, which seem like the exact semantics we'd want.


(In reply to Alexey Proskuryakov from comment #11)
> Comment on attachment 453560 [details]
> Integrate offlineasm using build rules and emit a depfile r5
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=453560&action=review
> 
> > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:2166
> > +			dependencyFile = "$(DERIVED_FILES_DIR)/$(CURRENT_ARCH)/$(INPUT_FILE_BASE).d";
> 
> For unrelated reasons, DerivedSources unfortunately end up in build
> artifacts. Can these .d files be somewhere else where they don't show up
> unnecessarily?

This is a different directory from the DerivedSources in build products that many of our script write to. On my machine, DERIVED_FILES_DIR is WebKitBuild/JavaScriptCore.build/Debug/JavaScriptCore.build/DerivedSources. This directory shouldn't show up in build artifacts -- it contains a bunch of machine-specific intermediate files.

> On my machine, this file is in WebKitBuild/Debug/DerivedSources/JavaScriptCore/LLIntAssembly.h. Is this basically a drive-by fix to move it to $(CURRENT_ARCH)? But I don't see where it's changed for the legacy build system.

This patch runs offlineasm once per architecture, which is why I had it generate separate LLIntAssembly.h's (otherwise, XCBuild will fail because multiple tasks are creating the same outputs). But it seems like the legacy build system does not do this, so I think I misunderstood how offlineasm handles multi-arch builds.

As we discussed IRL, I need to go back and make sure both build systems write to this path, so that the file reference in the project is reliable.

> > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:2175
> > +				"$(DERIVED_FILES_DIR)/$(CURRENT_ARCH)/LLIntAssembly.h",
> 
> On my machine, this file is in
> WebKitBuild/Debug/DerivedSources/JavaScriptCore/LLIntAssembly.h. Is this
> basically a drive-by fix to move it to $(CURRENT_ARCH)? But I don't see
> where it's changed for the legacy build system.
> 
> > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:2183
> > +			filePatterns = "*.asm";
> 
> There are three rules with the same filePatterns value. I'm curious how this
> works. Which rules process which inputs?

Build rules are per-target. In the order they appear in the pbxproj, they correspond with JavaScriptCore, JSCLLIntOffsetsExtractor, JSCLLIntSettingsExtractor respectively.

> > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:12004
> > +			shellScript = "[ \"${WK_USE_NEW_BUILD_SYSTEM}\" = \"YES\" ] && exit 0\n\nif [[ \"${ACTION}\" == \"installhdrs\" ]]; then\n    exit 0\nfi\n\nOFFLINEASM_ARGS=\"\"\nif [[ \"${DEPLOYMENT_LOCATION}\" == \"YES\" ]]; then\n    OFFLINEASM_ARGS=\"${OFFLINEASM_ARGS} --webkit-additions-path=${WK_WEBKITADDITIONS_HEADERS_FOLDER_PATH}\"\nfi\n\ncd \"${BUILT_PRODUCTS_DIR}/DerivedSources/JavaScriptCore\"\n\n/usr/bin/env ruby JavaScriptCore/offlineasm/asm.rb \"-I${BUILT_PRODUCTS_DIR}/DerivedSources/JavaScriptCore\" JavaScriptCore/llint/LowLevelInterpreter.asm \"${BUILT_PRODUCTS_DIR}/JSCLLIntOffsetsExtractor\" LLIntAssembly.h \"${BUILD_VARIANTS}\" ${OFFLINEASM_ARGS} || exit 1\n";
> 
> It seems like the check for installhdrs is not there for the modern build
> system. Is that right and OK?

Yes! These rules are now invoked as part of the "Compile Sources" phase, which is build and install-only, so we get the desired behavior without needing a check.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220303/8f0f9097/attachment-0001.htm>