The EWS build system is forcing a non UTF-8 locale and breaks meson.
authorclopez@igalia.com <clopez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Sep 2017 17:04:31 +0000 (17:04 +0000)
committerclopez@igalia.com <clopez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Sep 2017 17:04:31 +0000 (17:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176761

Reviewed by Michael Catanzaro.

On 2011 in order to disable GCC smartquotes (UTF-8 pretty quotes)
that were causing trouble with the commit-queue server, the tooling
for the EWS bots started to force an LC_ALL=C locale.

This was needed back then apparently because the commit-queue server
was unable to handle properly UTF-8 text.

6 years forward and none of the ports that commit-queue tests before
landing a patch use GCC, but Clang. So this should not be an issue anymore.

Forcing a non UTF-8 locale breaks the Meson build system (that the
GTK/WPE ports use for some dependencies of its JHBuild dependencies
local build system).
It has also the potential to break things in subtle ways that are
hard to debug and detect. So better stop doing this.

This patch also changes TERM=none to TERM=dumb that is the proper
way to disable VT100 codes (see bug 127922).

* Scripts/webkitpy/common/system/environment.py:
(Environment.to_dictionary):
(Environment.disable_jhbuild_VT100_output):
* Scripts/webkitpy/common/system/environment_unittest.py:
(EnvironmentTest.test_disable_jhbuild_VT100_output):
* Scripts/webkitpy/port/base.py:
(Port._build_driver):
(Port._build_image_diff):
* Scripts/webkitpy/port/port_testcase.py:
(PortTestCase.test_build_driver):
* Scripts/webkitpy/tool/commands/download_unittest.py:
* Scripts/webkitpy/tool/steps/build.py:
(Build.build):
* Scripts/webkitpy/tool/steps/steps_unittest.py:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221926 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/common/system/environment.py
Tools/Scripts/webkitpy/common/system/environment_unittest.py
Tools/Scripts/webkitpy/port/base.py
Tools/Scripts/webkitpy/port/port_testcase.py
Tools/Scripts/webkitpy/tool/commands/download_unittest.py
Tools/Scripts/webkitpy/tool/steps/build.py
Tools/Scripts/webkitpy/tool/steps/steps_unittest.py

index db1c918..d756177 100644 (file)
@@ -1,3 +1,44 @@
+2017-09-12  Carlos Alberto Lopez Perez  <clopez@igalia.com>
+
+        The EWS build system is forcing a non UTF-8 locale and breaks meson.
+        https://bugs.webkit.org/show_bug.cgi?id=176761
+
+        Reviewed by Michael Catanzaro.
+
+        On 2011 in order to disable GCC smartquotes (UTF-8 pretty quotes)
+        that were causing trouble with the commit-queue server, the tooling
+        for the EWS bots started to force an LC_ALL=C locale.
+
+        This was needed back then apparently because the commit-queue server
+        was unable to handle properly UTF-8 text.
+
+        6 years forward and none of the ports that commit-queue tests before
+        landing a patch use GCC, but Clang. So this should not be an issue anymore.
+
+        Forcing a non UTF-8 locale breaks the Meson build system (that the
+        GTK/WPE ports use for some dependencies of its JHBuild dependencies
+        local build system).
+        It has also the potential to break things in subtle ways that are
+        hard to debug and detect. So better stop doing this.
+
+        This patch also changes TERM=none to TERM=dumb that is the proper
+        way to disable VT100 codes (see bug 127922).
+
+        * Scripts/webkitpy/common/system/environment.py:
+        (Environment.to_dictionary):
+        (Environment.disable_jhbuild_VT100_output):
+        * Scripts/webkitpy/common/system/environment_unittest.py:
+        (EnvironmentTest.test_disable_jhbuild_VT100_output):
+        * Scripts/webkitpy/port/base.py:
+        (Port._build_driver):
+        (Port._build_image_diff):
+        * Scripts/webkitpy/port/port_testcase.py:
+        (PortTestCase.test_build_driver):
+        * Scripts/webkitpy/tool/commands/download_unittest.py:
+        * Scripts/webkitpy/tool/steps/build.py:
+        (Build.build):
+        * Scripts/webkitpy/tool/steps/steps_unittest.py:
+
 2017-09-12  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [WPE][GTK] Run tests with G_DEBUG=fatal-criticals
index c1bb04a..b7eb83f 100644 (file)
@@ -34,14 +34,8 @@ class Environment(object):
     def to_dictionary(self):
         return self.env
 
-    def disable_gcc_smartquotes(self):
-        # Technically we only need to set LC_CTYPE to disable current
-        # smartquote behavior: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38363
-        # Apple's XCode sets LC_ALL instead, probably to be future-proof.
-        self.env['LC_ALL'] = 'C'
-
     def disable_jhbuild_VT100_output(self):
         # JHBuild generates VT100 escape sequences if TERM is xterm or rxvt,
         # which makes browsers set wrong MIME type (e.g. Safari, Chrome).
         # https://bugs.webkit.org/show_bug.cgi?id=127922
-        self.env['TERM'] = 'none'
+        self.env['TERM'] = 'dumb'
index 6558b51..e861c08 100644 (file)
@@ -32,8 +32,8 @@ from .environment import Environment
 
 
 class EnvironmentTest(unittest.TestCase):
-    def test_disable_gcc_smartquotes(self):
+    def test_disable_jhbuild_VT100_output(self):
         environment = Environment({})
-        environment.disable_gcc_smartquotes()
+        environment.disable_jhbuild_VT100_output()
         env = environment.to_dictionary()
-        self.assertEqual(env['LC_ALL'], 'C')
+        self.assertEqual(env['TERM'], 'dumb')
index ab482b4..2a7417b 100644 (file)
@@ -1432,7 +1432,6 @@ class Port(object):
 
     def _build_driver(self):
         environment = self.host.copy_current_environment()
-        environment.disable_gcc_smartquotes()
         env = environment.to_dictionary()
 
         # FIXME: We build both DumpRenderTree and WebKitTestRunner for WebKitTestRunner runs because
@@ -1448,7 +1447,6 @@ class Port(object):
 
     def _build_image_diff(self):
         environment = self.host.copy_current_environment()
-        environment.disable_gcc_smartquotes()
         env = environment.to_dictionary()
         try:
             self._run_script("build-imagediff", env=env)
index 29e827d..9d29299 100644 (file)
@@ -557,18 +557,18 @@ class PortTestCase(unittest.TestCase):
         # Delay setting _executive to avoid logging during construction
         port._executive = MockExecutive(should_log=True)
         port._options = MockOptions(configuration="Release")  # This should not be necessary, but I think TestWebKitPort is actually reading from disk (and thus detects the current configuration).
-        expected_logs = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\n"
+        expected_logs = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}\n"
         self.assertTrue(output.assert_outputs(self, port._build_driver, expected_logs=expected_logs))
 
         # Make sure WebKitTestRunner is used.
         port._options = MockOptions(webkit_test_runner=True, configuration="Release")
