Merge pull request #29262 from Rostepher/migrate-migration-module

[NFC][Build System: build-script] Move the migration module.

Author: Rostepher

utils/build-script

@@ -21,14 +21,13 @@ import time
 
 from build_swift.build_swift import defaults
 from build_swift.build_swift import driver_arguments
+from build_swift.build_swift import migration
 from build_swift.build_swift import presets
-from build_swift.build_swift.migration import migrate_swift_sdks
 
 from swift_build_support.swift_build_support import (
     arguments,
     debug,
     diagnostics,
-    migration,
     products,
     shell,
     tar,
@@ -991,7 +990,7 @@ def main_preset():
     except presets.Error as e:
         diagnostics.fatal(e.message)
 
-    preset_args = migrate_swift_sdks(preset.format_args())
+    preset_args = migration.migrate_swift_sdks(preset.format_args())
 
     if args.distcc and (args.cmake_c_launcher or args.cmake_cxx_launcher):
         diagnostics.fatal(

utils/build_swift/build_swift/migration.py

@@ -11,9 +11,11 @@
 Temporary module with functionaly used to migrate away from build-script-impl.
 """
 
+
 from __future__ import absolute_import, unicode_literals
 
 import itertools
+import subprocess
 
 from swift_build_support.swift_build_support.targets import \
     StdlibDeploymentTarget
@@ -26,9 +28,19 @@
     imap = map
 
 
+try:
+    # Python 2
+    unicode
+except NameError:
+    unicode = str
+
+
 __all__ = [
     'UnknownSDKError',
+
+    'check_impl_args',
     'migrate_swift_sdks',
+    'parse_args',
 ]
 
 
@@ -88,3 +100,63 @@ def _migrate_swift_sdks_arg(arg):
         return '--stdlib-deployment-targets={}'.format(' '.join(target_names))
 
     return list(imap(_migrate_swift_sdks_arg, args))
+
+
+# -----------------------------------------------------------------------------
+
+def _process_disambiguation_arguments(args, unknown_args):
+    """These arguments are only listed in the driver arguments to stop argparse
+    from auto expanding arguments like --install-swift to the known argument
+    --install-swiftevolve. Remove them from args and add them to unknown_args
+    again.
+    """
+
+    if hasattr(args, 'impl_skip_test_swift'):
+        if args.impl_skip_test_swift:
+            unknown_args.append('--skip-test-swift')
+        del args.impl_skip_test_swift
+
+    if hasattr(args, 'impl_install_swift'):
+        if args.impl_install_swift:
+            unknown_args.append('--install-swift')
+        del args.impl_install_swift
+
+    return args, unknown_args
+
+
+def parse_args(parser, args, namespace=None):
+    """Parses a given argument list with the given argparse.ArgumentParser.
+
+    Return a processed arguments object. Any unknown arguments are stored in
+    `build_script_impl_args` attribute as a list. Ignores '--' to be compatible
+    with old style argument list.
+    """
+
+    args = [arg for arg in args if arg != '--']
+    args, unknown_args = parser.parse_known_args(args, namespace)
+    args, unknown_args = _process_disambiguation_arguments(args, unknown_args)
+
+    args.build_script_impl_args = unknown_args
+
+    return args
+
+
+# -----------------------------------------------------------------------------
+
+def check_impl_args(build_script_impl, args):
+    """Check whether given argv are all known arguments for
+    `build-script-impl`.
+
+    Raises a ValueError if any invalid argument is found. Return nothing
+    otherwise.
+    """
+
+    pipe = subprocess.Popen(
+        [build_script_impl, '--check-args-only=1'] + args,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE)
+
+    _, err = pipe.communicate()
+
+    if pipe.returncode != 0:
+        raise ValueError(unicode(err.splitlines()[0].decode()))

utils/build_swift/tests/test_driver_arguments.py

@@ -14,22 +14,16 @@
 from contextlib import contextmanager
 from io import StringIO
 
-from swift_build_support.swift_build_support import migration
 from swift_build_support.swift_build_support.SwiftBuildSupport import (
     get_all_preset_names, get_preset_options)
 
 from . import expected_options as eo
+from .utils import BUILD_SCRIPT_IMPL_PATH, UTILS_PATH
 from ..build_swift import argparse
 from ..build_swift import driver_arguments
+from ..build_swift import migration
 
 
-FILE_PATH = os.path.abspath(__file__)
-TESTS_PATH = os.path.abspath(os.path.join(FILE_PATH, os.pardir))
-BUILD_SWIFT_PATH = os.path.abspath(os.path.join(TESTS_PATH, os.pardir))
-UTILS_PATH = os.path.abspath(os.path.join(BUILD_SWIFT_PATH, os.pardir))
-
-BUILD_SCRIPT_IMPL = os.path.join(UTILS_PATH, 'build-script-impl')
-
 PRESETS_FILES = [
     os.path.join(UTILS_PATH, 'build-presets.ini'),
 ]
@@ -394,7 +388,7 @@ def _check_impl_args(self, namespace):
         assert hasattr(namespace, 'build_script_impl_args')
 
         try:
-            migration.check_impl_args(BUILD_SCRIPT_IMPL,
+            migration.check_impl_args(BUILD_SCRIPT_IMPL_PATH,
                                       namespace.build_script_impl_args)
         except (SystemExit, ValueError) as e:
             raise ParserError('failed to parse impl arguments: ' +
@@ -406,12 +400,12 @@ def parse_args_and_unknown_args(self, args, namespace=None):
 
         with self._quiet_output():
             try:
-                namespace, unknown_args =\
-                    super(self.parser.__class__, self.parser)\
-                    .parse_known_args(args, namespace)
-                namespace, unknown_args =\
-                    migration.process_disambiguation_arguments(namespace,
-                                                               unknown_args)
+                namespace, unknown_args = (
+                    super(self.parser.__class__, self.parser).parse_known_args(
+                        args, namespace))
+                namespace, unknown_args = (
+                    migration._process_disambiguation_arguments(
+                        namespace, unknown_args))
             except (SystemExit, argparse.ArgumentError) as e:
                 raise ParserError('failed to parse arguments: ' + str(args), e)
 

utils/build_swift/tests/test_migration.py

@@ -9,10 +9,16 @@
 
 from __future__ import absolute_import, unicode_literals
 
-from .utils import TestCase, add_metaclass
+import platform
+
+from .utils import BUILD_SCRIPT_IMPL_PATH, TestCase, add_metaclass
+from ..build_swift import argparse
 from ..build_swift import migration
 
 
+# -----------------------------------------------------------------------------
+# Helpers
+
 def _get_sdk_targets(sdk_names):
     targets = []
     for sdk_name in sdk_names:
@@ -26,6 +32,7 @@ def _get_sdk_target_names(sdk_names):
 
 
 # -----------------------------------------------------------------------------
+# Mirgrate Swift SDKs
 
 class TestMigrateSwiftSDKsMeta(type):
     """Metaclass used to dynamically generate test methods.
@@ -76,3 +83,88 @@ def test_multiple_swift_sdk_flags(self):
             '--stdlib-deployment-targets=',
             '--stdlib-deployment-targets={}'.format(' '.join(target_names))
         ])
