[afterimage] migrate from configure-make to CMake and merge with Windows#21649
[afterimage] migrate from configure-make to CMake and merge with Windows#21649ferdymercury wants to merge 3 commits intoroot-project:masterfrom
Conversation
ferdymercury
changed the title
|
Thanks for kicking off this improvement in uncharted territory! May I ask why skip-ci? Don't we maybe want to see this in action, to discover the failures not initially foreseen? |
This comment was marked as outdated.
This comment was marked as outdated.
|
Now it builds as part of ROOT in my Ubu22 system |
Test Results 22 files 22 suites 3d 7h 21m 58s ⏱️ Results for commit ee25b40. ♻️ This comment has been updated with latest results. |
ferdymercury
force-pushed
the
asimagecmake
branch
3 times, most recently
from
c1282f8 to
180ec90
Compare
|
I close and open PR to restart CI |
|
I guess Windows failure is a glitch. Alma failure seems either a missing clean build label, or a real issue. |
|
failed platform uses ninja, but I did not find any error indication. So try to re-run failed job. And as separate effort one should try to suppress tones of warnings from afterimage |
|
Error on alma is: |
|
Try on your machine with |
|
You made cmake for |
|
Thanks Sergei, alma10 build works now.
In this PR or a separate one? |
linev
left a comment
linev
left a comment
There was a problem hiding this comment.
I propose to rebase your PR on at least two separate commits - otherwise we mix here two different things.
First is about of freetype library usage. All builtin-related settings should go to builtins/freetype/CMakeLists.txt file. And one need to check if we really need extra configs for external freetype
Second commit should accumulate your changes in libAfterImage. They looks fine for me.
Probably extra commit(s) to fix major warnings. For the moment there are too many of them.
ferdymercury
force-pushed
the
asimagecmake
branch
from
969f39c to
5e4e1f2
Compare
|
Commits were squashed. Wrt Freetype: I did several attempts until it worked, so it might well be that some of these things were not necessary, so I'll try disabling them and going step by step to see what was really necessary. I couldn't try locally since failures were on other platforms than my local one. |
|
Your PR is grate and I really like conversion to cmake. |
|
I rebase your PR putting first changes in freetype cmake and than your modifications. |
| if(NOT FREETYPE_LIBRARY_RELEASE AND NOT FREETYPE_LIBRARY_DEBUG) | ||
| set_target_properties(Freetype::Freetype PROPERTIES | ||
| IMPORTED_LINK_INTERFACE_LANGUAGES "C" | ||
| IMPORTED_LOCATION "${FREETYPE_LIBRARIES}") |
There was a problem hiding this comment.
I guess it should be instead like in https://github.com/Kitware/CMake/blob/f5fc584bffee1a0641f611523741a99898a864d9/Modules/FindFreetype.cmake#L243 with the suffix. Same above for Release
There was a problem hiding this comment.
We have several other libs with similar logic - they do not have such special flags.
Therefore try first more simpler solutions
make it compile also as part of ROOT suppress debug output in all platforms alma10 has no MMX so detect it macos detect x11 disabled fix typo, simplify check, check for malloc that is not present on mac15 win32: fix wd flags passing add missing dependencies otherwise afterimage might be built before eg JPEG builtin is available fix typo and windows quote
ferdymercury
added
skip code analysis
|
Thanks for the fix Sergei! I will be on holidays so fixing the warnings will have to wait ;) |
linev
left a comment
linev
left a comment
There was a problem hiding this comment.
Now PR compiled on all platforms.
Please suppress main warnings before merge it
4 participants
Sub-part of work towards #9090
Builds locally as standalone project as well as part of my ROOT build, on my Ubu22.