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.

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

[libjpeg-turbo] Fixes cross compilation for armv8 #3392

Closed
Closed
Changes from all commits
Commits
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
20 changes: 15 additions & 5 deletions recipes/libjpeg-turbo/all/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ class LibjpegTurboConan(ConanFile):
"mem_src_dst": [True, False],
"turbojpeg": [True, False],
"java": [True, False],
"enable12bit": [True, False]}
"enable12bit": [True, False],
"keep_binaries": [True, False],
}
default_options = {"shared": False,
"fPIC": True,
"SIMD": True,
Expand All @@ -36,7 +38,9 @@ class LibjpegTurboConan(ConanFile):
"mem_src_dst": True,
"turbojpeg": True,
"java": False,
"enable12bit": False}
"enable12bit": False,
"keep_binaries" : False,
}

_cmake = None

Expand Down Expand Up @@ -89,11 +93,16 @@ def build_requirements(self):
def source(self):
tools.get(**self.conan_data["sources"][self.version])
os.rename(self.name + "-" + self.version, self._source_subfolder)
if tools.cross_building(self.settings):
tools.replace_in_file(os.path.join(self._source_subfolder,"CMakeLists.txt"),"CMAKE_SYSTEM_PROCESSOR","CMAKE_SYSTEM_PROCESSOR_TARGET")
Comment on lines +96 to +97
Copy link
Contributor

@SpaceIm SpaceIm Nov 3, 2020

Choose a reason for hiding this comment

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

patch in build() instead of source() please.
What is CMAKE_SYSTEM_PROCESSOR_TARGET? CMAKE_SYSTEM_PROCESSOR is already the target architecture.


def _configure_cmake(self):
if self._cmake:
return self._cmake
self._cmake = CMake(self, set_cmake_flags=True)
if tools.cross_building(self.settings):
if self.settings.arch == "armv8":
self._cmake.definitions["CMAKE_SYSTEM_PROCESSOR_TARGET"] = "aarch64"
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure why the only armv8 is affected? maybe there is a wider conan issue? /cc @jgsogo

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm with @SpaceIm , I don't know what is CMAKE_SYSTEM_PROCESSOR_TARGET, I cannot find it in CMake docs.

Copy link
Contributor

@prince-chrismc prince-chrismc Nov 4, 2020

Choose a reason for hiding this comment

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

maybe it was supposed to be https://cmake.org/cmake/help/v3.1/variable/CMAKE_SYSTEM_PROCESSOR.html ?

The OP is replacing the standard variable with a custom one for the recipe

self._cmake.definitions["ENABLE_STATIC"] = not self.options.shared
self._cmake.definitions["ENABLE_SHARED"] = self.options.shared
self._cmake.definitions["WITH_SIMD"] = self.options.get_safe("SIMD", False)
Expand Down Expand Up @@ -134,9 +143,10 @@ def package(self):
tools.rmdir(os.path.join(self.package_folder, "lib", "pkgconfig"))
tools.rmdir(os.path.join(self.package_folder, "doc"))
# remove binaries and pdb files
for pattern_to_remove in ["cjpeg*", "djpeg*", "jpegtran*", "tjbench*", "wrjpgcom*", "rdjpgcom*", "*.pdb"]:
for bin_file in glob.glob(os.path.join(self.package_folder, "bin", pattern_to_remove)):
os.remove(bin_file)
if not self.options.keep_binaries:
for pattern_to_remove in ["cjpeg*", "djpeg*", "jpegtran*", "tjbench*", "wrjpgcom*", "rdjpgcom*", "*.pdb"]:
for bin_file in glob.glob(os.path.join(self.package_folder, "bin", pattern_to_remove)):
os.remove(bin_file)
Comment on lines +146 to +149
Copy link
Contributor

Choose a reason for hiding this comment

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

don't keep pdb files.

Copy link
Contributor

@SSE4 SSE4 Nov 3, 2020

Choose a reason for hiding this comment

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

yes, this is unrelated change

Copy link
Contributor

Choose a reason for hiding this comment

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

If the executables are already generated, why removing them from the package? I would say that storage is not an issue nowadays (at least, it isn't for us in ConanCenter). I would remove keep_binaries option and keep these binaries in the package.

Copy link
Contributor

Choose a reason for hiding this comment

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

@KristianJerpetjon
We have an issue for discussing including pdbs: #1982
#1982 (comment) seems to suggest a conan feature will be added for handling pdb's.


def package_info(self):
self.cpp_info.components["jpeg"].names["pkg_config"] = "libjpeg"
Expand Down