Skip to content

Remove several HAVE directives now that macOS 14 is the minimum #47185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 28, 2025
Merged

Remove several HAVE directives now that macOS 14 is the minimum #47185

merged 1 commit into from
Jun 28, 2025

Conversation

annevk
Copy link
Contributor

@annevk annevk commented Jun 25, 2025 •
edited by webkit-early-warning-system
Loading

2d046f6

Remove several HAVE directives now that macOS 14 is the minimum
https://bugs.webkit.org/show_bug.cgi?id=294973

Reviewed by Tim Horton and Per Arne Vollan.

This removes:

- HAVE_NSVIEW_CLIPSTOBOUNDS_API
- HAVE_CGIMAGESOURCE_DISABLE_HARDWARE_DECODING
- HAVE_CGIMAGESOURCE_ENABLE_RESTRICTED_DECODING
- HAVE_AVURLASSET_ISPLAYABLEEXTENDEDMIMETYPEWITHOPTIONS
- HAVE_CTFONT_COPYCOLORGLYPHCOVERAGE
- HAVE_NSCOLORWELL_SUPPORTS_ALPHA

What was guarded by HAVE_CTFONT_COPYCOLORGLYPHCOVERAGE is now also
enabled on visionOS as that seemed like an oversight.

Canonical link: https://commits.webkit.org/296769@main

8f4ac84

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac   🛠 wpe   🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
  🧪 ios-wk2-wpt   🧪 mac-wk1   🛠 wpe-cairo
✅ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2   🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim   🧪 mac-wk2-stress   🧪 api-gtk
  🧪 vision-wk2   🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim   🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@annevk annevk self-assigned this Jun 25, 2025
@annevk annevk added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Jun 25, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 25, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 04fad31)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@annevk annevk marked this pull request as ready for review June 25, 2025 16:42
@annevk annevk requested a review from cdumez as a code owner June 25, 2025 16:42
hortont424
hortont424 reviewed Jun 27, 2025
View reviewed changes
Source/WTF/wtf/PlatformHave.h
|| PLATFORM(IOS) \
|| (PLATFORM(APPLETV) && __TV_OS_VERSION_MAX_ALLOWED >= 170000) \
|| PLATFORM(WATCHOS)
#define HAVE_CTFONT_COPYCOLORGLYPHCOVERAGE 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes for visionOS but I assume it's a good change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted this in the commit message.

hortont424
hortont424 reviewed Jun 27, 2025
View reviewed changes
Source/WebCore/PAL/pal/spi/cocoa/AVFoundationSPI.h
@@ -486,8 +486,7 @@ NS_ASSUME_NONNULL_END
@end
#endif

// FIXME: Move into !USE(APPLE_INTERNAL_SDK) section once rdar://111695863 has been in the build a while
#if HAVE(AVURLASSET_ISPLAYABLEEXTENDEDMIMETYPEWITHOPTIONS)
#if !USE(APPLE_INTERNAL_SDK)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this file not already have such a section to move these to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not appear to have a clear section, no. Lots of small islands.

@annevk annevk force-pushed the eng/Remove-several-HAVE-directives-now-that-macOS-14-is-the-minimum branch from 04fad31 to 8f4ac84 Compare June 28, 2025 09:30
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 28, 2025 •
edited
Loading

EWS run on current version of this PR (hash 8f4ac84)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac   🛠 wpe   🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
  🧪 ios-wk2-wpt   🧪 mac-wk1   🛠 wpe-cairo
✅ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2   🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim   🧪 mac-wk2-stress   🧪 api-gtk
  🧪 vision-wk2   🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim   🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@annevk annevk added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 28, 2025
@annevk
Remove several HAVE directives now that macOS 14 is the minimum
2d046f6 
https://bugs.webkit.org/show_bug.cgi?id=294973

Reviewed by Tim Horton and Per Arne Vollan.

This removes:

- HAVE_NSVIEW_CLIPSTOBOUNDS_API
- HAVE_CGIMAGESOURCE_DISABLE_HARDWARE_DECODING
- HAVE_CGIMAGESOURCE_ENABLE_RESTRICTED_DECODING
- HAVE_AVURLASSET_ISPLAYABLEEXTENDEDMIMETYPEWITHOPTIONS
- HAVE_CTFONT_COPYCOLORGLYPHCOVERAGE
- HAVE_NSCOLORWELL_SUPPORTS_ALPHA

What was guarded by HAVE_CTFONT_COPYCOLORGLYPHCOVERAGE is now also
enabled on visionOS as that seemed like an oversight.

Canonical link: https://commits.webkit.org/296769@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Remove-several-HAVE-directives-now-that-macOS-14-is-the-minimum branch from 8f4ac84 to 2d046f6 Compare June 28, 2025 09:33
@webkit-commit-queue
Copy link
Collaborator

Committed 296769@main (2d046f6): https://commits.webkit.org/296769@main

Reviewed commits have been landed. Closing PR #47185 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 2d046f6 into WebKit:main Jun 28, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 28, 2025
@annevk annevk deleted the eng/Remove-several-HAVE-directives-now-that-macOS-14-is-the-minimum branch June 28, 2025 09:36
@rkirsling
Copy link
Member

rkirsling commented Jun 28, 2025 •
edited
Loading

This broke the Windows build. EWS would have reported it, but here's the output for the Release builder:
https://build.webkit.org/#/builders/1192/builds/9701

@annevk
Copy link
Contributor Author

annevk commented Jun 29, 2025

I'm sorry about that. EWS was green for Windows though (tests never ran): #47185 (comment)

@rkirsling
Copy link
Member

Oh geez, huh. Is it a clean vs. incremental build thing? 🤔

@annevk
Copy link
Contributor Author

annevk commented Jun 29, 2025

No, https://commits.webkit.org/296713@main by @carlosgcampos happened between then and now. I can probably restore some of this to make it work again.

@annevk
Copy link
Contributor Author

annevk commented Jun 29, 2025

#47350

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hortont424 hortont424 hortont424 approved these changes

@pvollan pvollan pvollan approved these changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

Assignees

@annevk annevk

Labels
WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@annevk @webkit-early-warning-system @webkit-commit-queue @rkirsling @hortont424 @pvollan