This bug extends https://bugs.webkit.org/show_bug.cgi?id=160672, so that both the Mac ports and IOS ports inherit from a shared Darwin port.
Created attachment 285675 [details] Patch
After this patch only the win port inherits from ApplePort, doesn't it? If yes,you could move the remaining ApplePort to win.py.
Darwin still shares functionality with ApplePort, so Darwin inherits from ApplePort. I'm not exactly sure the original reason for separating the Windows port from the GTK and EFL, perhaps the Port class could take on some of the implementation from ApplePort?
Comment on attachment 285675 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285675&action=review > Tools/Scripts/webkitpy/port/darwin.py:59 > + def supports_per_test_timeout(self): > + return True Following the move of this method from class ApplePort to DarwinPort, the supports_per_test_timeout() returns False for the Windows port by <https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/port/base.py?rev=204603#L155>. It looks like all ports support per test timeouts. So, we should change the base class implementation to remove True and remove all the port-specific overrides. > Tools/Scripts/webkitpy/port/darwin.py:199 > + def xcrun_find(self, command, fallback): > + _log.warn("xcrun undefined; falling back to '%s'." % fallback) > + return fallback This does not seem correct. We can and should make use of xcrun to find command line tools for both the Mac and iOS ports.
(In reply to comment #4) > Comment on attachment 285675 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=285675&action=review .... > > > Tools/Scripts/webkitpy/port/darwin.py:199 > > + def xcrun_find(self, command, fallback): > > + _log.warn("xcrun undefined; falling back to '%s'." % fallback) > > + return fallback > > This does not seem correct. We can and should make use of xcrun to find > command line tools for both the Mac and iOS ports. We do use xcrun for both Mac and iOS, but iOS assumes it's being run through a simulator, so it has a few extra arguments. ios.py line 400 has the iOS simulator xcrun.
Created attachment 286405 [details] Patch
Attachment 286405 [details] did not pass style-queue: ERROR: Tools/Scripts/webkitpy/port/mac.py:37: No name 'darwin' in module 'webkitpy.port' [pylint/E0611] [5] ERROR: Tools/Scripts/webkitpy/port/mac.py:192: [MacPort.xcrun_find] Instance of 'MacPort' has no '_executive' member [pylint/E1101] [5] ERROR: Tools/Scripts/webkitpy/port/ios.py:36: No name 'darwin' in module 'webkitpy.port' [pylint/E0611] [5] Total errors found: 3 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 286407 [details] Patch
(In reply to comment #5) > (In reply to comment #4) > > Comment on attachment 285675 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=285675&action=review > .... > > > > > Tools/Scripts/webkitpy/port/darwin.py:199 > > > + def xcrun_find(self, command, fallback): > > > + _log.warn("xcrun undefined; falling back to '%s'." % fallback) > > > + return fallback > > > > This does not seem correct. We can and should make use of xcrun to find > > command line tools for both the Mac and iOS ports. > > We do use xcrun for both Mac and iOS, but iOS assumes it's being run through > a simulator, so it has a few extra arguments. ios.py line 400 has the iOS > simulator xcrun. Can we share more of the common code from MacPort.xcrun_find() and IOSPort.xcrun_find()?
(In reply to comment #9) > Can we share more of the common code from MacPort.xcrun_find() and > IOSPort.xcrun_find()? *IOSSimulatorPort
Comment on attachment 285675 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285675&action=review >>>> Tools/Scripts/webkitpy/port/darwin.py:199 >>>> + return fallback >>> >>> This does not seem correct. We can and should make use of xcrun to find command line tools for both the Mac and iOS ports. >> >> We do use xcrun for both Mac and iOS, but iOS assumes it's being run through a simulator, so it has a few extra arguments. ios.py line 400 has the iOS simulator xcrun. > > Can we share more of the common code from MacPort.xcrun_find() and IOSPort.xcrun_find()? So, just for reference (since the IOS xcrun isn't on this revision, here is that function definition: # iOS xcrun_find(...) definition def xcrun_find(self, command, fallback=None): fallback = fallback or command try: return self._executive.run_command(['xcrun', '--sdk', 'iphonesimulator', '-find', command]).rstrip() except ScriptError: _log.warn("xcrun failed; falling back to '%s'." % fallback) return fallback I think that xcrun_find should remain separate in the two ports, particularly since we may be creating a port for an iOS device, which would use an xcrun_find similar to iOS, but targeting the device instead of the simulator.
Created attachment 286786 [details] Patch
Created attachment 289235 [details] Patch
Comment on attachment 289235 [details] Patch Clearing flags on attachment: 289235 Committed r206934: <http://trac.webkit.org/changeset/206934>
All reviewed patches have been landed. Closing bug.
Committed r206940: <http://trac.webkit.org/changeset/206940>