+
+
+# -----------------------------------------------------------------------------
+
+class TestMigrateParseArgs(TestCase):
+
+    def test_report_unknown_args(self):
+        parser = argparse.ArgumentParser()
+        parser.add_argument('-R', '--release', action='store_true')
+        parser.add_argument('-T', '--validation-test', action='store_true')
+        parser.add_argument('--darwin-xcrun-toolchain')
+
+        args = migration.parse_args(parser, [
+            '-RT',
+            '--unknown', 'true',
+            '--darwin-xcrun-toolchain=foo',
+            '--',
+            '--darwin-xcrun-toolchain=bar',
+            '--other',
+        ])
+
+        expected = argparse.Namespace(
+            release=True,
+            validation_test=True,
+            darwin_xcrun_toolchain='bar',
+            build_script_impl_args=['--unknown', 'true', '--other'])
+
+        self.assertEqual(args, expected)
+
+    def test_no_unknown_args(self):
+        parser = argparse.ArgumentParser()
+        parser.add_argument('-R', '--release', action='store_true')
+        parser.add_argument('-T', '--validation-test', action='store_true')
+        parser.add_argument('--darwin-xcrun-toolchain')
+
+        args = migration.parse_args(
+            parser, ['-RT', '--darwin-xcrun-toolchain=bar'])
+
+        expected = argparse.Namespace(
+            release=True,
+            validation_test=True,
+            darwin_xcrun_toolchain='bar',
+            build_script_impl_args=[])
+
+        self.assertEqual(args, expected)
+
+    def test_forward_impl_args(self):
+        parser = argparse.ArgumentParser()
+        parser.add_argument('--skip-test-swift',
+                            dest='impl_skip_test_swift',
+                            action='store_true')
+        parser.add_argument('--install-swift',
+                            dest='impl_install_swift',
+                            action='store_true')
+
+        args = migration.parse_args(
+            parser, ['--skip-test-swift', '--install-swift'])
+
+        expected = argparse.Namespace(
+            build_script_impl_args=['--skip-test-swift', '--install-swift'])
+
+        self.assertEqual(args, expected)
+
+
+class TestMigrationCheckImplArgs(TestCase):
+
+    def test_check_impl_args(self):
+        if platform.system() == 'Windows':
+            self.skipTest("build-script-impl cannot run in Windows")
+            return
+
+        self.assertIsNone(migration.check_impl_args(
+            BUILD_SCRIPT_IMPL_PATH, ['--reconfigure']))
+
+        with self.assertRaises(ValueError) as cm:
+            migration.check_impl_args(
+                BUILD_SCRIPT_IMPL_PATH, ['foo'])
+
+        self.assertIn('foo', str(cm.exception))
+
+        with self.assertRaises(ValueError) as cm:
+            migration.check_impl_args(
+                BUILD_SCRIPT_IMPL_PATH, ['--reconfigure', '--foo=true'])
+
+        self.assertIn('foo', str(cm.exception))

