OpenColorIO icon indicating copy to clipboard operation
OpenColorIO copied to clipboard

Config type does not default to CMAKE_BUILD_TYPE on build/install under Windows

Open Simran-B opened this issue 5 years ago • 4 comments

Environment:

  • Windows 10
  • Visual Studio Community 2019, 64-bit toolchain (vcvarsall.bat x64)
  • CMake 3.18.2

I tried to build OCIO without bothering with any of its dependencies, which should be possible thanks to OCIO_INSTALL_EXT_PACKAGES:

cmake .. -DCMAKE_BUILD_TYPE=Release -DOCIO_INSTALL_EXT_PACKAGES=ALL
cmake --build .

A linker error occurred however:

LINK : fatal error LNK1104: cannot open file "..\..\ext\dist\lib\expatMD.lib"

And this was logged for some install steps:

-- Install configuration: "Debug"

There's a mismatch between CMAKE_BUILD_TYPE=Release and what configuration type is used when installing external packages. Supplying the config type to the --build command manually fixes the problem:

cmake --build . --config Release

In case of a Debug configuration, configuration and building worked, but not the installation:

cmake .. -DCMAKE_BUILD_TYPE=Debug -DOCIO_INSTALL_EXT_PACKAGES=ALL
cmake --build .
cmake --install . --prefix package
...
-- Install configuration: "Release"
...
  file INSTALL cannot find

"C:/OCIO/build/src/bindings/python/Release/PyOpenColorIO.pyd": File exists.

The problem here is that cmake --install defaults to Release. By explicitly setting the config type once again it works:

cmake --install . --config Debug --prefix package

This is unlike cmake --build . which defaults to Debug.

According to the cmake docs, it should be possible set the environment variable CMAKE_CONFIG_TYPE to control the default at least for the --build command. I tried to set it via an env var as well as with set() in relevant places in the CMake code, but it did not have any effect. It's unclear to me whether I'm just not using it right, or if it's incompatible with ExternalProject_Add() or perhaps broken.

If there's any way to make --build and --install default to CMAKE_BUILD_TYPE so that it only has to specified once, that would be splendid. If not, then it should be documented clearer that it should be set manually via --config.

According to @hodoulp, the correct config type is chosen under macOS automatically, and I don't think that multi-config generators are a thing under Linux, so I guess this is a Windows only issue. And to be clear, this isn't a request for supporting multiple configurations using a single build directory.

Simran-B avatar Sep 21 '20 17:09 Simran-B

I've noticed this as well on Windows specifically, which is why in OCIO CI the build call is:

          cmake --build . \
                --target install \
                --config ${{ matrix.build-type }}

Passing -DCMAKE_BUILD_TYPE=Release to the cmake configuration call should correct this too. In either case this should be solvable to default to Release, and at least honor the env var.

michdolan avatar Sep 21 '20 19:09 michdolan

Mark Boorer in TSC meeting:

  • Static linking on Windows
  • Libs with flags like Debug/Release and multi-threading (/MD) not compatible if different. Entire deps graph needs to have same flags
  • -DCMAKE_BUILD_TYPE not respected, only --config should be necessary

Regarding the last point:

In general, it's true that on Windows you don't set the build type when you configure with CMake, but only when you build and install (using a single build dir). But in case of OCIO, this doesn't hold (at least how it is implemented right now):

cmake .. -DOCIO_INSTALL_EXT_PACKAGES=ALL
cmake --build . --config Debug

Note the wrong build type:

  Performing configure step for 'expat_install'
...
  -- Configuration
  --   Prefix ..................... C:/OCIO/build/ext/dist
  --   Build type ................. Release

And this eventually leads to LINK : fatal error LNK1104: cannot open file "..\..\ext\dist\lib\expatMD.lib".

It works if CMAKE_BUILD_TYPE is set however:

cmake .. -DCMAKE_BUILD_TYPE=Debug -DOCIO_INSTALL_EXT_PACKAGES=ALL
cmake --build . --config Debug
  Performing configure step for 'expat_install'
...
  -- Configuration
  --   Prefix ..................... C:/Daten/Olive/Deps/OCIO_test/build/ext/dist
  --   Build type ................. Debug

@michdolan

Passing -DCMAKE_BUILD_TYPE=Release to the cmake configuration call should correct this too.

Yes, it has to be passed (plus --config for the build and install commands) to make it work. This is fine for me, but should be documented, as it's easy to trip over because of the non-conventional behavior of OCIO in this regard.

this should be solvable to default to Release

For configuring, it already defaults to Release:

if(NOT CMAKE_BUILD_TYPE)
	set(CMAKE_BUILD_TYPE Release)
endif()

https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/master/CMakeLists.txt#L24-L26

If no CMAKE_BUILD_TYPE is set, it is set to Release, which is then passed through here: https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/46ca87ac91afcbc16d61847cae939d4b2e3cadad/share/cmake/modules/Findexpat.cmake#L173

This doesn't appear to be a source of error however. With no build type set (above 3 lines commented out), it still fails with a linker error...

Simran-B avatar Sep 21 '20 21:09 Simran-B

Not meant to be a pain but, this really looks painful in Windows, like... really painful. If adoption is the goal, this should be as simply as a yum install or brew install in the worst of the cases.

Isn't it possible to use Homebrew for Linus on windows10 thanks to WSL?

jordibares avatar Oct 16 '20 09:10 jordibares

@jordibares If you can use Ninja the build process is simple, uniform and successful on all platforms:

  • git clone https://github.com/AcademySoftwareFoundation/OpenColorIO.git ocio
  • cd ocio
  • mkdir build
  • cd build
  • cmake .. -DCMAKE_BUILD_TYPE=Release -DOCIO_INSTALL_EXT_PACKAGES=ALL -GNinja
  • ninja all

hodoulp avatar Jun 22 '21 15:06 hodoulp