Share more `./mach build` logic with mach check, doc, test-unit

Fixes #23659

5 files changed, 147 insertions, 132 deletions

diff --git a/python/servo/build_commands.py b/python/servo/build_commands.py
index 6dd5a608de9..3937b73efd0 100644
--- a/python/servo/build_commands.py
+++ b/python/servo/build_commands.py
@@ -146,9 +146,6 @@ class MachCommands(CommandBase):
     @Command('build',
              description='Build Servo',
              category='build')
-    @CommandArgument('--target', '-t',
-                     default=None,
-                     help='Cross compile for given target platform')
     @CommandArgument('--release', '-r',
                      action='store_true',
                      help='Build in release mode')
@@ -158,25 +155,9 @@ class MachCommands(CommandBase):
     @CommandArgument('--jobs', '-j',
                      default=None,
                      help='Number of jobs to run in parallel')
-    @CommandArgument('--features',
-                     default=None,
-                     help='Space-separated list of features to also build',
-                     nargs='+')
-    @CommandArgument('--android',
-                     default=None,
-                     action='store_true',
-                     help='Build for Android')
-    @CommandArgument('--magicleap',
-                     default=None,
-                     action='store_true',
-                     help='Build for Magic Leap')
     @CommandArgument('--no-package',
                      action='store_true',
                      help='For Android, disable packaging into a .apk after building')
-    @CommandArgument('--debug-mozjs',
-                     default=None,
-                     action='store_true',
-                     help='Enable debug assertions in mozjs')
     @CommandArgument('--verbose', '-v',
                      action='store_true',
                      help='Print verbose output')
@@ -185,46 +166,14 @@ class MachCommands(CommandBase):
                      help='Print very verbose output')
     @CommandArgument('params', nargs='...',
                      help="Command-line arguments to be passed through to Cargo")
-    @CommandArgument('--with-debug-assertions',
-                     default=None,
-                     action='store_true',
-                     help='Enable debug assertions in release')
-    @CommandArgument('--libsimpleservo',
-                     default=None,
-                     action='store_true',
-                     help='Build the libsimpleservo library instead of the servo executable')
-    @CommandArgument('--with-frame-pointer',
-                     default=None,
-                     action='store_true',
-                     help='Build with frame pointer enabled, used by the background hang monitor.')
-    @CommandArgument('--with-raqote', default=None, action='store_true')
-    @CommandArgument('--without-wgl', default=None, action='store_true')
-    def build(self, target=None, release=False, dev=False, jobs=None,
-              features=None, android=None, magicleap=None, no_package=False, verbose=False, very_verbose=False,
-              debug_mozjs=False, params=None, with_debug_assertions=False,
-              libsimpleservo=False, with_frame_pointer=False, with_raqote=False, without_wgl=False):
-
+    @CommandBase.build_like_command_arguments
+    def build(self, release=False, dev=False, jobs=None, params=None,
+              no_package=False, verbose=False, very_verbose=False,
+              target=None, android=False, magicleap=False, libsimpleservo=False,
+              features=None, **kwargs):
         opts = params or []
-
-        if android is None:
-            android = self.config["build"]["android"]
-        features = features or self.servo_features()
-
-        if target and android:
-            print("Please specify either --target or --android.")
-            sys.exit(1)
-
-        if android:
-            target = self.config["android"]["target"]
-
-        if not magicleap:
-            features += ["native-bluetooth"]
-
-        if magicleap and not target:
-            target = "aarch64-linux-android"
-
-        if target and not android and not magicleap:
-            android = self.handle_android_target(target)
+        features = features or []
+        target, android = self.pick_target_triple(target, android, magicleap)
 
         target_path = base_path = self.get_target_dir()
         if android:
@@ -278,44 +227,13 @@ class MachCommands(CommandBase):
                 check_call(["rustup" + BIN_SUFFIX, "target", "add",
                             "--toolchain", self.toolchain(), target])
 
