Bug 85542 - --coverage should not not use the "feature" options system in build-webkit, similarly remove special-casing for WTF_URL
Summary: --coverage should not not use the "feature" options system in build-webkit, s...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 85456
  Show dependency treegraph
 
Reported: 2012-05-03 14:10 PDT by Eric Seidel (no email)
Modified: 2012-07-13 11:04 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.22 KB, patch)
2012-05-03 14:13 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (5.46 KB, patch)
2012-05-03 14:31 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (5.12 KB, patch)
2012-05-03 15:11 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2012-05-03 14:10:14 PDT
--coverage should not not use the "feature" options system in build-webkit, similarly remove special-casing for WTF_URL
Comment 1 Eric Seidel (no email) 2012-05-03 14:13:54 PDT
Created attachment 140091 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-05-03 14:15:26 PDT
Once this lands, I"ll move that big array of into its own file and then update my generate-feature-files command to autogenerate it. :)  (See bug 85456.)
Comment 3 Daniel Bates 2012-05-03 14:27:48 PDT
Comment on attachment 140091 [details]
Patch

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

Looks good to me.

> Tools/ChangeLog:15
> +        It should probably come back to life as an ENABLE_, or build-webkit or the XCode projects

Nit: XCode => Xcode
Comment 4 Eric Seidel (no email) 2012-05-03 14:29:37 PDT
Sorry, I always get that wrong.  There was some discussion of making the style-checker check for such. :)  Will fix.
Comment 5 Eric Seidel (no email) 2012-05-03 14:31:28 PDT
Created attachment 140093 [details]
Patch for landing
Comment 6 Eric Seidel (no email) 2012-05-03 15:11:55 PDT
Created attachment 140106 [details]
Patch for landing
Comment 7 Benjamin Poulain 2012-05-03 15:44:22 PDT
Comment on attachment 140106 [details]
Patch for landing

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

> Tools/ChangeLog:14
> +        I'm told that WTF_URL is not actively being worked on, so just removing it for now.

Actually I did not update for a while for personal reasons.

I should resume work on WTFURL this weekend.
Comment 8 WebKit Review Bot 2012-05-03 16:08:57 PDT
Comment on attachment 140106 [details]
Patch for landing

Clearing flags on attachment: 140106

Committed r116036: <http://trac.webkit.org/changeset/116036>
Comment 9 WebKit Review Bot 2012-05-03 16:09:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Eric Seidel (no email) 2012-05-03 16:11:55 PDT
We should probably just make WTF_URL an ENABLE.  I'm very happy to see that support continue to be worked on.  The support in build-webkit or WTF_URL was a one-off.
Comment 11 Benjamin Poulain 2012-05-03 16:14:36 PDT
(In reply to comment #10)
> We should probably just make WTF_URL an ENABLE.  I'm very happy to see that support continue to be worked on.  The support in build-webkit or WTF_URL was a one-off.

It will be a strange ENABLE but that is fine by me if that is the solution. I'll make a patch.
Comment 12 Eric Seidel (no email) 2012-05-03 16:16:37 PDT
Yeah, it's a bit odd.  That's probably the easiest way to deal with XCode however, since ENABLE_ seems special cased there.  Mark Rowe might know more about the details of how/why ENABLE_ handling is magical.
Comment 13 Eric Seidel (no email) 2012-05-03 16:17:58 PDT
I should also note that I'm not opposed to adding back in the CFLAGS special casing for WTF_URL if that's needed.  Just cleaning up these feature options code while I'm in there.