Page MenuHome

Fix build of library dependencies on Linux aarch64
ClosedPublic

Authored by Patrick Mours (pmoursnv) on Jun 29 2022, 1:48 PM.

Details

Summary

rBb9c37608a9e959a896f5358d4ab3d3d001a70833 moved evaluation of versions.cmake before options.cmake, as a result of which BLENDER_PLATFORM_ARM is no longer defined in versions.cmake, causing it to choose the wrong OpenSSL version for aarch64, which fails to build.

Also, probably doesn't warrant changing, but building flex is crashing the GCC version shipped with Ubuntu 18.04, which was fixed by a later commit (https://github.com/westes/flex/commit/24fd0551333e7eded87b64dd36062da3df2f6380) that is not part of the flex release used. Included that commit as a patch to make it work, but since that GCC version is technically below the minimum supported by Blender at this point, I'm fine with leaving it out too.

Diff Detail

Repository
rB Blender
Branch
fix_linux_aarch64_build (branched from master)
Build Status
Buildable 22769
Build 22769: arc lint + arc unit

Event Timeline

Patrick Mours (pmoursnv) requested review of this revision.Jun 29 2022, 1:48 PM
Patrick Mours (pmoursnv) created this revision.
Brecht Van Lommel (brecht) requested changes to this revision.Jun 29 2022, 3:29 PM
Brecht Van Lommel (brecht) added inline comments.
build_files/build_environment/cmake/flex.cmake
8

I'm fine with this patch, but add a comment here explaining what it's for.

build_files/build_environment/cmake/versions.cmake
392 ↗(On Diff #53055)

This is not equivalent on macOS, where it's arm64.

I didn't see the reason why the order of options.cmake and versions.cmake was changed, could we just change it back?

This revision now requires changes to proceed.Jun 29 2022, 3:29 PM

Added comments and rearranged cmake include order instead of using platform specific check in version.cmake

Fine with me, but let's wait for @Ray Molenkamp (LazyDodo) since there might be a good reason these were reordered.

kept no notes, but at quick glance i see no need for it, i likely wanted to keep options and the one with all boosts nonsense in it close together. changing the order back is fine with me.

That being said, i wouldn't mind getting rid of the arm specific versions and having everything back on a single recent version (but that perhaps is a story for a different diff)

build_files/build_environment/cmake/ispc.cmake
40

would -DARM_ENABLED=${BLENDER_PLATFORM_ARM} work for all platforms? would allow some deduplication of two identical looking blocks.

otherwise something like this would be my preferred way to go about this (not tested, i may have screwed up the syntax, test before blindly lifting this code)

elseif(UNIX)
    set(ISPC_EXTRA_ARGS_UNIX
      -DCMAKE_C_COMPILER=${LIBDIR}/llvm/bin/clang
      -DCMAKE_CXX_COMPILER=${LIBDIR}/llvm/bin/clang++
      -DFLEX_EXECUTABLE=${LIBDIR}/flex/bin/flex
    )
  if(BLENDER_PLATFORM_ARM)
    LIST(APPEND ISPC_EXTRA_ARGS_UNIX -DARM_ENABLED=On)
  endif()

suggested change is a suggestion only, if this lands as is it's fine with me.

This revision is now accepted and ready to land.Jun 29 2022, 9:22 PM
build_files/build_environment/cmake/ispc.cmake
40

I was concerned that wouldn't work because "BLENDER_PLATFORM_ARM" is not defined at all by default, it is only defined (and set to On) when on an ARM system, so CMake would probably misinterpret this on non-ARM systems.
And ISPC CMakeLists.txt defaults ARM_ENABLED to On, so would actually have to explicitly disable it on x86, rather than enable it on ARM. But since that might be changed, I thought it safer to simply explicitly set it in both cases.

cmake's docs on if say

True if the constant is 1, ON, YES, TRUE, Y, or a non-zero number. False if the constant is 0, OFF, NO, FALSE, N, IGNORE, NOTFOUND, the empty string, or ends in the suffix -NOTFOUND.

so it being undefined shouldn't be a problem

Simplified ISPC ARM_ENABLED update