-            opts += ["--target", target]
-
         env = self.build_env(target=target, is_build=True)
         self.ensure_bootstrapped(target=target)
         self.ensure_clobbered()
 
-        self.add_manifest_path(opts, android, libsimpleservo)
-
-        if debug_mozjs:
-            features += ["debugmozjs"]
-
-        if with_frame_pointer:
-            env['RUSTFLAGS'] = env.get('RUSTFLAGS', "") + " -C force-frame-pointers=yes"
-            features += ["profilemozjs"]
-
-        if with_raqote:
-            features += ["canvas2d-raqote"]
-
-        if without_wgl:
-            features += ["no_wgl"]
-
-        if self.config["build"]["webgl-backtrace"]:
-            features += ["webgl-backtrace"]
-        if self.config["build"]["dom-backtrace"]:
-            features += ["dom-backtrace"]
-
-        if "canvas2d-raqote" not in features:
-            features += ["canvas2d-azure"]
-
-        if features:
-            opts += ["--features", "%s" % ' '.join(features)]
-
         build_start = time()
         env["CARGO_TARGET_DIR"] = target_path
 
-        if with_debug_assertions:
-            env['RUSTFLAGS'] = env.get('RUSTFLAGS', "") + " -C debug_assertions"
-
         host = host_triple()
         if 'apple-darwin' in host and (not target or target == host):
             if 'CXXFLAGS' not in env:
@@ -612,7 +530,13 @@ class MachCommands(CommandBase):
             env.setdefault("CC", "clang")
             env.setdefault("CXX", "clang++")
 
-        status = self.call_rustup_run(["cargo", "build"] + opts, env=env, verbose=verbose)
+        status = self.run_cargo_build_like_command(
+            "build", opts, env=env, verbose=verbose,
+            target=target, android=android, magicleap=magicleap, libsimpleservo=libsimpleservo,
+            features=features, **kwargs
+        )
+        status = 0
+
         elapsed = time() - build_start
 
         # Do some additional things if the build succeeded
@@ -712,7 +636,7 @@ class MachCommands(CommandBase):
             print('Removing virtualenv directory: %s' % virtualenv_path)
             shutil.rmtree(virtualenv_path)
 
-        opts = ["--manifest-path", manifest_path or self.ports_glutin_manifest()]
+        opts = ["--manifest-path", manifest_path or path.join(self.context.topdir, "Cargo.toml")]
         if verbose:
             opts += ["-v"]
         opts += params
diff --git a/python/servo/command_base.py b/python/servo/command_base.py
index e05d37283cc..838e03f910c 100644
--- a/python/servo/command_base.py
+++ b/python/servo/command_base.py
@@ -28,6 +28,7 @@ from servo.util import download_file
 import urllib2
 from bootstrap import check_gstreamer_lib
 
+from mach.decorators import CommandArgument
 from mach.registrar import Registrar
 import toml
 
@@ -731,31 +732,130 @@ install them, let us know by filing a bug!")
 
         return env
 
