Date
1 - 8 of 8
Review: Visual Studio 2010 compile fix Edit
|
Jeremy Selan <jeremy...@...>
http://github.com/imageworks/OpenColorIO/pull/300
Frederik has submitted a bunch of changes related to Visual Studio 2010 compatibility and STATIC linking. As this touches CMake mojo as used on linux / osx, would those familiar with package management please check this request out? Thanks! -- Jeremy -------------------------------------------------------------------------------------------------------------- I changed the CMake files to fix the compilation issues I (and apparently others) had while trying to compile OpenColorIO on Windows 7 using Visual Studio 2010. This works for the STATIC version of OpenColorIO. I have not tried the SHARED version. Also, I only built the core OpenColorIO library and not any of the tools. Notable changes: Both tinyxml and yaml-cpp can now be found externally using the USE_EXTERNAL_* options in CMake. This is useful when ExternalProject_Add doesn't work as it should. Removed the PkgConfig dependency for yaml-cpp by adding a very raw FindYAML_CPP.cmake package. The latter has a lot of room for improvement and is merely intended as a quick-fix. This is useful when PkqConfig is not present which is rarely the case with Windows Installations The static build of OpenColorIO is no longer linked with external libraries. Static libraries do not incorporate the binary objects of its dependencies. It is up to the runtime binary that links against OpenColorIO to link the entire hierarchy of dependencies. In other words, the target_link_libraries has no effect on archive targets (ceated with add_library(TARGET_NAME STATIC ...)). Sources: http://stackoverflow.com/questions/1242820/can-a-c-static-library-link-to-shared-library http://stackoverflow.com/questions/2157629/linking-static-libraries-to-other-static-libraries Added the definition OCIO_BUILD_STATIC to static builds so the code doesn't use __declspec(dllimport) when building statically. This fixes a lot of linker errors. |
|
|
|
Richard Shaw <hobbe...@...>
On Fri, Dec 14, 2012 at 8:37 PM, Jeremy Selan <jeremy...@...> wrote:
http://github.com/imageworks/OpenColorIO/pull/300My github-fu isn't good enough. How would I go about getting a patch from the pull request so I can test it? Thanks, Richard |
|
|
|
dbr/Ben <dbr....@...>
On 18/12/2012, at 9:00 AM, Richard Shaw wrote:
Add the pull-request-creator's repo as a remote, and fetch the changes $ git remote add frederik git://github.com/frederikaalund/OpenColorIO.git $ git fetch frederik Then either check out the branch temporarily (in "detatched HEAD mode"): $ git checkout frederik/visual-studio-2010-compile-fix Or as a tracking branch, which makes a new local branch connected to the remote: $ git checkout -t frederik/visual-studio-2010-compile-fix Or, if you want an actual ye ol' patch, you can append ".patch" to the Github URL: |
|
|
|
Richard Shaw <hobbe...@...>
On Tue, Dec 18, 2012 at 2:57 AM, dbr/Ben <dbr....@...> wrote:
On 18/12/2012, at 9:00 AM, Richard Shaw wrote:That's what I was looking for! I remembered you could do something like that but I was trying .../pull/300/patch not .patch. Thanks! Richard |
|
|
|
Richard Shaw <hobbe...@...>
On Fri, Dec 14, 2012 at 8:37 PM, Jeremy Selan <jeremy...@...> wrote:
http://github.com/imageworks/OpenColorIO/pull/300Ok, first problem. When I apply the patch to 1.0.8 I get the following on my Linux build: cd /home/build/rpmbuild/OpenColorIO/BUILD/OpenColorIO-1.0.8/build/docs/setuptools-prefix/src/setuptools && /usr/bin/cmake -E touch /home/build/rpmbuild/OpenColorIO/BUILD/OpenColorIO-1.0.8/build/docs/setuptools-prefix/src/setuptools-stamp/setuptools-build /usr/bin/cmake -E cmake_progress_report /home/build/rpmbuild/OpenColorIO/BUILD/OpenColorIO-1.0.8/build/CMakeFiles /home/build/rpmbuild/OpenColorIO/BUILD/OpenColorIO-1.0.8/src/core/PathUtils.cpp:52:15: error: previous declaration of 'char** environ' with 'C++' linkage In file included from /home/build/rpmbuild/OpenColorIO/BUILD/OpenColorIO-1.0.8/src/core/PathUtils.cpp:55:0: /usr/include/unistd.h:546:15: error: conflicts with new declaration with 'C' linkage /home/build/rpmbuild/OpenColorIO/BUILD/OpenColorIO-1.0.8/src/core/PathUtils.cpp: In function 'std::string OpenColorIO::v1::pystring::os::getcwd()': /home/build/rpmbuild/OpenColorIO/BUILD/OpenColorIO-1.0.8/src/core/PathUtils.cpp:125:33: warning: ignoring return value of 'char* getcwd(char*, size_t)', declared with attribute warn_unused_result [-Wunused-result] make[2]: *** [src/core/CMakeFiles/OpenColorIO.dir/PathUtils.cpp.o] Error 1 make[2]: *** Waiting for unfinished jobs.... Richard |
|
|
|
Richard Shaw <hobbe...@...>
I'm not sure if this is causing any issues:
diff --git a/src/core/PathUtils.cpp b/src/core/PathUtils.cpp index a04ecee..4c944b1 100644 --- a/src/core/PathUtils.cpp +++ b/src/core/PathUtils.cpp @@ -49,10 +49,14 @@ #if defined(__APPLE__) && !defined(__IPHONE__) #include <crt_externs.h> // _NSGetEnviron() #elif !defined(WINDOWS) -#include <unistd.h> extern char **environ; #endif +#include <unistd.h> +#if defined(__clang__) +#include <unistd.h> +#endif Is it a problem to include the same header twice? I'm not sure what we should be doing differently for clang. Richard |
|
|
|
Richard Shaw <hobbe...@...>
On Tue, Dec 18, 2012 at 8:20 AM, Richard Shaw <hobbe...@...> wrote:
I'm not sure if this is causing any issues:I can confirm removing this commit fixes compilation on linux. I haven't done any detailed analysis to see if there's any other differences in the resultant package though. Richard |
|
|
|
Richard Shaw <hobbe...@...>
Ok, comprehensive package analysis:
pkgdiff: https://dl.dropbox.com/u/34775202/pkgdiff/ocio/changes_report.html abi-compliance-checker: https://dl.dropbox.com/u/34775202/pkgdiff/ocio/compat_report.html I don't see anything that makes me believe the changes aren't safe (minus the previously mentioned problem). At least on linux... Richard |
|
|