Attachment 343853 Details for Bug 180827 – Rolling out

[patch] Rolling out

180827-rolling-out.diff (text/plain), 6.80 KB, created by Basuke Suzuki on 2018-06-28 14:29:45 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-06-28 14:29:45 PDT

Size: 6.80 KB

patch

obsolete

>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 90a02bda326..9f6a46f2e1b 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-06-28  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        Unreviewed, rolling out r226652.
>+        https://bugs.webkit.org/show_bug.cgi?id=180827
>+
>+        "PlatformInfo should never be instantiated in isolation. So, PlatformInfo should
>+        not have default argument values. The preferred way to get a PlatformInfo object
>+        is to instantiate a Host object." (Requested by dbates).
>+
>+        Reverted changeset:
>+
>+        "[webkitpy] PlatformInfo should have default argument for casual use"
>+        https://bugs.webkit.org/show_bug.cgi?id=180827
>+
> 2018-06-26  Daniel Bates  <dabates@apple.com>
> 
>         EWS should pass --status-host-uses-http when invoking webkit-patch, if needed
>diff --git a/Tools/Scripts/webkitpy/common/system/platforminfo.py b/Tools/Scripts/webkitpy/common/system/platforminfo.py
>index cc3bc01ef5c..054a94f8be4 100644
>--- a/Tools/Scripts/webkitpy/common/system/platforminfo.py
>+++ b/Tools/Scripts/webkitpy/common/system/platforminfo.py
>@@ -29,7 +29,6 @@
> 
> import re
> import sys
>-import platform
> 
> from webkitpy.common.version import Version
> from webkitpy.common.version_name_map import PUBLIC_TABLE, INTERNAL_TABLE, VersionNameMap
>@@ -49,16 +48,13 @@ class PlatformInfo(object):
>     newer than one known to the code.
>     """
> 
>-    def __init__(self, sys_module=None, platform_module=None, executive=None):
>-        sys_platform = (sys_module or sys).platform
>-        platform_module = platform_module or platform
>-
>+    def __init__(self, sys_module, platform_module, executive):
>         self._executive = executive
>         self._platform_module = platform_module
>-        self.os_name = self._determine_os_name(sys_platform)
>+        self.os_name = self._determine_os_name(sys_module.platform)
>         self.os_version = None
> 
>-        self._is_cygwin = sys_platform == 'cygwin'
>+        self._is_cygwin = sys_module.platform == 'cygwin'
> 
>         if self.os_name.startswith('mac'):
>             self.os_version = Version.from_string(platform_module.mac_ver()[0])
>@@ -70,13 +66,6 @@ class PlatformInfo(object):
>             # Most other platforms (namely iOS) return conforming version strings.
>             self.os_version = Version.from_string(platform_module.release())
> 
>-    @property
>-    def executive(self):
>-        if self._executive is None:
>-            self._executive = Executive()
>-
>-        return self._executive
>-
>     def is_mac(self):
>         return self.os_name == 'mac'
> 
>@@ -126,7 +115,7 @@ class PlatformInfo(object):
> 
>     def total_bytes_memory(self):
>         if self.is_mac():
>-            return long(self.executive.run_command(["sysctl", "-n", "hw.memsize"]))
>+            return long(self._executive.run_command(["sysctl", "-n", "hw.memsize"]))
>         return None
> 
>     def terminal_width(self):
>@@ -157,7 +146,7 @@ class PlatformInfo(object):
>     def xcode_sdk_version(self, sdk_name):
>         if self.is_mac():
>             # Assumes that xcrun does not write to standard output on failure (e.g. SDK does not exist).
>-            xcrun_output = self.executive.run_command(['xcrun', '--sdk', sdk_name, '--show-sdk-version'], return_stderr=False, ignore_errors=True).rstrip()
>+            xcrun_output = self._executive.run_command(['xcrun', '--sdk', sdk_name, '--show-sdk-version'], return_stderr=False, ignore_errors=True).rstrip()
>             if xcrun_output:
>                 return Version.from_string(xcrun_output)
>         return None
>@@ -165,13 +154,13 @@ class PlatformInfo(object):
>     def xcode_simctl_list(self):
>         if not self.is_mac():
>             return ()
>-        output = self.executive.run_command(['xcrun', 'simctl', 'list'], return_stderr=False)
>+        output = self._executive.run_command(['xcrun', 'simctl', 'list'], return_stderr=False)
>         return (line for line in output.splitlines())
> 
>     def xcode_version(self):
>         if not self.is_mac():
>             raise NotImplementedError
>-        return Version.from_string(self.executive.run_command(['xcodebuild', '-version']).split()[1])
>+        return Version.from_string(self._executive.run_command(['xcodebuild', '-version']).split()[1])
> 
>     def _determine_os_name(self, sys_platform):
>         if sys_platform == 'darwin':
>@@ -201,4 +190,4 @@ class PlatformInfo(object):
>         if version:
>             return version
>         # Note that this should only ever be called on windows, so this should always work.
>-        return self.executive.run_command(['cmd', '/c', 'ver'], decode_output=False)
>+        return self._executive.run_command(['cmd', '/c', 'ver'], decode_output=False)
>diff --git a/Tools/Scripts/webkitpy/common/system/systemhost.py b/Tools/Scripts/webkitpy/common/system/systemhost.py
>index a15342201d8..3ef103e1807 100644
>--- a/Tools/Scripts/webkitpy/common/system/systemhost.py
>+++ b/Tools/Scripts/webkitpy/common/system/systemhost.py
>@@ -27,6 +27,8 @@
> # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> 
> import os
>+import platform
>+import sys
> 
> from . import environment, executive, file_lock, filesystem, platforminfo, user, workspace
> 
>@@ -35,7 +37,7 @@ class SystemHost(object):
>     def __init__(self):
>         self.executive = executive.Executive()
>         self.filesystem = filesystem.FileSystem()
>-        self.platform = platforminfo.PlatformInfo(executive=self.executive)
>+        self.platform = platforminfo.PlatformInfo(sys, platform, self.executive)
>         self.user = user.User(self.platform)
>         self.workspace = workspace.Workspace(self.filesystem, self.executive)
> 
>diff --git a/Tools/Scripts/webkitpy/common/system/user.py b/Tools/Scripts/webkitpy/common/system/user.py
>index 2a4ae31deb0..1d464dfe9be 100644
>--- a/Tools/Scripts/webkitpy/common/system/user.py
>+++ b/Tools/Scripts/webkitpy/common/system/user.py
>@@ -29,11 +29,14 @@
> import getpass
> import logging
> import os
>+import platform
> import re
> import shlex
>+import sys
> import subprocess
> import webbrowser
> 
>+from .executive import Executive
> from .platforminfo import PlatformInfo
> 
> 
>@@ -43,7 +46,7 @@ _log = logging.getLogger(__name__)
> try:
>     import readline
> except ImportError:
>-    if not PlatformInfo().is_native_win():
>+    if not sys.platform.startswith('win32'):
>         # There is no readline module for win32, not much to do except cry.
>         _log.warn("Unable to import readline.")
> 
>@@ -55,7 +58,7 @@ class User(object):
>     def __init__(self, platforminfo=None):
>         # We cannot get the PlatformInfo object from a SystemHost because
>         # User is part of SystemHost itself.
>-        self._platforminfo = platforminfo or PlatformInfo()
>+        self._platforminfo = platforminfo or PlatformInfo(sys, platform, Executive())
> 
>     # FIXME: These are @classmethods because bugzilla.py doesn't have a Tool object (thus no User instance).
>     @classmethod

Actions: View | Formatted Diff | Diff

Attachments on bug 180827: 329384 | 329493 | 329499 | 343851 | 343853