Bug 178361

Summary: [Settings] Generate Settings.h/cpp
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, commit-queue, darin, dino, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch dino: review+, commit-queue: commit-queue-

Description Sam Weinig 2017-10-16 15:21:55 PDT
[Settings] Generate Settings.h/cpp
Comment 1 Sam Weinig 2017-10-16 15:26:52 PDT
Created attachment 323948 [details]
Patch
Comment 2 Sam Weinig 2017-10-16 17:14:01 PDT
Created attachment 323960 [details]
Patch
Comment 3 Sam Weinig 2017-10-16 20:11:02 PDT
My initial guess for what's going wrong with the GTK and WPE builds is that the forwarding header for WebCore/Settings.h is not being regenerated for its new location as a derived source. Not sure how to address this yet.
Comment 4 Sam Weinig 2017-10-17 08:35:29 PDT
Created attachment 324016 [details]
Patch
Comment 5 Sam Weinig 2017-10-17 09:12:59 PDT
Created attachment 324018 [details]
Patch
Comment 6 Konstantin Tokarev 2017-10-17 09:28:02 PDT
Comment on attachment 324018 [details]
Patch

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

> Source/WebKit/PlatformGTK.cmake:26
> +file(REMOVE "${FORWARDING_HEADERS_DIR}/WebCore/Settings.h")

Please remove this now

> Source/WebKit/PlatformWPE.cmake:18
> +file(REMOVE "${FORWARDING_HEADERS_DIR}/WebCore/Settings.h")

and this
Comment 7 Sam Weinig 2017-10-17 09:41:25 PDT
(In reply to Konstantin Tokarev from comment #6)
> Comment on attachment 324018 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=324018&action=review
> 
> > Source/WebKit/PlatformGTK.cmake:26
> > +file(REMOVE "${FORWARDING_HEADERS_DIR}/WebCore/Settings.h")
> 
> Please remove this now
> 
> > Source/WebKit/PlatformWPE.cmake:18
> > +file(REMOVE "${FORWARDING_HEADERS_DIR}/WebCore/Settings.h")
> 
> and this

Pretty sure they will be necessary to build without a clean build. What do you propose I replace them with?
Comment 8 Dean Jackson 2017-10-17 13:35:43 PDT
Comment on attachment 324018 [details]
Patch

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

> Source/WebCore/Scripts/GenerateSettings/GenerateSettingsHeaderFile.py:31
> +
> +
> +def generateSettingsHeaderFile(outputDirectory, settings):

Nit: Too/two many blank lines.

> Source/WebCore/Scripts/GenerateSettings/GenerateSettingsHeaderFile.py:43
> +    outputFile.write("#pragma once\n\n")
> +
> +    outputFile.write("#include \"SettingsBase.h\"\n")
> +    outputFile.write("#include \"SettingsMacros.h\"\n")
> +    outputFile.write("#include <wtf/RefCounted.h>\n\n")
> +
> +    outputFile.write("namespace WebCore {\n\n")
> +

All this stuff might work better as a """ string, to avoid multiple write lines. But no biggy.
Comment 9 WebKit Commit Bot 2017-10-17 13:37:04 PDT
Comment on attachment 324018 [details]
Patch

Rejecting attachment 324018 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 324018, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebKit/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.webkit.org/results/4888683
Comment 10 Sam Weinig 2017-10-17 13:45:38 PDT
Committed r223574: <https://trac.webkit.org/changeset/223574>
Comment 11 Radar WebKit Bug Importer 2017-10-17 13:46:30 PDT
<rdar://problem/35037049>