Page MenuHome
Differential D16104

Build: add option to error when features can't be enabled
ClosedPublic
Actions

Authored by Brecht Van Lommel (brecht) on Sep 29 2022, 7:26 PM.

Details

Reviewers
Ray Molenkamp (LazyDodo)
Campbell Barton (campbellbarton)
Commits
rBe7a6917617fb: Build: add option to error when features can't be enabled
Summary

This is to help ensure buildbot builds are correct, while still gracefully
disabling features in user/developer builds.

  • Add WITH_STRICT_BUILD_OPTIONS to give an error when features can't be enabled due to missing libraries or other reasons. Add new macro set_and_warn_library_found used everywhere features were being automatically disabled.
  • Remove code from Windows and macOS for various libraries that would automatically disable features. set_and_warn_library_found could be used here also, but we are generally assuming the precompiled libraries are complete and only test for availability when libraries are just added.

Diff Detail

Repository
rB Blender
Branch
master
Build Status
Buildable 24370
Build 24370: arc lint + arc unit

Event Timeline

Brecht Van Lommel (brecht) created this revision.Sep 29 2022, 7:26 PM
Harbormaster completed remote builds in B24029: Diff 56296.Sep 29 2022, 7:26 PM
Brecht Van Lommel (brecht) added a comment.Sep 29 2022, 7:27 PM

https://builder.blender.org/admin/#/builders/136/builds/113

Brecht Van Lommel (brecht) updated this revision to Diff 56314.Sep 30 2022, 2:10 AM

Fix Metal, disable Jack on macOS

Harbormaster completed remote builds in B24036: Diff 56314.Sep 30 2022, 2:10 AM
Brecht Van Lommel (brecht) added a comment.Sep 30 2022, 3:43 PM

So this reveals two issues with our build:

  • OpenMP is seemingly disabled in the Linux builds. Missing some system libraries on CentOS 7 VMs, or some detection issue.
  • JACK is disabled on macOS, and probably has been for many years. We could either install the framework on all our macOS VMs or just keep it disabled since no one seems to have noticed.
Brecht Van Lommel (brecht) requested review of this revision.Sep 30 2022, 4:44 PM
Brecht Van Lommel (brecht) added reviewers: Ray Molenkamp (LazyDodo), Sybren A. Stüvel (sybren).Sep 30 2022, 4:47 PM
Brecht Van Lommel (brecht) added a subscriber: Sergey Sharybin (sergey).

No rush to review this, and the OpenMP issue still needs to be solved. Just something nice to address since it would have prevented some recent bugs in build system changes.

Sergey Sharybin (sergey) added a comment.Sep 30 2022, 6:20 PM

@Brecht Van Lommel (brecht), the OpenMP issue comes from the FindOpenSubdiv.cmake which invokes find_package(OpenMP) from a CMake section where system libraries are disabled. This gets the result cached, and the find_package(OpenMP) which happens later does not re-try to find the libraries. I do not see where we use the compute backend defines so proposal is simple: remove all of them, including the find_package(OpenMP) from FindOpenSubdiv.cmake:

P3226 (An Untitled Masterwork)
1diff --git a/build_files/cmake/Modules/FindOpenSubdiv.cmake b/build_files/cmake/Modules/FindOpenSubdiv.cmake
2index 37a2cddf3de..66d3b435c46 100644
3--- a/build_files/cmake/Modules/FindOpenSubdiv.cmake
4+++ b/build_files/cmake/Modules/FindOpenSubdiv.cmake
5@@ -71,21 +71,6 @@ FIND_PACKAGE_HANDLE_STANDARD_ARGS(OpenSubdiv DEFAULT_MSG
6 IF(OPENSUBDIV_FOUND)
7 SET(OPENSUBDIV_LIBRARIES ${_opensubdiv_LIBRARIES})
8 SET(OPENSUBDIV_INCLUDE_DIRS ${OPENSUBDIV_INCLUDE_DIR})
9-
10- # Find available compute controllers.
11-
12- FIND_PACKAGE(OpenMP)
13- IF(OPENMP_FOUND)
14- SET(OPENSUBDIV_HAS_OPENMP TRUE)
15- ELSE()
16- SET(OPENSUBDIV_HAS_OPENMP FALSE)
17- ENDIF()
18-
19- OPENSUBDIV_CHECK_CONTROLLER("tbbEvaluator.h" OPENSUBDIV_HAS_TBB)
20- OPENSUBDIV_CHECK_CONTROLLER("clEvaluator.h" OPENSUBDIV_HAS_OPENCL)
21- OPENSUBDIV_CHECK_CONTROLLER("cudaEvaluator.h" OPENSUBDIV_HAS_CUDA)
22- OPENSUBDIV_CHECK_CONTROLLER("glXFBEvaluator.h" OPENSUBDIV_HAS_GLSL_TRANSFORM_FEEDBACK)
23- OPENSUBDIV_CHECK_CONTROLLER("glComputeEvaluator.h" OPENSUBDIV_HAS_GLSL_COMPUTE)
24 ENDIF()
25
26 MARK_AS_ADVANCED(
27diff --git a/build_files/cmake/platform/platform_win32.cmake b/build_files/cmake/platform/platform_win32.cmake
28index 866d0bede3d..65980975eaf 100644
29--- a/build_files/cmake/platform/platform_win32.cmake
30+++ b/build_files/cmake/platform/platform_win32.cmake
31@@ -700,12 +700,6 @@ if(WITH_OPENSUBDIV)
32 debug ${OPENSUBDIV_LIBPATH}/osdCPU_d.lib
33 debug ${OPENSUBDIV_LIBPATH}/osdGPU_d.lib
34 )
35- set(OPENSUBDIV_HAS_OPENMP TRUE)
36- set(OPENSUBDIV_HAS_TBB FALSE)
37- set(OPENSUBDIV_HAS_OPENCL TRUE)
38- set(OPENSUBDIV_HAS_CUDA FALSE)
39- set(OPENSUBDIV_HAS_GLSL_TRANSFORM_FEEDBACK TRUE)
40- set(OPENSUBDIV_HAS_GLSL_COMPUTE TRUE)
41 endif()
42 endif()
43
44diff --git a/intern/opensubdiv/CMakeLists.txt b/intern/opensubdiv/CMakeLists.txt
45index 1dddd70928a..920b8d5c542 100644
46--- a/intern/opensubdiv/CMakeLists.txt
47+++ b/intern/opensubdiv/CMakeLists.txt
48@@ -81,12 +81,6 @@ if(WITH_OPENSUBDIV)
49 )
50 endif()
51
52- opensubdiv_define_component(OPENSUBDIV_HAS_OPENMP)
53- opensubdiv_define_component(OPENSUBDIV_HAS_OPENCL)
54- opensubdiv_define_component(OPENSUBDIV_HAS_CUDA)
55- opensubdiv_define_component(OPENSUBDIV_HAS_GLSL_TRANSFORM_FEEDBACK)
56- opensubdiv_define_component(OPENSUBDIV_HAS_GLSL_COMPUTE)
57-
58 if(WIN32)
59 add_definitions(-DNOMINMAX)
60 add_definitions(-D_USE_MATH_DEFINES)

