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

libx265: Disable assembly when building for Android #24627

Merged
merged 4 commits into from
Jul 17, 2024
Merged

libx265: Disable assembly when building for Android #24627

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85e64b6
Disable assembly when building Android
uilianries Jul 15, 2024
3fd7696
Raise invalid configuration for mac m1
uilianries Jul 16, 2024
ef6031a
update error message.
uilianries Jul 16, 2024
f3187d5
Add missing option
uilianries Jul 16, 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
31 changes: 26 additions & 5 deletions recipes/libx265/all/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def config_options(self):
del self.options.with_numa
# FIXME: Disable assembly by default if host is arm and compiler apple-clang for the moment.
# Indeed, apple-clang is not able to understand some asm instructions of libx265
if self.settings.compiler == "apple-clang" and "arm" in self.settings.arch:
# FIXME: Disable assembly by default if host is Android for the moment. It fails to build
if (self.settings.compiler == "apple-clang" and "arm" in self.settings.arch) or self.settings.os == "Android":
self.options.assembly = False
Copy link
Member

Choose a reason for hiding this comment

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

Is this even working? I've submitted conan-io/conan#16672 to show that changing values in config_options will have no effect if the user is setting the option in the CLI

Copy link
Member Author

Choose a reason for hiding this comment

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

If you change the option value in config_option, it will change the default value only. In case the user pass assembly=True, the validate_build will take care and raise and error. Indeed the user has priority over the value configured via config_options.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, the build log shows my command line, I didn't enforce the assembly option to False, but it passes:

https://gist.github.com/uilianries/971fc896b481170ee311ac6e9eb35152

In case passing assembly=True it will fail for sure, the issue #23721 has a very similar build log with my current error.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is how it has always worked since Conan 1.x - setting the value of an option in config_options makes it the "default" value - and the default value is only used if the user did not override it elsewhere (e.g via CLI).


def configure(self):
Expand All @@ -63,10 +64,27 @@ def requirements(self):
if self.options.get_safe("with_numa", False):
self.requires("libnuma/2.0.14")

def validate_build(self):
if cross_building(self) and self.settings.os == "Android" and self.options.assembly:
# FIXME: x265 uses custom command to invoke clang to compile assembly files.
# clang++ -fPIC -c src/source/common/aarch64/mc-a.S -o mc-a.S.o
# FAILED: mc-a.S.o libx2f309356bd8526/b/build/Release/mc-a.S.o
# clang++ -fPIC -c src/source/common/aarch64/mc-a.S -o mc-a.S.o
# <instantiation>:11:9: error: unknown directive
# .func x265_pixel_avg_pp_4x4_neon
raise ConanInvalidConfiguration(f"{self.ref} fails to build with '&:assembly=True' for Android. Contributions are welcome.")

def validate(self):
if self.options.shared and is_msvc(self) and is_msvc_static_runtime(self):
raise ConanInvalidConfiguration("shared not supported with static runtime")
uilianries marked this conversation as resolved.
Show resolved Hide resolved

if self.settings.compiler == "apple-clang" and "arm" in self.settings.arch and self.options.assembly:
# Undefined symbols for architecture arm64:
# "x265::setupAssemblyPrimitives(x265::EncoderPrimitives&, int)", referenced from:
# x265::x265_setup_primitives(x265_param*) in libx265.a[20](primitives.cpp.o)
# ld: symbol(s) not found for architecture arm64
raise ConanInvalidConfiguration(f"{self.ref} fails to build for Mac M1. Contributions are welcome.")

def build_requirements(self):
if self.options.assembly:
if self.settings.arch in ["x86", "x86_64"]:
Expand Down Expand Up @@ -154,7 +172,10 @@ def package_info(self):
if not self.options.shared:
self.cpp_info.sharedlinkflags = ["-Wl,-Bsymbolic,-znoexecstack"]
elif self.settings.os == "Android":
self.cpp_info.libs.extend(["dl", "m"])
libcxx = stdcpp_library(self)
if libcxx:
self.cpp_info.system_libs.append(libcxx)
self.cpp_info.system_libs.extend(["dl", "m"])
if not self.options.shared:
libcxx = stdcpp_library(self)
if libcxx:
if self.settings.os == "Android" and self.settings.compiler.libcxx == "c++_static":
self.cpp_info.system_libs.append("c++abi")
self.cpp_info.system_libs.append(libcxx)
Loading