-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[abseil] Don't override cmake system processor #8577
Conversation
I detected other pull requests that are modifying abseil/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
I think another solution could be more robust: In CMakeLists wrapper: ...
if(NOT DEFINED CMAKE_SYSTEM_PROCESSOR)
set(CMAKE_SYSTEM_PROCESSOR ${CONAN_ABSEIL_SYSTEM_PROCESSOR})
endif()
add_subdirectory(source_subfolder) In if tools.cross_building(self):
self._cmake.definitions["CONAN_ABSEIL_SYSTEM_PROCESSOR"] = str(self.settings.arch) If a toolchain file has defined |
Yeah, maybe. But I'm not sure why? It's not like any other recipes does this.. |
Few other recipes do this (arrow, cpu_features, cpuinfo, libjpeg-turbo, libjxl, libpng, libx265, mozjpeg, opencv, opengv, tensorflow-lite, tesseract, xnnpack). It's required as soon as upstream CMakeLists has some logic based on |
Ah, OK. Didn't know that and I thought Conan set I will add in your suggestion. |
c29e85d
to
1a2239b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Don't understand how this change could cause this error:
[required] abseil/20200225.2@ Linux, GCC |
It's unrelated, CI of CCI now tests gcc-11, there is a missing include ( Oh, probably something wrong with #7443 I guess it should be something like this instead: def _abseil_abi_config(self):
"""Determine the Abseil ABI for polyfills (absl::any, absl::optional, absl::string_view, and absl::variant)"""
return "1" if tools.valid_min_cppstd(self, 17) else "0" |
7151026
to
9e44e15
Compare
This comment has been minimized.
This comment has been minimized.
Overriding CMAKE_SYSTEM_PROCESSOR breaks Android builds since arch is note correctly translated from Conan format to Android format (like armv8 -> aarch64).
9e44e15
to
8bcda2b
Compare
This comment has been minimized.
This comment has been minimized.
Didn't seem to work unfortunately :( |
Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
Failure in build 6 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
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. |
@anton-danielsson there are conflicts, please rebase |
This PR can be closed, #6383 has fixed this issue. |
Specify library name and version: abseil/all
Overriding CMAKE_SYSTEM_PROCESSOR breaks Android builds since
arch is note correctly translated from Conan format to Android format
(like armv8 -> aarch64).
conan-center hook activated.