AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.74k stars 431 forks source link

2.3.1 release build issue #1920

Closed chenrui333 closed 5 months ago

chenrui333 commented 7 months ago

👋 trying to build the latest release, but run into some build issue. The error log is as below:

error build log ``` /tmp/opencolorio-20231211-6437-rqqe1t/OpenColorIO-2.3.1/src/OpenColorIO/ConfigUtils.cpp:6:10: fatal error: pystring/pystring.h: No such file or directory 6 | #include "pystring/pystring.h" | ^~~~~~~~~~~~~~~~~~~~~ compilation terminated. ```

relates to Homebrew/homebrew-core#156951

doug-walker commented 7 months ago

Thanks for the report, I'll submit a fix for this ASAP.

chenrui333 commented 7 months ago

Thanks @doug-walker!

doug-walker commented 7 months ago

@chenrui333 , would you please post the CMake command you are using? And please let me know the path to your pystring.h file.

The pystring problem typically does not occur (e.g. either in the CI builds or my local builds when using an external pystring), so I want to make sure I'm replicating the issue you found. Thanks!

wahn commented 7 months ago

Same here on a MacBook Pro (Late 2013):

% pwd
/Users/jan/git/github/OpenColorIO
% mkdir build
% cd build
% ccmake ..

The only thing I changed was the directory to find the header files for OpenImageIO:

OpenImageIO_INCLUDE_DIR          /Users/jan/git/github/OpenImageIO/src/include

Continue with cmake:

% make -j
[  0%] Building CXX object src/OpenColorIO/CMakeFiles/OpenColorIO.dir/apphelpers/ColorSpaceHelpers.cpp.o
...
[ 37%] Building CXX object src/OpenColorIO/CMakeFiles/OpenColorIO.dir/SystemMonitor.cpp.o
/Users/jan/git/github/OpenColorIO/src/OpenColorIO/ConfigUtils.cpp:6:10: fatal error: 'pystring/pystring.h' file not found
#include "pystring/pystring.h"
         ^~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [src/OpenColorIO/CMakeFiles/OpenColorIO.dir/ConfigUtils.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [src/OpenColorIO/CMakeFiles/OpenColorIO.dir/all] Error 2
make: *** [all] Error 2
wahn commented 7 months ago

Changing the include line:

% git diff
diff --git a/src/OpenColorIO/ConfigUtils.cpp b/src/OpenColorIO/ConfigUtils.cpp
index 2e774726..b4228ff7 100644
--- a/src/OpenColorIO/ConfigUtils.cpp
+++ b/src/OpenColorIO/ConfigUtils.cpp
@@ -3,7 +3,7 @@

 #include "ConfigUtils.h"
 #include "MathUtils.h"
-#include "pystring/pystring.h"
+#include "pystring.h"
 #include "utils/StringUtils.h"

 namespace OCIO_NAMESPACE

Seems to fix the problem (at least on my machine) ...

% make -j 4
...
[100%] Linking CXX shared module PyOpenColorIO/PyOpenColorIO.so
[100%] Built target PyOpenColorIO
wahn commented 7 months ago

All other places seem to use include <...> instead anyway:

% rg -tcpp "pystring.h"
src/OpenColorIO/ConfigUtils.cpp
6:#include "pystring.h"

src/OpenColorIO/Config.cpp
15:#include <pystring.h>

src/OpenColorIO/transforms/FileTransform.cpp
12:#include <pystring.h>
...

So the diff should probably be:

% git diff
diff --git a/src/OpenColorIO/ConfigUtils.cpp b/src/OpenColorIO/ConfigUtils.cpp
index 2e774726..90e90449 100644
--- a/src/OpenColorIO/ConfigUtils.cpp
+++ b/src/OpenColorIO/ConfigUtils.cpp
@@ -3,7 +3,7 @@

 #include "ConfigUtils.h"
 #include "MathUtils.h"
-#include "pystring/pystring.h"
+#include <pystring.h>
 #include "utils/StringUtils.h"

 namespace OCIO_NAMESPACE
chenrui333 commented 7 months ago

yeah, like @wahn, for pystring ti would be installed into the system path, and it is does not have pystring/. Use system include also works for me.

doug-walker commented 7 months ago

Thank you both. Yes, I was aware of the problem introduced with ConfigUtils.h (it started in a branch that predated the pystring changes and I was too quick to accept git's merge of main). What I'm wondering about is how to prevent this sort of break again in the future since neither our CI or my local builds detected the error. We could change OCIO's local install to mimic the install structure used by, e.g. brew, but I was wondering if there are other variations out there. (The pystring project itself doesn't seem to install the header, so maybe that contributes to the ambiguity about where it should go.) Anyway, thanks for the PR!

wahn commented 7 months ago

You could use the language server protocol with basically any text editor which supports it. To make CMake spit out a compile_commands.json file you could use e.g.:

% cmake .. -DCMAKE_EXPORT_COMPILE_COMMANDS=1

After that I could for example use Helix (as described here and there) to see some useful messages within Helix:

Screenshot 2023-12-20 at 11 53 11

Should work with MS Code as well and any text editor which supports the language server protocol. Things might be platform dependent, but I use it on MacOS and Linux. Should work on Windows as well ...