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

KristianJerpetjon · 2020-11-03T09:04:35Z

Specify library name and version: libjpeg-turbo/all

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

conan-center-bot · 2020-11-03T09:39:58Z

All green in build 1 (bd2cf3f0d0348f15c6263b52441409a402611b4e)! 😊

libjpeg-turbo/2.0.2: Generated 82 packages. All logs here
libjpeg-turbo/2.0.4: Generated 82 packages. All logs here
libjpeg-turbo/2.0.5: Generated 82 packages. All logs here

SpaceIm · 2020-11-03T11:47:22Z

recipes/libjpeg-turbo/all/conanfile.py

+        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")


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

SpaceIm · 2020-11-03T11:47:58Z

recipes/libjpeg-turbo/all/conanfile.py

+        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)


don't keep pdb files.

yes, this is unrelated change

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.

@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.

Croydon · 2020-11-03T13:17:02Z

Doesn't this sound like something that Conan cross-building features should be able to handle? It is certainly impracticable adding a keep_binaries to every recipe

SSE4 · 2020-11-03T14:15:13Z

recipes/libjpeg-turbo/all/conanfile.py


    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"


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

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

~~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

madebr · 2020-11-04T02:43:09Z

I don't fully understand what you're trying to fix.
Has it something to do with this? https://github.com/libjpeg-turbo/libjpeg-turbo/blob/c3bfbde21d81438b731aeefa69011024e8ae08c2/CMakeLists.txt#L57-L64

What is the value of CMAKE_SYSTEM_PROCESSOR that cmake detects when using arm architecture?

Conan has a way to overload some cmake defaults: see https://docs.conan.io/en/latest/reference/build_helpers/cmake.html#definitions
You can add libjpeg-turbo:CONAN_CMAKE_SYSTEM_PROCESSOR=aarch64 to your profile.
But better would be to create a proper cmake toolchain file and add it to the CONAN_CMAKE_TOOLCHAIN environment variable.
See https://docs.conan.io/en/latest/reference/env_vars.html#cmake-related-variables

SSE4 · 2020-11-04T09:20:08Z

@KristianJerpetjon I don't think it's a right place to have a fix in the recipe(s). I also don't think it's correct to have it exclusively for amrv8.
reading docs I see:

CMAKE_SYSTEM_PROCESSOR : optional, processor (or hardware) of the
target system. This variable is not used very much except for one
purpose, it is used to load a
CMAKE_SYSTEM_NAME-compiler-CMAKE_SYSTEM_PROCESSOR.cmake file,
which can be used to modify settings like compiler flags etc. for
the target. You probably only have to set this one if you are using
a cross compiler where every target hardware needs special build
settings.

so it gives me the impression that CMAKE_SYSTEM_PROCESSOR is optional and shouldn't be vital.
we also don't set this variable by default at all: https://github.com/conan-io/conan/blob/6287050a4ca74f47bdcccbb82a66eb749046fa8b/conans/client/build/cmake_flags.py#L224

I'd recommend:

open conan issue https://github.com/conan-io/conan/issues/new/choose
attach logs so we can understand what exactly fails in your case

I believe the correct fix may require setting CMAKE_SYSTEM_PROCESSOR and CMAKE_HOST_SYSTEM_PROCESSOR in conan, and having a mapping for all the supported architectures (not just armv8).

conan-center-bot · 2020-11-07T13:05:42Z

All green in build 2 (bd2cf3f0d0348f15c6263b52441409a402611b4e)! 😊

libjpeg-turbo/2.0.2: Generated 82 packages. All logs here
libjpeg-turbo/2.0.4: Generated 82 packages. All logs here
libjpeg-turbo/2.0.5: Generated 82 packages. All logs here

SSE4 · 2020-11-07T14:38:14Z

created conan-io/conan#8025

ghost · 2020-11-22T14:47:51Z

I detected other pull requests that are modifying libjpeg-turbo recipe:

bump libjpeg-turbo/2.0.6 #3647

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

stale · 2020-12-22T14:57:25Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2021-01-21T15:51:07Z

This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions.

fixed cross compilation for armv8

bd2cf3f

SpaceIm reviewed Nov 3, 2020

View reviewed changes

SSE4 requested review from danimtb and uilianries November 3, 2020 14:13

SSE4 reviewed Nov 3, 2020

View reviewed changes

SSE4 changed the title ~~Fixes cross compilation for armv8~~ [libjpeg-turbo] Fixes cross compilation for armv8 Nov 7, 2020

SSE4 mentioned this pull request Nov 7, 2020

[bug] CMAKE_SYSTEM_PROCESSOR isn't set conan-io/conan#8025

Closed

ericLemanissier mentioned this pull request Nov 14, 2020

[service] Potentially conflicting recipes #3517

Closed

ghost mentioned this pull request Nov 22, 2020

[service] Potentially conflicting recipes #3571

Closed

ghost mentioned this pull request Nov 22, 2020

bump libjpeg-turbo/2.0.6 #3647

Merged

4 tasks

stale bot added the stale label Dec 22, 2020

ghost mentioned this pull request Jan 7, 2021

libjpeg-turbo: fix hooks #4146

Merged

4 tasks

stale bot closed this Jan 21, 2021

szigetics mentioned this pull request Oct 12, 2021

[package] qt/5.15.2: Crash under 32-bits Windows machine when loading a jpeg using QPixmap::loadFromData (under 64-bits it works well) #7596

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

KristianJerpetjon commented Nov 3, 2020 •

edited

Loading

conan-center-bot commented Nov 3, 2020

SpaceIm Nov 3, 2020 •

edited

Loading

SpaceIm Nov 3, 2020

SSE4 Nov 3, 2020 •

edited

Loading

jgsogo Nov 3, 2020

madebr Nov 3, 2020

Croydon commented Nov 3, 2020

SSE4 Nov 3, 2020

jgsogo Nov 3, 2020

prince-chrismc Nov 4, 2020 •

edited

Loading

madebr commented Nov 4, 2020

SSE4 commented Nov 4, 2020 •

edited

Loading

conan-center-bot commented Nov 7, 2020

SSE4 commented Nov 7, 2020

ghost commented Nov 22, 2020

stale bot commented Dec 22, 2020

stale bot commented Jan 21, 2021

		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")

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

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

Conversation

KristianJerpetjon commented Nov 3, 2020 • edited Loading

conan-center-bot commented Nov 3, 2020

SpaceIm Nov 3, 2020 • edited Loading

Choose a reason for hiding this comment

SpaceIm Nov 3, 2020

Choose a reason for hiding this comment

SSE4 Nov 3, 2020 • edited Loading

Choose a reason for hiding this comment

jgsogo Nov 3, 2020

Choose a reason for hiding this comment

madebr Nov 3, 2020

Choose a reason for hiding this comment

Croydon commented Nov 3, 2020

SSE4 Nov 3, 2020

Choose a reason for hiding this comment

jgsogo Nov 3, 2020

Choose a reason for hiding this comment

prince-chrismc Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

madebr commented Nov 4, 2020

SSE4 commented Nov 4, 2020 • edited Loading

conan-center-bot commented Nov 7, 2020

SSE4 commented Nov 7, 2020

ghost commented Nov 22, 2020

stale bot commented Dec 22, 2020

stale bot commented Jan 21, 2021

KristianJerpetjon commented Nov 3, 2020 •

edited

Loading

SpaceIm Nov 3, 2020 •

edited

Loading

SSE4 Nov 3, 2020 •

edited

Loading

prince-chrismc Nov 4, 2020 •

edited

Loading

SSE4 commented Nov 4, 2020 •

edited

Loading