Bug 185502 - Update ICU headers for macOS to v60
Summary: Update ICU headers for macOS to v60
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy VanWagoner
URL:
Keywords:
Depends on:
Blocks: 185375
  Show dependency treegraph
 
Reported: 2018-05-09 18:17 PDT by Andy VanWagoner
Modified: 2018-05-10 16:35 PDT (History)
4 users (show)

See Also:


Attachments
Patch (849.87 KB, patch)
2018-05-09 18:20 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch (882.51 KB, patch)
2018-05-09 19:10 PDT, Andy VanWagoner
ap: review-
ap: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy VanWagoner 2018-05-09 18:17:29 PDT
Update ICU headers for macOS to v60
Comment 1 Andy VanWagoner 2018-05-09 18:20:46 PDT
Created attachment 340057 [details]
Patch
Comment 2 Andy VanWagoner 2018-05-09 18:22:47 PDT
Comment on attachment 340057 [details]
Patch

This updates all the ICU headers to v60.2 so building on macOS can use the newer features. All modifications from the original ICU source are documented in README.
Comment 3 EWS Watchlist 2018-05-09 18:23:41 PDT
Attachment 340057 [details] did not pass style-queue:


ERROR: Source/WTF/ChangeLog:193:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: spoof  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 169 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Andy VanWagoner 2018-05-09 19:10:28 PDT
Created attachment 340062 [details]
Patch
Comment 5 Andy VanWagoner 2018-05-09 19:11:14 PDT
Comment on attachment 340062 [details]
Patch

Includes new headers missing in last patch.
Comment 6 EWS Watchlist 2018-05-09 19:13:54 PDT
Attachment 340062 [details] did not pass style-queue:


ERROR: Source/WTF/ChangeLog:196:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: spoof  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 172 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Andy VanWagoner 2018-05-09 19:19:44 PDT
(In reply to Build Bot from comment #6)
> Attachment 340062 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WTF/ChangeLog:196:  Please consider whether the use of
> security-sensitive phrasing could help someone exploit WebKit: spoof 
> [changelog/unwantedsecurityterms] [3]
> Total errors found: 1 in 172 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

uspoof.h is one of the updated headers, and it provides a spoof checker. It can be helpful in detecting a string that looks deceptively like another. I don't think removing this reference from the Changelog would be helpful.
Comment 8 Alexey Proskuryakov 2018-05-09 23:16:20 PDT
Comment on attachment 340062 [details]
Patch

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

> Source/WTF/ChangeLog:3
> +        Update ICU headers for macOS to v60

Is this moving too far? I think that macOS Sierra has v57.
Comment 9 Andy VanWagoner 2018-05-10 08:13:44 PDT
(In reply to Alexey Proskuryakov from comment #8)
> Comment on attachment 340062 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340062&action=review
> 
> > Source/WTF/ChangeLog:3
> > +        Update ICU headers for macOS to v60
> 
> Is this moving too far? I think that macOS Sierra has v57.

I'm running High Sierra (10.13.4), and it has v59.

I believe all of the ICU APIs the Intl APIs are trying to use are included in v59, so I could use those headers instead, if you think that's a better move.

It's a shame these headers are not provided by the system, or at least by the SDK, so that the feature detecting macros could work as expected.
Comment 10 Alexey Proskuryakov 2018-05-10 09:19:13 PDT
> I'm running High Sierra (10.13.4), and it has v59.

That's High Sierra, not Sierra. We need to support Sierra too.

Having newer headers is not strictly forbidden, but there are problems with that:

- As this presumably changes web exposed functionality, WebKit will behave differently on Sierra and on High Sierra. It's best to minimize such differences, as we want to have a consistent developer story across OS versions, as much as possible.

- Having only the oldest supported version makes it trivial to ensure that we aren't using unsupported features. With a version that contains supported and unsupported functionality, how is one to know what's safe to use?

> It's a shame these headers are not provided by the system, or at least by the SDK, so that the feature detecting macros could work as expected.

Please file a bug with Apple (bugreport.apple.com), Bugzilla comments will not be heard.
Comment 11 Andy VanWagoner 2018-05-10 09:28:59 PDT
(In reply to Alexey Proskuryakov from comment #10)
> > I'm running High Sierra (10.13.4), and it has v59.
> 
> That's High Sierra, not Sierra. We need to support Sierra too.
> 
> Having newer headers is not strictly forbidden, but there are problems with
> that:
> 
> - As this presumably changes web exposed functionality, WebKit will behave
> differently on Sierra and on High Sierra. It's best to minimize such
> differences, as we want to have a consistent developer story across OS
> versions, as much as possible.
> 
> - Having only the oldest supported version makes it trivial to ensure that
> we aren't using unsupported features. With a version that contains supported
> and unsupported functionality, how is one to know what's safe to use?

In JavaScript you already have to detect whether the function is available or not because SpiderMonkey and V8 have it, but Chakra and JSC do not.

That being said, it does make sense to use lowest common denominator here. Is there someone that can say A) What the oldest macOS version this needs to support is, then B) what version of ICU is available on that OS version?

For features like this that require a newer system library does it just need to wait a few years, or can system libraries be updated in older OS versions?

> 
> > It's a shame these headers are not provided by the system, or at least by the SDK, so that the feature detecting macros could work as expected.
> 
> Please file a bug with Apple (bugreport.apple.com), Bugzilla comments will
> not be heard.

I will take my complaint there. :)
Comment 12 Alexey Proskuryakov 2018-05-10 09:56:42 PDT
Comment on attachment 340062 [details]
Patch

> Is there someone that can say A) What the oldest macOS version this needs to support is, then B) what version of ICU is available on that OS version?

I think that I answered that when saying Sierra and v57.

> In JavaScript you already have to detect whether the function is available or not because SpiderMonkey and V8 have it, but Chakra and JSC do not.

This is true. I still think that it's better for the same version of WebKit on different platforms to expose the same Intl behavior, especially across macOS versions. But it's OK to discuss on webkit-dev to give others an opportunity to weigh in. r- for now, but please feel to re-nominate if there is support on webkit-dev.

> For features like this that require a newer system library does it just need to wait a few years, or can system libraries be updated in older OS versions?

We can't update the system ICU, but we may be able to develop a technology to have a separate copy just for Safari on Sierra. The benefit would need to be high for such a project to be worth the effort, and there might be difficulties that I'm not thinking of now.
Comment 13 Andy VanWagoner 2018-05-10 16:35:32 PDT
macOS Sierra only has v57, and it cannot be updated, so we cannot update the headers to a newer version than that yet.

Updating the headers to v57 does not enable any APIs, so we can defer updating the headers until it is practical to update them to at least v59.