WebKitTestRunner: move code sign configurations from WebKitTestRunner.xcconfig to DebugRelease.xcconfig.
<rdar://problem/56087327>
Created attachment 388456 [details] Patch
Comment on attachment 388456 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388456&action=review > Tools/ChangeLog:3 > + WebKitTestRunner: move code sign configurations from WebKitTestRunner.xcconfig to DebugRelease.xcconfig This is probably fine, but I don't understand why this changes any behavior. These Xcode settings shouldn't matter for production builds AFAIK.
This more closely matches what we've done for JavaScriptCore and WebKit, so even if this doesn't fix Jiewen's problem (and I'm not convinced it will) it does seem more correct than what we were doing previously.
Comment on attachment 388456 [details] Patch Lets wait for a more authoritative answer from EWS, if this is going to break open source, EWS should catch it.
Created attachment 388476 [details] Patch V2
(In reply to Jiewen Tan from comment #6) > Created attachment 388476 [details] > Patch V2 This is an attempt to address those layout test failures.
Created attachment 388483 [details] Patch V3
Created attachment 388486 [details] Patch V4
Comment on attachment 388486 [details] Patch V4 Yes, I think that this is right. May be worth doing a production build locally to be super safe.
(In reply to Alexey Proskuryakov from comment #10) > Comment on attachment 388486 [details] > Patch V4 > > Yes, I think that this is right. May be worth doing a production build > locally to be super safe. Thanks Alexey for r+ this patch.
Comment on attachment 388486 [details] Patch V4 cq+ as mac-wk2 is green.
Comment on attachment 388486 [details] Patch V4 Clearing flags on attachment: 388486 Committed r254955: <https://trac.webkit.org/changeset/254955>
Reverted r254955 for reason: Broke internal builds Committed r254963: <https://trac.webkit.org/changeset/254963>
Created attachment 388596 [details] Patch V5
Created attachment 388597 [details] Patch V5
Comment on attachment 388597 [details] Patch V5 Not a super great way to test this one, so we probably need to actually tests this with production builders. Keith and Jiewen talked about this, and think it will fix our problems. I'm going to r+ so we can test this, Jiewen said he's going to watch the bots to rollout if everything catches on fire.
(In reply to Jonathan Bedard from comment #17) > Comment on attachment 388597 [details] > Patch V5 > > Not a super great way to test this one, so we probably need to actually > tests this with production builders. > > Keith and Jiewen talked about this, and think it will fix our problems. > > I'm going to r+ so we can test this, Jiewen said he's going to watch the > bots to rollout if everything catches on fire. Thank you, Jonathan. Will land it manually to catch things faster.
Committed r255041: <https://trac.webkit.org/changeset/255041>
A speculative test fix: Committed r255111: <https://trac.webkit.org/changeset/255111>
A build fix after r255111: Committed r255188: <https://trac.webkit.org/changeset/255188>
Reopening to attach new patch.
Created attachment 389282 [details] Another speculative test fix
Comment on attachment 389282 [details] Another speculative test fix View in context: https://bugs.webkit.org/attachment.cgi?id=389282&action=review > Source/WebKit/ChangeLog:3 > + Unreviewed, another speculative test fix after r255041 This says "unreviewed", but the patch is marked for review. Looks reasonable, rs=me with one question. > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:359 > + (id)kSecAttrAccessible: (id)kSecAttrAccessibleAfterFirstUnlock, Why not kSecAttrAccessibleAlways? I don't think that there is a practical difference, but "always" is what we want for testing, so it seems better to request that explicitly.
Thanks, Alexey. (In reply to Alexey Proskuryakov from comment #24) > Comment on attachment 389282 [details] > Another speculative test fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=389282&action=review > > > Source/WebKit/ChangeLog:3 > > + Unreviewed, another speculative test fix after r255041 > > This says "unreviewed", but the patch is marked for review. Looks > reasonable, rs=me with one question. > > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:359 > > + (id)kSecAttrAccessible: (id)kSecAttrAccessibleAfterFirstUnlock, > > Why not kSecAttrAccessibleAlways? I don't think that there is a practical > difference, but "always" is what we want for testing, so it seems better to > request that explicitly. The API is marked as deprecated. I guess we should avoid using deprecated API.
Committed r255466: <https://trac.webkit.org/changeset/255466>