utils/build_swift/tests/utils.py

@@ -7,32 +7,39 @@
 # See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
 
-try:
-    from StringIO import StringIO
-except ImportError:
-    from io import StringIO
-
 import os
 import sys
 import unittest
 from contextlib import contextmanager
 
 
+try:
+    from StringIO import StringIO
+except ImportError:
+    from io import StringIO
+
+
 __all__ = [
     'add_metaclass',
     'redirect_stderr',
     'redirect_stdout',
     'TestCase',
 
+    'BUILD_SCRIPT_IMPL_PATH',
+    'BUILD_SWIFT_PATH',
+    'TESTS_PATH',
     'UTILS_PATH',
 ]
 
 
-UTILS_PATH = os.path.abspath(os.path.join(
-    os.path.dirname(__file__),
-    os.pardir,
-    os.pardir,
-))
+# -----------------------------------------------------------------------------
+# Constants
+
+TESTS_PATH = os.path.abspath(os.path.dirname(__file__))
+BUILD_SWIFT_PATH = os.path.abspath(os.path.join(TESTS_PATH, os.pardir))
+UTILS_PATH = os.path.abspath(os.path.join(BUILD_SWIFT_PATH, os.pardir))
+
+BUILD_SCRIPT_IMPL_PATH = os.path.join(UTILS_PATH, 'build-script-impl')
 
 
 # -----------------------------------------------------------------------------

utils/swift_build_support/swift_build_support/__init__.py

@@ -22,7 +22,6 @@
     "debug",
     "diagnostics",
     "host_specific_configuration",
-    "migration",
     "tar",
     "targets",
     "toolchain",