-    def add_manifest_path(self, args, android=False, libsimpleservo=False):
+    @staticmethod
+    def build_like_command_arguments(decorated_function):
+        decorators = [
+            CommandArgument(
+                '--target', '-t',
+                default=None,
+                help='Cross compile for given target platform',
+            ),
+            CommandArgument(
+                '--android',
+                default=None,
+                action='store_true',
+                help='Build for Android',
+            ),
+            CommandArgument(
+                '--magicleap',
+                default=None,
+                action='store_true',
+                help='Build for Magic Leap',
+            ),
+            CommandArgument(
+                '--libsimpleservo',
+                default=None,
+                action='store_true',
+                help='Build the libsimpleservo library instead of the servo executable',
+            ),
+            CommandArgument(
+                '--features',
+                default=None,
+                help='Space-separated list of features to also build',
+                nargs='+',
+            ),
+            CommandArgument(
+                '--debug-mozjs',
+                default=None,
+                action='store_true',
+                help='Enable debug assertions in mozjs',
+            ),
+            CommandArgument(
+                '--with-debug-assertions',
+                default=None,
+                action='store_true',
+                help='Enable debug assertions in release',
+            ),
+            CommandArgument(
+                '--with-frame-pointer',
+                default=None,
+                action='store_true',
+                help='Build with frame pointer enabled, used by the background hang monitor.',
+            ),
+            CommandArgument('--with-raqote', default=None, action='store_true'),
+            CommandArgument('--without-wgl', default=None, action='store_true'),
+        ]
+
+        for decorator in decorators:
+            decorated_function = decorator(decorated_function)
+        return decorated_function
+
+    def pick_target_triple(self, target, android, magicleap):
+        if android is None:
+            android = self.config["build"]["android"]
+        if target and android:
+            assert self.handle_android_target(target)
+        if android and not target:
+            target = self.config["android"]["target"]
+        if magicleap and not target:
+            target = "aarch64-linux-android"
+        if target and not android and not magicleap:
+            android = self.handle_android_target(target)
+        return target, android
+
+    def run_cargo_build_like_command(
+        self, command, cargo_args,
+        env=None, verbose=False,
+        target=None, android=False, magicleap=False, libsimpleservo=False,
+        features=None, debug_mozjs=False, with_debug_assertions=False,
+        with_frame_pointer=False, with_raqote=False, without_wgl=False,
+    ):
+        env = env or self.build_env()
+        target, android = self.pick_target_triple(target, android, magicleap)
+
+        args = []
         if "--manifest-path" not in args:
             if libsimpleservo or android:
-                manifest = self.ports_libsimpleservo_manifest(android)
+                if android:
+                    api = "jniapi"
+                else:
+                    api = "capi"
+                port = path.join("libsimpleservo", api)
             else:
-                manifest = self.ports_glutin_manifest()
-            args.append("--manifest-path")
-            args.append(manifest)
-
-    def ports_glutin_manifest(self):
-        return path.join(self.context.topdir, "ports", "glutin", "Cargo.toml")
-
-    def ports_libsimpleservo_manifest(self, android=False):
-        if android:
-            api = "jniapi"
-        else:
-            api = "capi"
-        return path.join(self.context.topdir, "ports", "libsimpleservo", api, "Cargo.toml")
-
-    def servo_features(self):
-        """Return a list of optional features to enable for the Servo crate"""
-        features = []
-        if self.config["build"]["debug-mozjs"]:
-            features += ["debugmozjs"]
-        return features
+                port = "glutin"
+            args += [
+                "--manifest-path",
+                path.join(self.context.topdir, "ports", port, "Cargo.toml"),
+            ]
+        if target:
+            args += ["--target", target]
+
+        if features is None:  # If we're passed a list, mutate it even if it's empty
+            features = []
+        if self.config["build"]["debug-mozjs"] or debug_mozjs:
+            features.append("debugmozjs")
+        if not magicleap:
+            features.append("native-bluetooth")
+        if with_raqote and "canvas2d-azure" not in features:
+            features.append("canvas2d-raqote")
+        elif "canvas2d-raqote" not in features:
+            features.append("canvas2d-azure")
+        if with_frame_pointer:
+            env['RUSTFLAGS'] = env.get('RUSTFLAGS', "") + " -C force-frame-pointers=yes"
+            features.append("profilemozjs")
+        if without_wgl:
+            features.append("no_wgl")
+        if self.config["build"]["webgl-backtrace"]:
+            features.append("webgl-backtrace")
+        if self.config["build"]["dom-backtrace"]:
+            features.append("dom-backtrace")
+        if with_debug_assertions:
+            env['RUSTFLAGS'] = env.get('RUSTFLAGS', "") + " -C debug_assertions"
+
+        assert "--features" not in cargo_args
+        args += ["--features", " ".join(features)]
+
+        return self.call_rustup_run(["cargo", command] + args + cargo_args, env=env, verbose=verbose)
 
     def android_support_dir(self):
         return path.join(self.context.topdir, "support", "android")
