Skip to content

Commit

Permalink
Fix the build on Ubuntu 20.04 (servo#31019)
Browse files Browse the repository at this point in the history
Ubuntu 20.04 doesn't have a new enough version of GStreamer, so
automatically disable media when running on that platform.

This also cleans up the media detection a bit, putting the result in a
`enable-media` variable and moving some of the logic into the build
scripts themselves rather than the platform module.
  • Loading branch information
mrobinson authored and mukilan committed Jan 31, 2024
1 parent 7fa4ea9 commit 98bea3e
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 45 deletions.
7 changes: 3 additions & 4 deletions python/servo/build_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ class MachCommands(CommandBase):
def build(self, build_type: BuildType, jobs=None, params=None, no_package=False,
verbose=False, very_verbose=False, libsimpleservo=False, **kwargs):
opts = params or []
has_media_stack = "media-gstreamer" in self.features

if build_type.is_release():
opts += ["--release"]
Expand All @@ -74,7 +73,7 @@ def build(self, build_type: BuildType, jobs=None, params=None, no_package=False,
if very_verbose:
opts += ["-vv"]

env = self.build_env(is_build=True)
env = self.build_env()
self.ensure_bootstrapped()
self.ensure_clobbered()

Expand Down Expand Up @@ -173,7 +172,7 @@ def package_generated_shared_libraries(libs, build_path, servo_exe_dir):
status = 1

# copy needed gstreamer DLLs in to servo.exe dir
if has_media_stack:
if self.enable_media:
print("Packaging gstreamer DLLs")
if not package_gstreamer_dlls(env, servo_exe_dir, target_triple):
status = 1
Expand All @@ -189,7 +188,7 @@ def package_generated_shared_libraries(libs, build_path, servo_exe_dir):
servo_bin_dir = os.path.dirname(servo_path)
assert os.path.exists(servo_bin_dir)

if has_media_stack:
if self.enable_media:
print("Packaging gstreamer dylibs")
if not package_gstreamer_dylibs(self.cross_compile_target, servo_path):
return 1
Expand Down
42 changes: 32 additions & 10 deletions python/servo/command_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ class CommandBase(object):

def __init__(self, context):
self.context = context
self.enable_media = False
self.features = []
self.cross_compile_target = None
self.is_android_build = False
Expand Down Expand Up @@ -493,13 +494,13 @@ def vs_dirs(self):
'vcdir': vcinstalldir,
}

def build_env(self, is_build=False):
def build_env(self):
"""Return an extended environment dictionary."""
env = os.environ.copy()

servo.platform.get().set_gstreamer_environment_variables_if_necessary(
env, cross_compilation_target=self.cross_compile_target,
check_installation=is_build)
if self.enable_media and not self.is_android_build:
servo.platform.get().set_gstreamer_environment_variables_if_necessary(
env, cross_compilation_target=self.cross_compile_target)

effective_target = self.cross_compile_target or servo.platform.host_triple()
if "msvc" in effective_target:
Expand Down Expand Up @@ -806,7 +807,7 @@ def configuration_decorator(self, *args, **kwargs):
if build_configuration:
self.configure_cross_compilation(kwargs['target'], kwargs['android'], kwargs['win_arm64'])
self.features = kwargs.get("features", None) or []
self.configure_media_stack(kwargs['media_stack'])
self.enable_media = self.is_media_enabled(kwargs['media_stack'])

return original_function(self, *args, **kwargs)

Expand Down Expand Up @@ -874,10 +875,10 @@ def configure_cross_compilation(
if self.cross_compile_target:
print(f"Targeting '{self.cross_compile_target}' for cross-compilation")

def configure_media_stack(self, media_stack: Optional[str]):
"""Determine what media stack to use based on the value of the build target
def is_media_enabled(self, media_stack: Optional[str]):
"""Determine whether media is enabled based on the value of the build target
platform and the value of the '--media-stack' command-line argument.
The chosen media stack is written into the `features` instance variable."""
Returns true if media is enabled."""
if not media_stack:
if self.config["build"]["media-stack"] != "auto":
media_stack = self.config["build"]["media-stack"]
Expand All @@ -890,8 +891,16 @@ def configure_media_stack(self, media_stack: Optional[str]):
media_stack = "gstreamer"
else:
media_stack = "dummy"
if media_stack != "dummy":
self.features += ["media-" + media_stack]

# This is a workaround for Ubuntu 20.04, which doesn't support a new enough GStreamer.
# Once we drop support for this platform (it's currently needed for wpt.fyi runners),
# we can remove this workaround and officially only support Ubuntu 22.04 and up.
platform = servo.platform.get()
if not self.cross_compile_target and platform.is_linux and \
not platform.is_gstreamer_installed(self.cross_compile_target):
return False

return media_stack != "dummy"

def run_cargo_build_like_command(
self, command: str, cargo_args: List[str],
Expand All @@ -903,6 +912,17 @@ def run_cargo_build_like_command(
):
env = env or self.build_env()

# Android GStreamer integration is handled elsewhere.
# NB: On non-Linux platforms we cannot check whether GStreamer is installed until
# environment variables are set via `self.build_env()`.
platform = servo.platform.get()
if self.enable_media and not self.is_android_build and \
not platform.is_gstreamer_installed(self.cross_compile_target):
raise FileNotFoundError(
"GStreamer libraries not found (>= version 1.18)."
"Please see installation instructions in README.md"
)

args = []
if "--manifest-path" not in cargo_args:
if libsimpleservo or self.is_android_build:
Expand All @@ -922,6 +942,8 @@ def run_cargo_build_like_command(

if "-p" not in cargo_args: # We're building specific package, that may not have features
features = list(self.features)
if self.enable_media:
features.append("media-gstreamer")
if self.config["build"]["debug-mozjs"] or debug_mozjs:
features.append("debugmozjs")

Expand Down
13 changes: 0 additions & 13 deletions python/servo/platform/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ def __init__(self, triple: str):
def set_gstreamer_environment_variables_if_necessary(
self, env: Dict[str, str], cross_compilation_target: Optional[str], check_installation=True
):
# Environment variables are not needed when cross-compiling on any platform other
# than Windows. GStreamer for Android is handled elsewhere.
if cross_compilation_target and (not self.is_windows or "android" in cross_compilation_target):
return

# We may not need to update environment variables if GStreamer is installed
# for the system on Linux.
gstreamer_root = self.gstreamer_root(cross_compilation_target)
Expand All @@ -52,14 +47,6 @@ def set_gstreamer_environment_variables_if_necessary(
)
env["GST_PLUGIN_SYSTEM_PATH"] = os.path.join(gstreamer_root, "lib", "gstreamer-1.0")

# If we are not cross-compiling GStreamer must be installed for the system. In
# the cross-compilation case, we might be picking it up from another directory.
if check_installation and not self.is_gstreamer_installed(cross_compilation_target):
raise FileNotFoundError(
"GStreamer libraries not found (>= version 1.18)."
"Please see installation instructions in README.md"
)

def gstreamer_root(self, _cross_compilation_target: Optional[str]) -> Optional[str]:
raise NotImplementedError("Do not know how to get GStreamer path for platform.")

Expand Down
15 changes: 1 addition & 14 deletions python/servo/platform/linux.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from typing import Optional, Tuple

import distro
from .. import util
from .base import Base

# Please keep these in sync with the packages on the wiki, using the instructions below
Expand Down Expand Up @@ -75,8 +74,6 @@

GSTREAMER_URL = \
"https://github.com/servo/servo-build-deps/releases/download/linux/gstreamer-1.16-x86_64-linux-gnu.20190515.tar.gz"
PREPACKAGED_GSTREAMER_ROOT = \
os.path.join(util.get_target_dir(), "dependencies", "gstreamer")


class Linux(Base):
Expand Down Expand Up @@ -154,7 +151,6 @@ def _platform_bootstrap(self, force: bool) -> bool:
f"{self.distro}, please file a bug")

installed_something = self.install_non_gstreamer_dependencies(force)
installed_something |= self._platform_bootstrap_gstreamer(force)
return installed_something

def install_non_gstreamer_dependencies(self, force: bool) -> bool:
Expand Down Expand Up @@ -199,18 +195,9 @@ def run_as_root(command, force=False):
return True

def gstreamer_root(self, cross_compilation_target: Optional[str]) -> Optional[str]:
if cross_compilation_target:
return None
if os.path.exists(PREPACKAGED_GSTREAMER_ROOT):
return PREPACKAGED_GSTREAMER_ROOT
# GStreamer might be installed system-wide, but we do not return a root in this
# case because we don't have to update environment variables.
return None

def _platform_bootstrap_gstreamer(self, force: bool) -> bool:
if not force and self.is_gstreamer_installed(cross_compilation_target=None):
return False

def _platform_bootstrap_gstreamer(self, _force: bool) -> bool:
raise EnvironmentError(
"Bootstrapping GStreamer on Linux is not supported. "
+ "Please install it using your distribution package manager.")
2 changes: 1 addition & 1 deletion python/servo/post_build_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def doc(self, params: List[str], **kwargs):
else:
copy2(full_name, destination)

env = self.build_env(is_build=True)
env = self.build_env()
returncode = self.run_cargo_build_like_command("doc", params, env=env, **kwargs)
if returncode:
return returncode
Expand Down
4 changes: 1 addition & 3 deletions python/servo/testing_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,7 @@ def test_unit(self, build_type: BuildType, test_name=None, package=None, bench=F
if nocapture:
args += ["--", "--nocapture"]

# We are setting is_build here to true, because running `cargo test` can trigger builds.
env = self.build_env(is_build=True)

env = self.build_env()
return self.run_cargo_build_like_command(
"bench" if bench else "test",
args,
Expand Down

0 comments on commit 98bea3e

Please sign in to comment.