Bug 207155

Summary: Centralize Cocoa ENABLES (Part 1): Move trivial definitions from FeatureDefines.xcconfig to PlatformEnableCocoa.h
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: ap, benjamin, cdumez, cmarcelo, commit-queue, darin, dbates, ews-watchlist, keith_miller, mark.lam, msaboff, saam, thorton, tsavell, tzagallo, 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
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Sam Weinig 2020-02-03 15:16:52 PST 
Move trivial definitions from FeatureDefines.xcconfig to PlatformEnableCocoa.h
Comment 1 Sam Weinig 2020-02-03 16:22:00 PST Comment hidden (obsolete)
Comment 2 Sam Weinig 2020-02-03 20:20:34 PST Comment hidden (obsolete)
Comment 3 Sam Weinig 2020-02-03 20:53:50 PST Comment hidden (obsolete)
Comment 4 Sam Weinig 2020-02-04 20:26:54 PST Comment hidden (obsolete)
Comment 5 Tim Horton 2020-02-05 13:50:13 PST 
Comment on attachment 389763 [details]
Patch

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

> Source/WebCore/DerivedSources.make:1506
>      HTML_FLAGS := $(HTML_FLAGS) ENABLE_VIDEO_TRACK=0

Confused, why is this a 0
Comment 6 Sam Weinig 2020-02-05 13:55:06 PST 
(In reply to Tim Horton from comment #5)
> Comment on attachment 389763 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389763&action=review
> 
> > Source/WebCore/DerivedSources.make:1506
> >      HTML_FLAGS := $(HTML_FLAGS) ENABLE_VIDEO_TRACK=0
> 
> Confused, why is this a 0

Wasn't it already 0? It does seem wrong though.
Comment 7 Tim Horton 2020-02-05 13:57:17 PST 
Twas, but it's very confusing
Comment 8 Sam Weinig 2020-02-05 14:49:31 PST Comment hidden (obsolete)
Comment 9 Sam Weinig 2020-02-05 17:09:25 PST Comment hidden (obsolete)
Comment 10 Sam Weinig 2020-02-06 07:00:30 PST Comment hidden (obsolete)
Comment 11 Sam Weinig 2020-02-06 16:15:55 PST Comment hidden (obsolete)
Comment 12 Sam Weinig 2020-02-06 17:00:22 PST Comment hidden (obsolete)
Comment 13 Sam Weinig 2020-02-06 22:04:01 PST Comment hidden (obsolete)
Comment 14 Sam Weinig 2020-02-07 08:31:44 PST 
Created attachment 390085 [details]
Patch
Comment 15 WebKit Commit Bot 2020-02-07 12:22:20 PST 
Comment on attachment 390085 [details]
Patch

Rejecting attachment 390085 [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', 'apply-attachment', '--no-update', '--non-interactive', 390085, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
it/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Tim Horton']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 25 diffs from patch file(s).
patching file Source/JavaScriptCore/ChangeLog
patching file Source/JavaScriptCore/DerivedSources.make
patching file Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig
Hunk #2 succeeded at 61 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 71 (offset 6 lines).
Hunk #4 succeeded at 83 (offset 6 lines).
Hunk #5 succeeded at 125 (offset 6 lines).
Hunk #6 succeeded at 152 (offset 6 lines).
Hunk #7 succeeded at 184 (offset 6 lines).
Hunk #8 succeeded at 208 (offset 6 lines).
Hunk #9 succeeded at 253 (offset 6 lines).
Hunk #10 succeeded at 268 (offset 6 lines).
Hunk #11 FAILED at 290.
1 out of 11 hunks FAILED -- saving rejects to file Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig.rej
patching file Source/WTF/ChangeLog
patching file Source/WTF/wtf/PlatformEnable.h
patching file Source/WTF/wtf/PlatformEnableCocoa.h
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/DerivedSources.make
patching file Source/WebCore/Configurations/FeatureDefines.xcconfig
Hunk #2 succeeded at 61 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 71 (offset 6 lines).
Hunk #4 succeeded at 83 (offset 6 lines).
Hunk #5 succeeded at 125 (offset 6 lines).
Hunk #6 succeeded at 152 (offset 6 lines).
Hunk #7 succeeded at 184 (offset 6 lines).
Hunk #8 succeeded at 208 (offset 6 lines).
Hunk #9 succeeded at 253 (offset 6 lines).
Hunk #10 succeeded at 268 (offset 6 lines).
Hunk #11 FAILED at 290.
1 out of 11 hunks FAILED -- saving rejects to file Source/WebCore/Configurations/FeatureDefines.xcconfig.rej
patching file Source/WebCore/Configurations/WebCore.xcconfig
patching file Source/WebCore/PAL/ChangeLog
patching file Source/WebCore/PAL/Configurations/FeatureDefines.xcconfig
Hunk #2 succeeded at 61 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 71 (offset 6 lines).
Hunk #4 succeeded at 83 (offset 6 lines).
Hunk #5 succeeded at 125 (offset 6 lines).
Hunk #6 succeeded at 152 (offset 6 lines).
Hunk #7 succeeded at 184 (offset 6 lines).
Hunk #8 succeeded at 208 (offset 6 lines).
Hunk #9 succeeded at 253 (offset 6 lines).
Hunk #10 succeeded at 268 (offset 6 lines).
Hunk #11 FAILED at 290.
1 out of 11 hunks FAILED -- saving rejects to file Source/WebCore/PAL/Configurations/FeatureDefines.xcconfig.rej
patching file Source/WebCore/PAL/Configurations/PAL.xcconfig
patching file Source/WebCore/Scripts/generate-unified-sources.sh
patching file Source/WebKit/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebKit/DerivedSources.make
patching file Source/WebKit/Configurations/FeatureDefines.xcconfig
Hunk #2 succeeded at 61 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 71 (offset 6 lines).
Hunk #4 succeeded at 83 (offset 6 lines).
Hunk #5 succeeded at 125 (offset 6 lines).
Hunk #6 succeeded at 152 (offset 6 lines).
Hunk #7 succeeded at 184 (offset 6 lines).
Hunk #8 succeeded at 208 (offset 6 lines).
Hunk #9 succeeded at 253 (offset 6 lines).
Hunk #10 succeeded at 268 (offset 6 lines).
Hunk #11 FAILED at 290.
1 out of 11 hunks FAILED -- saving rejects to file Source/WebKit/Configurations/FeatureDefines.xcconfig.rej
patching file Source/WebKit/Scripts/generate-unified-sources.sh
patching file Source/WebKitLegacy/ChangeLog
patching file Source/WebKitLegacy/mac/ChangeLog
patching file Source/WebKitLegacy/mac/MigrateHeaders.make
patching file Source/WebKitLegacy/mac/Configurations/FeatureDefines.xcconfig
Hunk #2 succeeded at 61 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 71 (offset 6 lines).
Hunk #4 succeeded at 83 (offset 6 lines).
Hunk #5 succeeded at 125 (offset 6 lines).
Hunk #6 succeeded at 152 (offset 6 lines).
Hunk #7 succeeded at 184 (offset 6 lines).
Hunk #8 succeeded at 208 (offset 6 lines).
Hunk #9 succeeded at 253 (offset 6 lines).
Hunk #10 succeeded at 268 (offset 6 lines).
Hunk #11 FAILED at 290.
1 out of 11 hunks FAILED -- saving rejects to file Source/WebKitLegacy/mac/Configurations/FeatureDefines.xcconfig.rej
patching file Source/WebKitLegacy/scripts/generate-unified-sources.sh
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig
Hunk #2 succeeded at 61 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 71 (offset 6 lines).
Hunk #4 succeeded at 83 (offset 6 lines).
Hunk #5 succeeded at 125 (offset 6 lines).
Hunk #6 succeeded at 152 (offset 6 lines).
Hunk #7 succeeded at 184 (offset 6 lines).
Hunk #8 succeeded at 208 (offset 6 lines).
Hunk #9 succeeded at 253 (offset 6 lines).
Hunk #10 succeeded at 268 (offset 6 lines).
Hunk #11 FAILED at 290.
1 out of 11 hunks FAILED -- saving rejects to file Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Tim Horton']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/13318684
Comment 16 Sam Weinig 2020-02-07 12:42:03 PST 
Committed r256051: <https://trac.webkit.org/changeset/256051>
Comment 17 Radar WebKit Bug Importer 2020-02-07 12:43:17 PST 
<rdar://problem/59269578>
Comment 18 Ryan Haddad 2020-02-07 14:50:04 PST 
Reverted r256051 for reason:

Broke internal builds.

Committed r256066: <https://trac.webkit.org/changeset/256066>
Comment 19 Sam Weinig 2020-02-07 15:39:23 PST 
(In reply to Ryan Haddad from comment #18)
> Reverted r256051 for reason:
> 
> Broke internal builds.
> 
> Committed r256066: <https://trac.webkit.org/changeset/256066>

Want to give me a hint as to what broke?
Comment 20 Ryan Haddad 2020-02-07 15:56:19 PST 
(In reply to Sam Weinig from comment #19)
> (In reply to Ryan Haddad from comment #18)
> > Reverted r256051 for reason:
> > 
> > Broke internal builds.
> > 
> > Committed r256066: <https://trac.webkit.org/changeset/256066>
> 
> Want to give me a hint as to what broke?
The build log is in the radar.
Comment 21 Sam Weinig 2020-02-08 09:40:54 PST 
Created attachment 390172 [details]
Patch
Comment 22 WebKit Commit Bot 2020-02-08 10:29:44 PST 
Comment on attachment 390172 [details]
Patch

Clearing flags on attachment: 390172

Committed r256091: <https://trac.webkit.org/changeset/256091>
Comment 23 WebKit Commit Bot 2020-02-08 10:29:46 PST 
All reviewed patches have been landed.  Closing bug.
Comment 24 Truitt Savell 2020-02-10 09:28:36 PST 
Reverted r256091 for reason:

Broke internal builds

Committed r256184: <https://trac.webkit.org/changeset/256184>
Comment 25 Alexey Proskuryakov 2020-02-10 17:11:51 PST 
This also makes the build ~5% slower. Need to find a way to avoid that before re-landing.
Comment 26 Sam Weinig 2020-02-10 19:29:10 PST 
(In reply to Alexey Proskuryakov from comment #25)
> This also makes the build ~5% slower. Need to find a way to avoid that
> before re-landing.

5% slower doing what? Clean build? Building which port?
Comment 27 Sam Weinig 2020-02-10 19:47:27 PST 
(In reply to Sam Weinig from comment #26)
> (In reply to Alexey Proskuryakov from comment #25)
> > This also makes the build ~5% slower. Need to find a way to avoid that
> > before re-landing.
> 
> 5% slower doing what? Clean build? Building which port?

I mean whatever it was, yowzers. Obviously needs to be fixed. Can't quite imagine what I did yet though.
Comment 28 Alexey Proskuryakov 2020-02-10 19:52:52 PST 
Internal Mac production build was a where I saw it. Probably same thing with a clean release build.
Comment 29 Tim Horton 2020-02-10 19:54:37 PST 
Regression seems to be all in WebCore (~30%)
Comment 30 Sam Weinig 2020-02-11 10:33:02 PST 
Going to take a different approach, and not move any of the ENABLE_* flags that are used by scripts for now. Eventually, we should also consider fixing the scripts not to rely on ENABLE_* flags at all, but that will take time.
Comment 31 Sam Weinig 2020-02-11 21:48:42 PST 
Created attachment 390492 [details]
Patch
Comment 32 Sam Weinig 2020-02-12 13:19:04 PST 
Created attachment 390550 [details]
Patch
Comment 33 Sam Weinig 2020-02-21 11:18:20 PST 
Created attachment 391413 [details]
Patch
Comment 34 Sam Weinig 2020-02-21 18:14:15 PST 
Created attachment 391439 [details]
Patch
Comment 35 Sam Weinig 2020-02-21 18:45:32 PST 
Created attachment 391441 [details]
Patch
Comment 36 Darin Adler 2020-05-27 18:22:30 PDT 
I’m tackling these problems in bug 212451, bug 212389, bug 212418.
Comment 37 Sam Weinig 2020-09-04 10:25:16 PDT 
Darin completed this.