diff --git a/python/servo/devenv_commands.py b/python/servo/devenv_commands.py
index 2545693207d..ad5a02c270d 100644
--- a/python/servo/devenv_commands.py
+++ b/python/servo/devenv_commands.py
@@ -37,7 +37,8 @@ class MachCommands(CommandBase):
     @CommandArgument(
         'params', default=None, nargs='...',
         help="Command-line arguments to be passed through to cargo check")
-    def check(self, params):
+    @CommandBase.build_like_command_arguments
+    def check(self, params, **kwargs):
         if not params:
             params = []
 
@@ -45,12 +46,8 @@ class MachCommands(CommandBase):
         self.ensure_clobbered()
         env = self.build_env()
 
-        params = ['check'] + params
-
-        self.add_manifest_path(params)
-
         build_start = time()
-        status = self.call_rustup_run(["cargo"] + params, env=env)
+        status = self.run_cargo_build_like_command("check", params, env=env, **kwargs)
         elapsed = time() - build_start
 
         notify_build_done(self.config, elapsed, status == 0)
diff --git a/python/servo/post_build_commands.py b/python/servo/post_build_commands.py
index 61a575b2db1..b9dc9b080ef 100644
--- a/python/servo/post_build_commands.py
+++ b/python/servo/post_build_commands.py
@@ -236,7 +236,8 @@ class PostBuildCommands(CommandBase):
     @CommandArgument(
         'params', nargs='...',
         help="Command-line arguments to be passed through to cargo doc")
-    def doc(self, params):
+    @CommandBase.build_like_command_arguments
+    def doc(self, params, **kwargs):
         env = os.environ.copy()
         env["RUSTUP_TOOLCHAIN"] = self.toolchain()
         rustc_path = check_output(["rustup" + BIN_SUFFIX, "which", "rustc"], env=env)
@@ -264,11 +265,7 @@ class PostBuildCommands(CommandBase):
                     else:
                         copy2(full_name, destination)
 
-        params += ["--features", "canvas2d-azure"]
-
-        returncode = self.call_rustup_run(
-            ["cargo", "doc", "--manifest-path", self.ports_glutin_manifest()] + params,
-            env=self.build_env())
+        returncode = self.run_cargo_build_like_command("doc", params, **kwargs)
         if returncode:
             return returncode
 
diff --git a/python/servo/testing_commands.py b/python/servo/testing_commands.py
index 8696f02f8dc..d1880f6385e 100644
--- a/python/servo/testing_commands.py
+++ b/python/servo/testing_commands.py
@@ -214,7 +214,8 @@ class MachCommands(CommandBase):
                      help="Run in bench mode")
     @CommandArgument('--nocapture', default=False, action="store_true",
                      help="Run tests with nocapture ( show test stdout )")
-    def test_unit(self, test_name=None, package=None, bench=False, nocapture=False):
+    @CommandBase.build_like_command_arguments
+    def test_unit(self, test_name=None, package=None, bench=False, nocapture=False, **kwargs):
         if test_name is None:
             test_name = []
 
@@ -277,22 +278,18 @@ class MachCommands(CommandBase):
             # in to the servo.exe build dir, so just point PATH to that.
             env["PATH"] = "%s%s%s" % (path.dirname(self.get_binary_path(False, False)), os.pathsep, env["PATH"])
 
-        features = self.servo_features()
         if len(packages) > 0 or len(in_crate_packages) > 0:
-            args = ["cargo", "bench" if bench else "test", "--manifest-path", self.ports_glutin_manifest()]
+            args = []
             for crate in packages:
                 args += ["-p", "%s_tests" % crate]
             for crate in in_crate_packages:
                 args += ["-p", crate]
             args += test_patterns
 
-            if features:
-                args += ["--features", "%s" % ' '.join(features)]
-
             if nocapture:
                 args += ["--", "--nocapture"]
 
-            err = self.call_rustup_run(args, env=env)
+            err = self.run_cargo_build_like_command("bench" if bench else "test", args, env=env, **kwargs)
             if err is not 0:
                 return err