Repository: servo/servo
File: python/servo/build_commands.py (Python)

Code before:
        self.ensure_clobbered()

        host = servo.platform.host_triple()
        target_triple = self.target.triple()

        if self.enable_code_coverage:
            print("Building with code coverage instrumentation...")
            # We don't want coverage for build-scripts and proc macros.
            kwargs["target_override"] = target_triple
            this_dir = pathlib.Path(os.path.dirname(__file__))
            servo_root_dir = this_dir.parent.parent
            coverage_workspace_wrapper = servo_root_dir.joinpath("etc/coverage_workspace_wrapper.py")
            if not coverage_workspace_wrapper.exists():
                print(
                    f"Could not find rustc workspace wrapper script at expected location: {coverage_workspace_wrapper}"
                )
            env["RUSTC_WORKSPACE_WRAPPER"] = str(coverage_workspace_wrapper)

        if sanitizer.is_some():
            self.build_sanitizer_env(env, opts, kwargs, target_triple, sanitizer)

        if build_type.is_dev():
            # Increase stylo thread stack size to 2 MiB for debug builds since the stack usage is higher
            # and crashes have been reported. The default is 512 KiB.
            # Note: This is placed after `build_sanitizer_env()` so that the higher stack size with ASAN takes
            # priority over the debug build stack size, which matters for ASAN builds with the debug profile.
            env["SERVO_STYLE_THREAD_STACK_SIZE_KB"] = env.get("SERVO_STYLE_THREAD_STACK_SIZE_KB", str(2 * 1024))
        build_start = time()

        if host != target_triple and "windows" in target_triple:
            if os.environ.get("VisualStudioVersion") or os.environ.get("VCINSTALLDIR"):
                print(
                    "Can't cross-compile for Windows inside of a Visual Studio shell.\n"
                    "Please run `python mach build [arguments]` to bypass automatic "
                    "Visual Studio shell, and make sure the VisualStudioVersion and "
                    "VCINSTALLDIR environment variables are not set."
                )
                sys.exit(1)

        # Gather Cargo build timings (https://doc.rust-lang.org/cargo/reference/timings.html).
        opts = ["--timings"] + opts

        crown_enabled = "enabled" if kwargs.get("use_crown", False) else "disabled (no JS garbage collection linting)"
        print(f"Building `{build_type.directory_name()}` build with crown {crown_enabled}.")
        if very_verbose:
            for key in env:
                print((key, env[key]))

        status = self.run_cargo_build_like_command("rustc", opts, env=env, verbose=verbose, **kwargs)

        if status == 0:

Review comment (security):
We should be setting all environment variables in one place instead of many places. I think you can do this by passing in an optional sanitizer argument to `build_env()` and calling `build_sanitizer_env` from there. Sorry that I have not spotted this earlier.

Setting up the environment in one place makes it easier understand where environment variables get set.

Code after:
        self.ensure_clobbered()

        host = servo.platform.host_triple()
        target_triple = self.target.triple()

        if self.enable_code_coverage:
            print("Building with code coverage instrumentation...")
            # We don't want coverage for build-scripts and proc macros.
            kwargs["target_override"] = target_triple
            this_dir = pathlib.Path(os.path.dirname(__file__))
            servo_root_dir = this_dir.parent.parent
            coverage_workspace_wrapper = servo_root_dir.joinpath("etc/coverage_workspace_wrapper.py")
            if not coverage_workspace_wrapper.exists():
                print(
                    f"Could not find rustc workspace wrapper script at expected location: {coverage_workspace_wrapper}"
                )
            env["RUSTC_WORKSPACE_WRAPPER"] = str(coverage_workspace_wrapper)

        if sanitizer.is_some():
            self.build_sanitizer_env(env, opts, kwargs, target_triple, sanitizer)
        build_start = time()

        if host != target_triple and "windows" in target_triple:
            if os.environ.get("VisualStudioVersion") or os.environ.get("VCINSTALLDIR"):
                print(
                    "Can't cross-compile for Windows inside of a Visual Studio shell.\n"
                    "Please run `python mach build [arguments]` to bypass automatic "
                    "Visual Studio shell, and make sure the VisualStudioVersion and "
                    "VCINSTALLDIR environment variables are not set."
                )
                sys.exit(1)

        # Gather Cargo build timings (https://doc.rust-lang.org/cargo/reference/timings.html).
        opts = ["--timings"] + opts

        crown_enabled = "enabled" if kwargs.get("use_crown", False) else "disabled (no JS garbage collection linting)"
        print(f"Building `{build_type.directory_name()}` build with crown {crown_enabled}.")
        if very_verbose:
            for key in env:
                print((key, env[key]))

        status = self.run_cargo_build_like_command("rustc", opts, env=env, verbose=verbose, **kwargs)

        if status == 0:
            if not no_package and self.target.needs_packaging():
                return_value = Registrar.dispatch(
                    "package", context=self.context, build_type=build_type, flavor=flavor, sanitizer=sanitizer
                )
                if return_value:
                    return return_value