Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(macos): fix boost on macos #2733

Merged
merged 4 commits into from
Jun 21, 2024
Merged

fix(macos): fix boost on macos #2733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5902e3c
fix(macos): fix boost on macos
Hazer Jun 20, 2024
7048881
chore: alphabetically sorted
Hazer Jun 20, 2024
0d1d9b4
chore: brew audit fixes
Hazer Jun 20, 2024
94afcef
chore: the joy of proper grammar
Hazer Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmake/compile_definitions/common.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ include_directories(
"${CMAKE_SOURCE_DIR}/third-party/moonlight-common-c/enet/include"
"${CMAKE_SOURCE_DIR}/third-party/nanors"
"${CMAKE_SOURCE_DIR}/third-party/nanors/deps/obl"
${Boost_INCLUDE_DIRS}
${FFMPEG_INCLUDE_DIRS}
${Boost_INCLUDE_DIRS} # has to be the last, or we get runtime error on macOS ffmpeg encoder
)

list(APPEND SUNSHINE_EXTERNAL_LIBRARIES
Expand Down
6 changes: 5 additions & 1 deletion cmake/prep/options.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ option(SUNSHINE_REQUIRE_TRAY "Require system tray icon. Fail the build if tray r

option(SUNSHINE_SYSTEM_WAYLAND_PROTOCOLS "Use system installation of wayland-protocols rather than the submodule." OFF)

option(BOOST_USE_STATIC "Use static boost libraries." ON)
if(APPLE)
option(BOOST_USE_STATIC "Use static boost libraries." OFF)
else()
option(BOOST_USE_STATIC "Use static boost libraries." ON)
endif()
Comment on lines +18 to +22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you still getting issues with BOOST_USE_STATIC=ON? If not we should revert this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without manually setting icu path, it won't build with ON, so to make easier to build locally for development, I think OFF is a good default for Clion starting setup. We could let be ON by default, but then we need to update the build docs with a step to set icu path and flags manually.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'm fine with it as is if we must.

Before we have different defaults though, could you investigate finding the icu path with CMake functions, such as find_package?

Copy link
Member Author

@Hazer Hazer Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried a bit, but had no success, as icu4c is a keg-only formula, it's not widely available in the system, I plan on finding a way to do that, but I'd rather fix the build first for most users, then improve it, since I don't know how long it will take me to do it and I won't be able to build the project for a few weeks, starting today.


option(CUDA_INHERIT_COMPILE_OPTIONS
"When building CUDA code, inherit compile options from the the main project. You may want to disable this if
Expand Down
3 changes: 2 additions & 1 deletion packaging/macos/Portfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ depends_lib port:avahi \
depends_test port:doxygen \
port:graphviz

configure.args -DBUILD_WERROR=ON \
configure.args -DBOOST_USE_STATIC=ON \
-DBUILD_WERROR=ON \
-DCMAKE_INSTALL_PREFIX=${prefix} \
-DSUNSHINE_ASSETS_DIR=etc/sunshine/assets

Expand Down
25 changes: 25 additions & 0 deletions packaging/sunshine.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ class @PROJECT_NAME@ < Formula
end
end

option "without-dynamic-boost", "Statically link Boost libraries" # default option
option "with-dynamic-boost", "Dynamically link Boost libraries"

depends_on "cmake" => :build
depends_on "node" => :build
depends_on "pkg-config" => :build
depends_on "curl"
depends_on "miniupnpc"
depends_on "openssl"
depends_on "opus"
depends_on "icu4c" => :recommended

on_linux do
depends_on "libcap"
Expand Down Expand Up @@ -64,6 +68,27 @@ def install
-DSUNSHINE_ENABLE_TRAY=OFF
-DTESTS_ENABLE_PYTHON_TESTS=OFF
]

if build.without? "dynamic-boost"
args << "-DBOOST_USE_STATIC=ON"
ohai "Statically linking Boost libraries"

unless Formula["icu4c"].any_version_installed?
odie <<~EOS
icu4c must be installed to link against static Boost libraries,
either install icu4c or use brew install sunshine --with-dynamic-boost instead
EOS
end
ENV.append "CXXFLAGS", "-I#{Formula["icu4c"].opt_include}"
icu4c_lib_path = Formula["icu4c"].opt_lib.to_s
ENV.append "LDFLAGS", "-L#{icu4c_lib_path}"
ENV["LIBRARY_PATH"] = icu4c_lib_path
ohai "Linking against ICU libraries at: #{icu4c_lib_path}"
else
args << "-DBOOST_USE_STATIC=OFF"
ohai "Dynamically linking Boost libraries"
end

system "cmake", "-S", ".", "-B", "build", *std_cmake_args, *args

cd "build" do
Expand Down
Loading