Evan Martin <
ev...@chromium.org> writes:
> On Thu, Nov 8, 2012 at 5:26 PM, Jan Beich <
jbe...@tormail.org> wrote:
>> I'm trying to make WebRTC code work on BSDs. This needs adding
>> more 'if' statements which look ugly:
>
> I don't know this code, but it's not clear to me what "GetFlavor" is
> even supposed to *do*. (I think Mark had this same objection.)
GetFlavor is currently used to set OS variable which can later be
checked in gyp files e.g.,
# A flag for BSD platforms
['OS=="dragonfly" or OS=="freebsd" or OS=="netbsd" or \
OS=="openbsd"', {
'os_bsd%': 1,
}, {
'os_bsd%': 0,
}],
...
['OS=="linux"', {
'defines': [
'WEBRTC_LINUX',
'WEBRTC_THREAD_RR',
],
}],
['os_bsd==1', {
'defines': [
'WEBRTC_BSD',
'WEBRTC_THREAD_RR',
],
}],
['OS=="dragonfly" or OS=="netbsd"', {
'defines': [
# doesn't support pthread_condattr_setclock
'WEBRTC_CLOCK_TYPE_REALTIME',
],
}],
> Does gyp even care about which kind of Unix you are?
Only in fringe cases. 'OS' variable is an example where things get ugly.
> The special handling of Mac in this code I think is because gyp needs
> to understand about Mac-specific stuff like .dylib, but as far as gyp
> goes I think FreeBSD/sorlaris/etc. should all be treated identically
> to Linux.
What to do with flock/sun_tool.py then?
def GenerateOutput(target_list, target_dicts, data, params):
options = params['options']
flavor = gyp.common.GetFlavor(params)
...
elif flavor == 'solaris':
header_params.update({
'flock': './gyp-sun-tool flock',
'flock_index': 2,
'extra_commands': SHARED_HEADER_SUN_COMMANDS,
})
elif flavor == 'freebsd':
header_params.update({
'flock': 'lockf',
})
Anyway, here's v2 for better flavoring. I've tried to reduce
proliferation of startswith() without changing fallback.
# - add 'bsd' flavor but keep old value for 'OS' to avoid churn
# - use platform.system(), it's more consistent than sys.platform
# - update tests
Index: pylib/gyp/common.py
===================================================================
--- pylib/gyp/common.py (revision 1533)
+++ pylib/gyp/common.py (working copy)
@@ -10,6 +10,7 @@ import os.path
import re
import tempfile
import sys
+import platform
# A minimal memoizing decorator. It'll blow up if the args aren't immutable,
@@ -364,20 +365,24 @@ def WriteOnDiff(filename):
def GetFlavor(params):
"""Returns |params.flavor| if it's set, the system's default flavor else."""
+ system = platform.system().lower()
flavors = {
- 'cygwin': 'win',
- 'win32': 'win',
- 'darwin': 'mac',
+ 'microsoft': 'win',
+ 'windows' : 'win',
+ 'darwin' : 'mac',
+ 'sunos' : 'solaris',
+ 'dragonfly': 'bsd',
+ 'freebsd' : 'bsd',
+ 'netbsd' : 'bsd',
+ 'openbsd' : 'bsd',
}
if 'flavor' in params:
return params['flavor']
- if sys.platform in flavors:
- return flavors[sys.platform]
- if sys.platform.startswith('sunos'):
- return 'solaris'
- if sys.platform.startswith('freebsd'):
- return 'freebsd'
+ if system.startswith('cygwin'):
+ return 'win'
+ if system in flavors:
+ return flavors[system]
return 'linux'
Index: pylib/gyp/common_test.py
===================================================================
--- pylib/gyp/common_test.py (revision 1533)
+++ pylib/gyp/common_test.py (working copy)
@@ -9,6 +9,7 @@
import gyp.common
import unittest
import sys
+import platform
class TestTopologicallySorted(unittest.TestCase):
@@ -43,28 +44,27 @@ class TestTopologicallySorted(unittest.TestCase):
class TestGetFlavor(unittest.TestCase):
"""Test that gyp.common.GetFlavor works as intended"""
- original_platform = ''
+ original_system = ''
def setUp(self):
- self.original_platform = sys.platform
+ self.original_system = platform.system
def tearDown(self):
- sys.platform = self.original_platform
+ platform.system = self.original_system
def assertFlavor(self, expected, argument, param):
- sys.platform = argument
+ platform.system = lambda: argument
self.assertEqual(expected, gyp.common.GetFlavor(param))
def test_platform_default(self):
- self.assertFlavor('freebsd', 'freebsd9' , {})
- self.assertFlavor('freebsd', 'freebsd10', {})
- self.assertFlavor('solaris', 'sunos5' , {});
- self.assertFlavor('solaris', 'sunos' , {});
- self.assertFlavor('linux' , 'linux2' , {});
- self.assertFlavor('linux' , 'linux3' , {});
+ self.assertFlavor('win' , 'CYGWIN_NT-6.1-WOW64', {})
+ self.assertFlavor('win' , 'Microsoft' , {})
+ self.assertFlavor('linux' , 'Linux' , {})
+ self.assertFlavor('solaris', 'SunOS' , {})
+ self.assertFlavor('bsd' , 'DragonFly' , {})
def test_param(self):
- self.assertFlavor('foobar', 'linux2' , {'flavor': 'foobar'})
+ self.assertFlavor('foobar', 'Linux' , {'flavor': 'foobar'})
if __name__ == '__main__':
Index: pylib/gyp/generator/make.py
===================================================================
--- pylib/gyp/generator/make.py (revision 1533)
+++ pylib/gyp/generator/make.py (working copy)
@@ -24,6 +24,7 @@
import os
import re
import sys
+import platform
import subprocess
import gyp
import gyp.common
@@ -85,8 +86,9 @@ def CalculateVariables(default_variables, params):
COMPILABLE_EXTENSIONS.update({'.m': 'objc', '.mm' : 'objcxx'})
else:
operating_system = flavor
- if flavor == 'android':
- operating_system = 'linux' # Keep this legacy behavior for now.
+ if flavor in ['android', 'bsd']:
+ # Keep this legacy behavior for now.
+ operating_system = platform.system().lower()
default_variables.setdefault('OS', operating_system)
default_variables.setdefault('SHARED_LIB_SUFFIX', '.so')
default_variables.setdefault('SHARED_LIB_DIR','$(builddir)/lib.$(TOOLSET)')
@@ -2000,7 +2002,7 @@ def GenerateOutput(target_list, target_dicts, data
'flock_index': 2,
'extra_commands': SHARED_HEADER_SUN_COMMANDS,
})
- elif flavor == 'freebsd':
+ elif flavor == 'bsd':
header_params.update({
'flock': 'lockf',
})
Index: pylib/gyp/generator/ninja.py
===================================================================
--- pylib/gyp/generator/ninja.py (revision 1533)
+++ pylib/gyp/generator/ninja.py (working copy)
@@ -10,6 +10,7 @@ import re
import signal
import subprocess
import sys
+import platform
import gyp
import gyp.common
import gyp.msvs_emulation
@@ -1295,8 +1296,9 @@ def CalculateVariables(default_variables, params):
default_variables['MSVS_OS_BITS'] = 32
else:
operating_system = flavor
- if flavor == 'android':
- operating_system = 'linux' # Keep this legacy behavior for now.
+ if flavor in ['android', 'bsd']:
+ # Keep this legacy behavior for now.
+ operating_system = platform.system().lower()
default_variables.setdefault('OS', operating_system)
default_variables.setdefault('SHARED_LIB_SUFFIX', '.so')
default_variables.setdefault('SHARED_LIB_DIR',