Fix macOS and Windows build failures when threading is disabled (#1462)

* Fix macOS and Windows build failures when threading is disabled Addresses #1455 The logic around which implementation of the IlmThread::Semaphore class to use is now consolidated into the block of code that deciphers the platform and specifies the #includes, rather than spread across multiple files. Signed-off-by: Cary Phillips <[email protected]> * Add macOS/Windows builds with threading disabled Signed-off-by: Cary Phillips <[email protected]> * Fix typo with ILMTHREAD_SEMAPHORE_OTHER, and add missing macOS includes Signed-off-by: Cary Phillips <[email protected]> * Add threads-enabled='ON' so it shows up in the job name Signed-off-by: Cary Phillips <[email protected]> --------- Signed-off-by: Cary Phillips <[email protected]>
AcademySoftwareFoundation · Jun 26, 2023 · 049eb3a · 049eb3a
1 parent 11a99df
commit 049eb3a
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 60 deletions.
diff --git a/.github/workflows/ci_workflow.yml b/.github/workflows/ci_workflow.yml
@@ -317,11 +317,12 @@ jobs:
        C++${{ matrix.cxx-standard }},
        config=${{ matrix.build-type }},
        shared=${{ matrix.build-shared }},
-       cxx=${{ matrix.cxx-standard }}>'
+       cxx=${{ matrix.cxx-standard }},
+       threads=${{ matrix.threads-enabled }}>'
     runs-on: macos-${{ matrix.osver }}
     strategy:
       matrix:
-        build: [1, 2, 3, 4]
+        build: [1, 2, 3, 4, 5]
         include:
           # -------------------------------------------------------------------
           # VFX CY2023 - C++17 - MacOS 11.0 
@@ -330,26 +331,40 @@ jobs:
           - build: 1
             build-type: Release
             build-shared: 'ON'
+            threads-enabled: 'ON'
             compiler-desc: AppleClang11.0
             cxx-standard: 17
             osver: 11.0
             exclude-tests:
             vfx-cy: 23
 
-          # Static, Release
+          # Shared, Release, Threads OFF
           - build: 2
+            build-type: Release
+            build-shared: 'ON'
+            threads-enabled: 'OFF'
+            compiler-desc: AppleClang11.0
+            cxx-standard: 17
+            osver: 11.0
+            exclude-tests:
+            vfx-cy: 23
+
+          # Static, Release
+          - build: 3
             build-type: Release
             build-shared: 'OFF'
+            threads-enabled: 'ON'
             compiler-desc: AppleClang11.0
             cxx-standard: 17
             osver: 11.0
             exclude-tests:
             vfx-cy: 23
 
           # Shared, Debug
-          - build: 3
+          - build: 4
             build-type: Debug
             build-shared: 'ON'
+            threads-enabled: 'ON'
             compiler-desc: AppleClang11.0
             cxx-standard: 17
             osver: 11.0
@@ -360,9 +375,10 @@ jobs:
           # VFX CY2023 - C++17 - MacOS 12
           # -------------------------------------------------------------------
           # Shared, Release
-          - build: 4
+          - build: 5
             build-type: Release
             build-shared: 'ON'
+            threads-enabled: 'ON'
             compiler-desc: AppleClang11.0
             cxx-standard: 17
             osver: 12.0
@@ -428,11 +444,12 @@ jobs:
       <${{ matrix.compiler-desc }},
        config=${{ matrix.build-type }}, 
        shared=${{ matrix.build-shared }}, 
-       cxx=${{ matrix.cxx-standard }}>'
+       cxx=${{ matrix.cxx-standard }},
+       threads=${{ matrix.threads-enabled }}>'
     runs-on: windows-${{ matrix.osver }}
     strategy:
       matrix:
-        build: [1, 2, 3, 4]
+        build: [1, 2, 3, 4, 5]
         include:
           # -------------------------------------------------------------------
           # VFX CY2023 - C++17 - Windows 2022 runner - MSVC 2022 (17.5)
@@ -441,16 +458,29 @@ jobs:
           - build: 1
             build-type: Release
             build-shared: 'ON'
+            threads-enabled: 'ON'
             compiler-desc: msvc17.5
             cxx-standard: 17
             vfx-cy: 2023
             exclude-tests: ''
             osver: 2022
 
-          # Static, Release
+          # Shared, Release, Threads OFF
           - build: 2
+            build-type: Release
+            build-shared: 'ON'
+            threads-enabled: 'OFF'
+            compiler-desc: msvc17.5
+            cxx-standard: 17
+            vfx-cy: 2023
+            exclude-tests: ''
+            osver: 2022
+
+          # Static, Release
+          - build: 3
             build-type: Release
             build-shared: 'OFF'
+            threads-enabled: 'ON'
             compiler-desc: msvc17.5
             cxx-standard: 17
             vfx-cy: 2023
@@ -461,19 +491,21 @@ jobs:
           # VFX CY2022 - C++17 - Windows 2019 runner - MSVC 2019 (16.11) 
           # -------------------------------------------------------------------
           # Shared, Release
-          - build: 3
+          - build: 4
             build-type: Release
             build-shared: 'ON'
+            threads-enabled: 'ON'
             compiler-desc: msvc16.11
             cxx-standard: 17
             vfx-cy: 2022
             exclude-tests: ''
             osver: 2019
 
           # Static, Release
-          - build: 4
+          - build: 5
             build-type: Release
             build-shared: 'OFF'
+            threads-enabled: 'ON'
             compiler-desc: msvc16.11
             cxx-standard: 17
             vfx-cy: 2022

diff --git a/src/lib/IlmThread/IlmThreadSemaphore.cpp b/src/lib/IlmThread/IlmThreadSemaphore.cpp
@@ -10,11 +10,9 @@
 //
 //-----------------------------------------------------------------------------
 
-#include "IlmThreadConfig.h"
+#include "IlmThreadSemaphore.h"
 
-#if !(ILMTHREAD_THREADING_ENABLED)
-
-#    include "IlmThreadSemaphore.h"
+#if ILMTHREAD_SEMAPHORE_DISABLED
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER
 
@@ -41,4 +39,4 @@ Semaphore::value () const
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT
 
-#endif
+#endif // ILMTHREAD_SEMAPHORE_DISABLED
diff --git a/src/lib/IlmThread/IlmThreadSemaphore.h b/src/lib/IlmThread/IlmThreadSemaphore.h
@@ -18,25 +18,41 @@
 #include "IlmThreadConfig.h"
 #include "IlmThreadNamespace.h"
 
-#if defined(__APPLE__)
-#   include <AvailabilityMacros.h>
-#endif
+//
+// Decipher the platform-specific threading support.
+// Set the ILMTHREAD_SEMAPHORE_* defines to indicate the corresponding
+// implementation of the Semaphore class. Only one of these should be
+// defined.
+//
 
 #if ILMTHREAD_THREADING_ENABLED
 #    if ILMTHREAD_HAVE_POSIX_SEMAPHORES
 #        include <semaphore.h>
-#    elif defined(__APPLE__) && MAC_OS_X_VERSION_MIN_REQUIRED > 1050 && !defined(__ppc__)
-#        include <dispatch/dispatch.h>
+#        define ILMTHREAD_SEMAPHORE_POSIX 1
+#    elif defined(__APPLE__)
+#        include <AvailabilityMacros.h>
+#        if MAC_OS_X_VERSION_MIN_REQUIRED > 1050 && !defined(__ppc__)
+#            include <dispatch/dispatch.h>
+#            define ILMTHREAD_SEMAPHORE_OSX 1
+#        else
+#            include <condition_variable>
+#            include <mutex>
+#            define ILMTHREAD_SEMAPHORE_OTHER 1
+#        endif
 #    elif (defined(_WIN32) || defined(_WIN64))
 #        ifdef NOMINMAX
 #            undef NOMINMAX
 #        endif
 #        define NOMINMAX
 #        include <windows.h>
+#        define ILMTHREAD_SEMAPHORE_WINDOWS 1
 #    else
 #        include <condition_variable>
 #        include <mutex>
+#        define ILMTHREAD_SEMAPHORE_OTHER 1
 #    endif
+#else
+#    define ILMTHREAD_SEMAPHORE_DISABLED 1
 #endif
 
 ILMTHREAD_INTERNAL_NAMESPACE_HEADER_ENTER
@@ -53,18 +69,21 @@ class ILMTHREAD_EXPORT_TYPE Semaphore
     ILMTHREAD_EXPORT int  value () const;
 
 private:
-#if ILMTHREAD_HAVE_POSIX_SEMAPHORES
+
+#if ILMTHREAD_SEMAPHORE_POSIX
 
     mutable sem_t _semaphore;
 
-#elif defined(__APPLE__) && MAC_OS_X_VERSION_MIN_REQUIRED > 1050 && !defined(__ppc__)
+#elif ILMTHREAD_SEMAPHORE_OSX
+
     mutable dispatch_semaphore_t _semaphore;
 
-#elif (defined(_WIN32) || defined(_WIN64))
+#elif ILMTHREAD_SEMAPHORE_WINDOWS
 
     mutable HANDLE _semaphore;
 
-#elif ILMTHREAD_THREADING_ENABLED
+#elif ILMTHREAD_SEMAPHORE_OTHER
+
     //
     // If the platform has threads but no semaphores,
     // then we implement them ourselves using condition variables

diff --git a/src/lib/IlmThread/IlmThreadSemaphoreOSX.cpp b/src/lib/IlmThread/IlmThreadSemaphoreOSX.cpp
@@ -5,19 +5,18 @@
 
 //-----------------------------------------------------------------------------
 //
-//	class Semaphore -- implementation for OSX platform(it don't support unnamed Posix semaphores)
+//	class Semaphore -- implementation for OSX platform (it doesn't
+//	support unnamed Posix semaphores)
+//
 //	std::condition_variable + std::mutex emulation show poor performance
 //
 //-----------------------------------------------------------------------------
 
-#if defined(__APPLE__) && !ILMTHREAD_HAVE_POSIX_SEMAPHORES
-#    include <AvailabilityMacros.h>
+#include "IlmThreadSemaphore.h"
 
-// No libdispatch prior to 10.6, and no support for it on any ppc.
-#if MAC_OS_X_VERSION_MIN_REQUIRED > 1050 && !defined(__ppc__)
+#if ILMTHREAD_SEMAPHORE_OSX
 
-#    include "Iex.h"
-#    include "IlmThreadSemaphore.h"
+#include "Iex.h"
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER
 
@@ -63,5 +62,4 @@ Semaphore::value () const
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT
 
-#    endif
-#endif
+#endif // ILMTHREAD_SEMAPHORE_OSX
diff --git a/src/lib/IlmThread/IlmThreadSemaphorePosix.cpp b/src/lib/IlmThread/IlmThreadSemaphorePosix.cpp
@@ -10,14 +10,13 @@
 //
 //-----------------------------------------------------------------------------
 
-#include "IlmThreadConfig.h"
+#include "IlmThreadSemaphore.h"
 
-#if ILMTHREAD_HAVE_POSIX_SEMAPHORES
+#if ILMTHREAD_SEMAPHORE_POSIX
 
-#    include "Iex.h"
-#    include "IlmThreadSemaphore.h"
-#    include <assert.h>
-#    include <errno.h>
+#include "Iex.h"
+#include <assert.h>
+#include <errno.h>
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER
 
@@ -70,4 +69,4 @@ Semaphore::value () const
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT
 
-#endif
+#endif // ILMTHREAD_SEMAPHORE_POSIX
diff --git a/src/lib/IlmThread/IlmThreadSemaphorePosixCompat.cpp b/src/lib/IlmThread/IlmThreadSemaphorePosixCompat.cpp
@@ -10,19 +10,11 @@
 //
 //-----------------------------------------------------------------------------
 
-#include "IlmThreadConfig.h"
+#include "IlmThreadSemaphore.h"
 
-#if defined(__APPLE__)
-#    include <AvailabilityMacros.h>
-#endif
+#if ILMTHREAD_SEMAPHORE_OTHER
 
 // Use this code as a fallback for macOS versions without libdispatch.
-#if ILMTHREAD_THREADING_ENABLED
-#    if (!(ILMTHREAD_HAVE_POSIX_SEMAPHORES) && !defined(_WIN32) && !defined(_WIN64) && \
-        (!defined(__APPLE__) || (defined(__APPLE__) && \
-        (MAC_OS_X_VERSION_MIN_REQUIRED < 1060 || defined(__ppc__)))))
-
-#    include "IlmThreadSemaphore.h"
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER
 
@@ -84,5 +76,4 @@ Semaphore::value () const
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT
 
-#    endif // posix semaphore compat
-#endif     // enable threading
+#endif // ILMTHREAD_SEMAPHORE_OTHER
diff --git a/src/lib/IlmThread/IlmThreadSemaphoreWin32.cpp b/src/lib/IlmThread/IlmThreadSemaphoreWin32.cpp
@@ -9,15 +9,14 @@
 //
 //-----------------------------------------------------------------------------
 
-#include "IlmThreadConfig.h"
+#include "IlmThreadSemaphore.h"
 
-#if (defined(_WIN32) || defined(_WIN64)) && !ILMTHREAD_HAVE_POSIX_SEMAPHORES
+#if ILMTHREAD_SEMAPHORE_WINDOWS
 
-#    include "Iex.h"
-#    include "IlmThreadSemaphore.h"
-#    include <assert.h>
-#    include <iostream>
-#    include <string>
+#include "Iex.h"
+#include <assert.h>
+#include <iostream>
+#include <string>
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER
 
@@ -126,4 +125,5 @@ Semaphore::value () const
 
 ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT
 
-#endif // _WIN32
+#endif // ILMTHREAD_SEMAPHORE_WINDOWS
+