-        expected_logs = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\nMOCK run_command: ['Tools/Scripts/build-webkittestrunner', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\n"
+        expected_logs = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}\nMOCK run_command: ['Tools/Scripts/build-webkittestrunner', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}\n"
         self.assertTrue(output.assert_outputs(self, port._build_driver, expected_logs=expected_logs))
 
         # Make sure we show the build log when --verbose is passed, which we simulate by setting the logging level to DEBUG.
         output.set_log_level(logging.DEBUG)
         port._options = MockOptions(configuration="Release")
-        expected_logs = """MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}
+        expected_logs = """MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}
 Output of ['Tools/Scripts/build-dumprendertree', '--release']:
 MOCK output of child process
 """
@@ -578,7 +578,7 @@ MOCK output of child process
         # Make sure that failure to build returns False.
         port._executive = MockExecutive(should_log=True, should_throw=True)
         # Because WK2 currently has to build both webkittestrunner and DRT, if DRT fails, that's the only one it tries.
-        expected_logs = """MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}
+        expected_logs = """MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}
 MOCK ScriptError
 
 MOCK output of child process
index 0c9bad0..91e2993 100644 (file)
@@ -164,7 +164,7 @@ MOCK run_command: ['ruby', '-I', '/mock-checkout/Websites/bugs.webkit.org/Pretty
 MOCK: user.open_url: file://...
 Was that diff correct?
 Building WebKit
-MOCK run_and_throw_if_fail: ['mock-build-webkit', 'ARCHS=MOCK ARCH'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'TERM': 'none', 'MOCK_ENVIRON_COPY': '1'}
+MOCK run_and_throw_if_fail: ['mock-build-webkit', 'ARCHS=MOCK ARCH'], cwd=/mock-checkout, env={'TERM': 'dumb', 'MOCK_ENVIRON_COPY': '1'}
 Running Python unit tests
 MOCK run_and_throw_if_fail: ['mock-test-webkitpy'], cwd=/mock-checkout
 Running Perl unit tests
index 6d648e3..8c995e0 100644 (file)
@@ -47,7 +47,6 @@ class Build(AbstractStep):
 
     def build(self, build_style, group):
         environment = self._tool.copy_current_environment()
-        environment.disable_gcc_smartquotes()
         environment.disable_jhbuild_VT100_output()
         env = environment.to_dictionary()
 
index 9ae81aa..26209c4 100644 (file)
@@ -184,7 +184,7 @@ MOCK run_and_throw_if_fail: ['Tools/Scripts/run-webkit-tests', '--debug', '--qui
         tool._deprecated_port = DeprecatedPort()
         step = steps.Build(tool, mock_options)
         expected_logs = """Building WebKit
-MOCK run_and_throw_if_fail: ['Tools/Scripts/build-jsc', '--debug', 'ARCHS=True'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'TERM': 'none', 'MOCK_ENVIRON_COPY': '1'}
+MOCK run_and_throw_if_fail: ['Tools/Scripts/build-jsc', '--debug', 'ARCHS=True'], cwd=/mock-checkout, env={'TERM': 'dumb', 'MOCK_ENVIRON_COPY': '1'}
 """
         OutputCapture().assert_outputs(self, step.run, [{}], expected_logs=expected_logs)
 
@@ -200,7 +200,7 @@ MOCK run_and_throw_if_fail: ['Tools/Scripts/build-jsc', '--debug', 'ARCHS=True']
         tool._deprecated_port = DeprecatedPort()
         step = steps.Build(tool, mock_options)
         expected_logs = """Building WebKit
-MOCK run_and_throw_if_fail: ['Tools/Scripts/build-jsc', '--release', 'ARCHS=True'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'TERM': 'none', 'MOCK_ENVIRON_COPY': '1'}
+MOCK run_and_throw_if_fail: ['Tools/Scripts/build-jsc', '--release', 'ARCHS=True'], cwd=/mock-checkout, env={'TERM': 'dumb', 'MOCK_ENVIRON_COPY': '1'}
 """
         OutputCapture().assert_outputs(self, step.run, [{}], expected_logs=expected_logs)