Currently AVFoundationSupport.py looks around for include files. This is something that CMake could be used for instead to determine.
Created attachment 334394 [details] Patch This is going to need to be tested on the internal AppleWin build. Please run it initially to get the contents of WebKitBuild/Release/DerivedSources/PAL/PALHeaderDetection.h From there make sure that the HAVE checks in cmakeconfig.h match up. If they do then this is good to go. If not then we need to start debugging in the CMake logs.
Created attachment 334423 [details] Patch Rebased on latest revision
Created attachment 334424 [details] Patch
Comment on attachment 334424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334424&action=review Ok this should hopefully work. Still needs to be checked on Apple Win internal builds > Source/WebCore/config.h:-28 > -#if PLATFORM(COCOA) This is already in Platform.h so no need to redefine it here.
Created attachment 334661 [details] Patch Rebasing after r229048
Attachment 334661 [details] did not pass style-queue: ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:39: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:62: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 4 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 334662 [details] Patch Make style queue happier...
Attachment 334662 [details] did not pass style-queue: ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:39: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:62: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 3 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 334750 [details] Patch Potential fix for apple internal build
Attachment 334750 [details] did not pass style-queue: ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:39: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:62: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 3 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 334759 [details] Patch Use HAVE_SYMBOL to hopefully get further in the apple internal build
Attachment 334759 [details] did not pass style-queue: ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:39: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:62: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/cmake/OptionsAppleWin.cmake:33: No trailing spaces [whitespace/trailing] [5] Total errors found: 4 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 334759 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334759&action=review > Source/cmake/OptionsAppleWin.cmake:43 > + set(CMAKE_REQUIRED_INCLUDES ${WEBKIT_LIBRARIES_INCLUDE_DIR}) > + set(CMAKE_REQUIRED_LIBRARIES > + "${WEBKIT_LIBRARIES_LINK_DIR}/CoreFoundation${DEBUG_SUFFIX}.lib" > + "${WEBKIT_LIBRARIES_LINK_DIR}/AVFoundationCF${DEBUG_SUFFIX}.lib" > + "${WEBKIT_LIBRARIES_LINK_DIR}/QuartzCore${DEBUG_SUFFIX}.lib" > + ) > + > + WEBKIT_CHECK_HAVE_INCLUDE(HAVE_AVCF AVFoundationCF/AVCFBase.h) > + > + if (HAVE_AVCF) > + SET_AND_EXPOSE_TO_BUILD(USE_AVFOUNDATION ON) > + endif () > + > + WEBKIT_CHECK_HAVE_SYMBOL(HAVE_AVCF_LEGIBLE_OUTPUT AVCFPlayerItemLegibleOutputSetCallbacks "TargetConditionals.h;dispatch/dispatch.h;AVFoundationCF/AVFoundationCF.h;AVFoundationCF/AVCFPlayerItemLegibleOutput.h") > + WEBKIT_CHECK_HAVE_SYMBOL(HAVE_AVFOUNDATION_LOADER_DELEGATE AVCFAssetResourceLoaderSetCallbacks "TargetConditionals.h;dispatch/dispatch.h;AVFoundationCF/AVFoundationCF.h") > + WEBKIT_CHECK_HAVE_SYMBOL(HAVE_AVCFURL_PLAYABLE_MIMETYPE AVCFURLAssetIsPlayableExtendedMIMEType "TargetConditionals.h;dispatch/dispatch.h;AVFoundationCF/AVFoundationCF.h") > + WEBKIT_CHECK_HAVE_SYMBOL(HAVE_AVCFPLAYERITEM_CALLBACK_VERSION_2 kAVCFPlayerItemLegibleOutput_CallbacksVersion_2 "TargetConditionals.h;dispatch/dispatch.h;AVFoundationCF/AVFoundationCF.h") On public apple win I am using have symbol rather than looking directly for the include. The have symbol actually creates a C file and tries to compile and link it. The ones expected for public apple win are present so things SHOULD work for the internal apple build
Per Arne if everything is getting detected properly on Apple's internal windows build I'd like to get this reviewed and merged.
Created attachment 348206 [details] Patch Rebased and trying to use a compile check for the callback v2
Created attachment 348207 [details] Patch
Attachment 348207 [details] did not pass style-queue: ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:39: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 3 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 348207 [details] Patch Per Arne everything seems to have built on AppleWin side. Just need confirmation that AppleWin internal builds are happy and have the right values present. If so its good to go.
Created attachment 348325 [details] Patch Verified that the public AppleWin build has the same behavior through the CMake checks. https://gist.github.com/donny-dont/78a553447df58882b38854863749a6d1 has the outputted cmakeconfig.h
Comment on attachment 348325 [details] Patch Still needs to be verified with the internal AppleWin builds to make sure the have checks pick up everything properly.
Attachment 348325 [details] did not pass style-queue: ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:39: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 3 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 348325 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348325&action=review > Source/WebCore/config.h:-30 > -#if PLATFORM(COCOA) > -#define USE_FILE_LOCK 1 > -#endif Is it correct to remove this define?
Created attachment 348355 [details] Patch Updating based on review feedback
Attachment 348355 [details] did not pass style-queue: ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:55: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 348359 [details] Patch
Comment on attachment 348359 [details] Patch Attachment 348359 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9018378 New failing tests: accessibility/smart-invert-reference.html
Created attachment 348381 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
(In reply to Build Bot from comment #27) > Created attachment 348381 [details] > Archive of layout-test-results from ews106 for mac-sierra-wk2 > > The attached test failures were seen while running run-webkit-tests on the > mac-wk2-ews. > Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6 I don't see why this would be caused by this patch and I saw a couple bot failures on mac-wk2 for other patches. Per Arne, Alex or Brent could you give this a review? This should work for Apple's internal builds. I tested it as much as I could without access to it.
Comment on attachment 348359 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348359&action=review r=me. Please make sure the bots are green before landing. > Source/WebCore/PAL/ChangeLog:8 > + Remove AVFoundationSupport.py file and invokation. invocation > Source/WebKitLegacy/win/WebKitPrefix.h:69 > +#if USE(CG) > +#ifndef CGFLOAT_DEFINED > +#if (defined(__LP64__) && __LP64__) || (defined(__x86_64__) && __x86_64__) || defined(_M_X64) || defined(__amd64__) > +typedef double CGFloat; > +#else > +typedef float CGFloat; > +#endif > +#define CGFLOAT_DEFINED 1 > +#endif > +#endif /* USE(CG) */ This could perhaps be put in a new header file (CGFloat.h) to avoid duplication. > Source/cmake/WebKitCommon.cmake:-78 > - file(MAKE_DIRECTORY ${DERIVED_SOURCES_PAL_DIR}) Is creating this directory not needed anymore?
Created attachment 348532 [details] Patch Updated based on review
Created attachment 348544 [details] Patch Ok it appears those #defines are needed in config.h so adding them back
(In reply to Per Arne Vollan from comment #29) > Comment on attachment 348359 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=348359&action=review > > r=me. Please make sure the bots are green before landing. > > > Source/WebCore/PAL/ChangeLog:8 > > + Remove AVFoundationSupport.py file and invokation. > > invocation > > > Source/WebKitLegacy/win/WebKitPrefix.h:69 > > +#if USE(CG) > > +#ifndef CGFLOAT_DEFINED > > +#if (defined(__LP64__) && __LP64__) || (defined(__x86_64__) && __x86_64__) || defined(_M_X64) || defined(__amd64__) > > +typedef double CGFloat; > > +#else > > +typedef float CGFloat; > > +#endif > > +#define CGFLOAT_DEFINED 1 > > +#endif > > +#endif /* USE(CG) */ > > This could perhaps be put in a new header file (CGFloat.h) to avoid > duplication. Will look into getting rid of this in a follow up. > > > Source/cmake/WebKitCommon.cmake:-78 > > - file(MAKE_DIRECTORY ${DERIVED_SOURCES_PAL_DIR}) > > Is creating this directory not needed anymore? Nope its not. AVFoundationSupport.py was the only thing writing to it.
Comment on attachment 348544 [details] Patch Clearing flags on attachment: 348544 Committed r235531: <https://trac.webkit.org/changeset/235531>
All reviewed patches have been landed. Closing bug.
*** Bug 182800 has been marked as a duplicate of this bug. ***
Committed build fix for WPE in r235536: <https://trac.webkit.org/changeset/235536>