Brecht Van Lommel (brecht) updated this revision to Diff 56340.Sep 30 2022, 6:33 PM

Include patch from Sergey

Harbormaster completed remote builds in B24051: Diff 56340.Sep 30 2022, 6:33 PM
Brecht Van Lommel (brecht) updated this revision to Diff 56341.Sep 30 2022, 6:44 PM

Restore Metal changes that accidentally got lost in last update

Harbormaster completed remote builds in B24052: Diff 56341.Sep 30 2022, 6:44 PM
Brecht Van Lommel (brecht) added a comment.Sep 30 2022, 6:57 PM

Works on all platforms now:
https://builder.blender.org/admin/#/builders/136/builds/122

Sergey Sharybin (sergey) awarded a token.Oct 3 2022, 9:11 AM
Sybren A. Stüvel (sybren) edited reviewers, added: Campbell Barton (campbellbarton); removed: Sybren A. Stüvel (sybren).Oct 20 2022, 3:27 PM
Campbell Barton (campbellbarton) accepted this revision.Oct 21 2022, 2:06 AM
This revision is now accepted and ready to land.Oct 21 2022, 2:06 AM
Campbell Barton (campbellbarton) awarded a token.Oct 21 2022, 2:06 AM
Brecht Van Lommel (brecht) updated this revision to Diff 57019.Oct 21 2022, 4:16 PM

Rebase

Harbormaster completed remote builds in B24370: Diff 57019.Oct 21 2022, 4:16 PM
Closed by commit rBe7a6917617fb: Build: add option to error when features can't be enabled (authored by Brecht Van Lommel (brecht)). · Explain WhyOct 21 2022, 8:04 PM
This revision was automatically updated to reflect the committed changes.
Brecht Van Lommel (brecht) added a commit: rBe7a6917617fb: Build: add option to error when features can't be enabled.
Brecht Van Lommel (brecht) mentioned this in rB841460afcebd: Build: add back WITH_JACK option on macOS, but disabled by default.Nov 16 2022, 6:09 PM

Revision Contents
Changeset List

PathSize
28 lines
build_files/
cmake/
Modules/
15 lines
config/
4 lines
7 lines
18 lines
platform/
89 lines
205 lines
65 lines
intern/
cycles/
6 lines
cmake/
51 lines
opensubdiv/
6 lines

Diff 57019

View Options

CMakeLists.txt

Loading...
View Options

build_files/cmake/Modules/FindOpenSubdiv.cmake

Loading...
View Options

build_files/cmake/config/blender_full.cmake

Loading...
View Options

build_files/cmake/config/blender_release.cmake

Loading...
View Options

build_files/cmake/macros.cmake

Loading...
View Options

build_files/cmake/platform/platform_apple.cmake

Loading...
View Options

build_files/cmake/platform/platform_unix.cmake

Loading...
View Options

build_files/cmake/platform/platform_win32.cmake

Loading...
View Options

intern/cycles/CMakeLists.txt

Loading...
View Options

intern/cycles/cmake/external_libs.cmake

Loading...
View Options

intern/opensubdiv/CMakeLists.txt

Loading...