Skip to content

Commit

Permalink
chore: fix linking with component ffmpeg (electron#35178)
Browse files Browse the repository at this point in the history
  • Loading branch information
zcbenz authored and schetle committed Aug 18, 2022
1 parent 3ea9a35 commit a8e8412
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 53 deletions.
7 changes: 7 additions & 0 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,13 @@ if (is_mac) {
"@executable_path/../../../../../..",
]
}

# For component ffmpeg under non-component build, it is linked from
# @loader_path. However the ffmpeg.dylib is moved to a different place
# when generating app bundle, and we should change to link from @rpath.
if (is_component_ffmpeg && !is_component_build) {
ldflags += [ "-Wcrl,installnametool,-change,@loader_path/libffmpeg.dylib,@rpath/libffmpeg.dylib" ]
}
}

template("electron_helper_app") {
Expand Down
20 changes: 0 additions & 20 deletions build/config/BUILD.gn
Original file line number Diff line number Diff line change
@@ -1,23 +1,3 @@
config("build_time_executable") {
configs = []

if (is_electron_build && !is_component_build) {
# The executables which have this config applied are dependent on ffmpeg,
# which is always a shared library in an Electron build. However, in the
# non-component build, executables don't have rpath set to search for
# libraries in the executable's directory, so ffmpeg cannot be found. So
# let's make sure rpath is set here.
# See '//build/config/gcc/BUILD.gn' for details on the rpath setting.
if (is_linux) {
configs += [ "//build/config/gcc:rpath_for_built_shared_libraries" ]
}

if (is_mac) {
ldflags = [ "-Wl,-rpath,@loader_path/." ]
}
}
}

# For MAS build, we force defining "MAS_BUILD".
config("mas_build") {
if (is_mas_build) {
Expand Down
1 change: 0 additions & 1 deletion patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ webview_cross_drag.patch
gin_enable_disable_v8_platform.patch
disable-redraw-lock.patch
enable_reset_aspect_ratio.patch
v8_context_snapshot_generator.patch
boringssl_build_gn.patch
pepper_plugin_support.patch
gtk_visibility.patch
Expand Down
20 changes: 0 additions & 20 deletions patches/chromium/v8_context_snapshot_generator.patch

This file was deleted.

2 changes: 2 additions & 0 deletions patches/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

"src/electron/patches/devtools_frontend": "src/third_party/devtools-frontend/src",

"src/electron/patches/ffmpeg": "src/third_party/ffmpeg",

"src/electron/patches/webrtc": "src/third_party/webrtc",

"src/electron/patches/v8": "src/v8",
Expand Down
1 change: 1 addition & 0 deletions patches/ffmpeg/.patches
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
link_with_loader_path.patch
29 changes: 29 additions & 0 deletions patches/ffmpeg/link_with_loader_path.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Tue, 2 Aug 2022 11:53:00 +0900
Subject: fix: link with @loader_path/libffmpeg.dylib

Submitted to https://chromium-review.googlesource.com/c/chromium/third_party/ffmpeg/+/3803946.

When building with `is_component_build=false is_component_ffmpeg=true`,
we must manually instruct executables to link with the ffmpeg.dylib,
which is generated at the @loader_path, where for most targets is the
out/{Release,Debug} dir.

Using @rpath is wrong because the @rpath of most targets does not
include the out dir, and the linker won't be able to find ffmpeg.dylib
because of so.

diff --git a/BUILD.gn b/BUILD.gn
index 5ea4e373360f1e09d8295be89beb063f482ec165..4fd65674d1a2a94868de6aa32bdde04a4b9c2662 100755
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -448,7 +448,7 @@ if (is_component_ffmpeg) {

if (!is_component_build) {
if (is_mac) {
- ldflags += [ "-Wl,-install_name,@rpath/libffmpeg.dylib" ]
+ ldflags += [ "-Wl,-install_name,@loader_path/libffmpeg.dylib" ]
} else if (is_linux) {
all_dependent_configs = [
"//build/config/gcc:rpath_for_built_shared_libraries",
11 changes: 1 addition & 10 deletions patches/v8/build_gn.patch
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ necessary for native modules to load.
Also, some fixes relating to mksnapshot on ARM.

diff --git a/BUILD.gn b/BUILD.gn
index c918081b31fdf15efd325ae9d688f6c4f59aded1..3820a03365e58a05f2df16195ed6061f1c451a05 100644
index c918081b31fdf15efd325ae9d688f6c4f59aded1..15d0dcf9f6ea151f4dcacff0f3dc11b318a1e1cd 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -636,7 +636,7 @@ config("internal_config") {
Expand All @@ -30,12 +30,3 @@ index c918081b31fdf15efd325ae9d688f6c4f59aded1..3820a03365e58a05f2df16195ed6061f

deps = [
":v8_libbase",
@@ -6037,6 +6037,8 @@ if (current_toolchain == v8_snapshot_toolchain) {

configs = [ ":internal_config" ]

+ configs += [ "//electron/build/config:build_time_executable" ]
+
deps = [
":v8_base_without_compiler",
":v8_compiler_for_mksnapshot",
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ This patch can be safely removed if, when it is removed, `node.lib` does not
contain any standard C++ library exports (e.g. `std::ostringstream`).

diff --git a/BUILD.gn b/BUILD.gn
index b6a232081301a74ffbffb98311fc29c4015fc9f9..f5d605043e7f43324fd0bdf74f1decf634143cb0 100644
index 88d775bfe22d6f1ff738120d11cb1123c5ac106c..feab71df47427d6aaebddb581762fa1a2fd16887 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -636,6 +636,10 @@ config("internal_config") {
Expand Down
2 changes: 1 addition & 1 deletion patches/v8/expose_mksnapshot.patch
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Subject: expose_mksnapshot.patch
Needed in order to target mksnapshot for mksnapshot zip.

diff --git a/BUILD.gn b/BUILD.gn
index 3820a03365e58a05f2df16195ed6061f1c451a05..b6a232081301a74ffbffb98311fc29c4015fc9f9 100644
index 15d0dcf9f6ea151f4dcacff0f3dc11b318a1e1cd..88d775bfe22d6f1ff738120d11cb1123c5ac106c 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -6011,7 +6011,6 @@ if (current_toolchain == v8_generator_toolchain) {
Expand Down

0 comments on commit a8e8412

Please sign in to comment.