Summary: | Transition "WebKit Library Version" checks to SDK version checks | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||||
Component: | WebKit Misc. | Assignee: | Brady Eidson <beidson> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, benjamin, cdumez, cmarcelo, commit-queue, dbates, ggaren, thorton | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=168124 | ||||||||
Bug Depends on: | 168072 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Brady Eidson
2017-02-09 10:37:24 PST
Created attachment 301056 [details]
Patch
Comment on attachment 301056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301056&action=review r=me > Source/WebKit2/UIProcess/Cocoa/VersionChecks.h:32 > +enum class SDKVersion { Since we're going to cast to uint32_t, we should inherit from uint32_t here. (In reply to comment #2) > Comment on attachment 301056 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=301056&action=review > > r=me > > > Source/WebKit2/UIProcess/Cocoa/VersionChecks.h:32 > > +enum class SDKVersion { > > Since we're going to cast to uint32_t, we should inherit from uint32_t here. 👍👍 Build fix followup in https://trac.webkit.org/changeset/211987 Re-opened since this is blocked by bug 168072 This change didn't allow ToT WebKit changes to work if you build against an older or not-yet-released SDK. We can't just ignore WebKit library version numbers altogether - We need to try them first and then fall back to the SDK version number when necessary. Created attachment 301093 [details]
Patch
Comment on attachment 301093 [details]
Patch
r=me
Is there any way for a developer to avoid using or specifying LibraryVersion?
LibraryVersion seems like an anti-pattern because we communicate compatibility issues to developers based on SDK version and not library version.
(In reply to comment #9) > Comment on attachment 301093 [details] > Patch > > r=me > > Is there any way for a developer to avoid using or specifying LibraryVersion? I don't know. > LibraryVersion seems like an anti-pattern because we communicate > compatibility issues to developers based on SDK version and not library > version. I agree with this, but we do have this dilemma... Maybe the right approach is to do the SDK check *first*, then fallback to the library version if the SDK check returns false. That would prefer SDK for 3rd party apps, but let ToT WebKit development prefer the library version for testing purposes. (In reply to comment #10) > (In reply to comment #9) > > Comment on attachment 301093 [details] > > Patch > > > > r=me > > > > Is there any way for a developer to avoid using or specifying LibraryVersion? > > I don't know. > > > LibraryVersion seems like an anti-pattern because we communicate > > compatibility issues to developers based on SDK version and not library > > version. > > I agree with this, but we do have this dilemma... > > Maybe the right approach is to do the SDK check *first*, then fallback to > the library version if the SDK check returns false. > > That would prefer SDK for 3rd party apps, but let ToT WebKit development > prefer the library version for testing purposes. I'm going to do it this way and assume you're okay with it, and then you can yell at me later if that was a bad assumption. Comment on attachment 301093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301093&action=review > Source/WTF/wtf/spi/darwin/dyldSPI.h:61 > +#define DYLD_MACOSX_VERSION_10_12_4 0x000A0C04 As previously commented in follow-up bug, we should never check for minor SDK version - it's not OK for WebKit to change behavior when the developer switches to a new minor Xcode version. (In reply to comment #13) > Comment on attachment 301093 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=301093&action=review > > > Source/WTF/wtf/spi/darwin/dyldSPI.h:61 > > +#define DYLD_MACOSX_VERSION_10_12_4 0x000A0C04 > > As previously commented in follow-up bug, we should never check for minor > SDK version - it's not OK for WebKit to change behavior when the developer > switches to a new minor Xcode version. Replying in the other bug. |