AcademySoftwareFoundation / openvdb

OpenVDB - Sparse volume data structure and tools
http://www.openvdb.org/
Mozilla Public License 2.0
2.64k stars 653 forks source link

[BUILD] Building with Houdini 18.5 has linking issue #1020

Closed jaronwilding closed 3 years ago

jaronwilding commented 3 years ago

Environment

Operating System: Windows 10 - 20H2 Version / Commit SHA: VDB 8.1.0 - SHA: e44a869ff57af243f316e5e4f60d3935363dedb5 CMake Version: 3.20.0 - rc4 Compiler: Visual Studio 15 2017 Win64

Describe the problem

Compiling against my current version of Houdini (18.5.514) results in a linking error.

LINK : fatal error LNK1104: cannot open file 'C:\Program Files\Side Effects Software\Houdini 18.5.514\custom\houdini\ds olib\hboost_regex-mt.lib' [D:\Libraries\openvdb-source\build\openvdb_houdini\openvdb_houdini\openvdb_houdini.vcxproj]

Diving inside to where it is trying to find those libraries, I find its trying to link against a .lib file that has the same name, but instead has a "-x64" appended after the name. For example, the compiler looks for hboost_regex-mt.lib, and in my Houdini install I have hboost_regex-mt-x64.lib.

Additional context

Just renaming the .lib files won't work, and changing the linking inside the cmake/OpenVDBHoudiniSetup.cmake to find those libraries instead doesn't work either.

Idclip commented 3 years ago

Hey @JaronWilding, thanks for reporting! You should be able to change lines 207-210 in OpenVDBHoudiniSetup.cmake from:

  list(APPEND _HOUDINI_EXTRA_LIBRARIES
    ${HOUDINI_DSOLIB_DIR}/hboost_regex-mt.lib
    ${HOUDINI_DSOLIB_DIR}/hboost_thread-mt.lib
  )

to:

  list(APPEND _HOUDINI_EXTRA_LIBRARIES
    ${HOUDINI_DSOLIB_DIR}/hboost_regex-mt-x64.lib
    ${HOUDINI_DSOLIB_DIR}/hboost_thread-mt-x64.lib
  )

Make sure you've cleaned your cmake build directory before rebuilding. I've confirmed this fixes the issue for me on Win10 H18.5.499.

@danrbailey how hard would it be to add Windows CI for Houdini? Can we make the target platform an argument to the download scripts? This wouldn't need to be a commitly job, can be down weekly.

@jmlait @e4lam couldn't spot this change in the release notes, tagging for visibility.

e4lam commented 3 years ago

@Idclip The boost library naming convention changed due to the upgrade in Houdini 18.5 to Boost 1.72 from 1.61. Compare here vs here. Note that this convention of boost library names is also present on all 3 OSes (Windows, Linux, macOS). The difference is that Boost on non-Windows still provides a libhboost_<lib>.{so,dylib} symlink that resolves to the underlying library.

I'm not too familiar with the HDK cmake side of things but looking at $HT/cmake/HoudiniConfig.cmake, it looks like it provides only a few third party library targets like Houdini::hboost_system. I'm not sure why it doesn't provide any of other the third party dependencies. For the long term, will it be useful if SideFX looks at adding more third party library targets to $HT/HoudiniConfig.cmake so that OpenVDBHoudiniSetup.cmake can use them?

Idclip commented 3 years ago

I'm not sure why it doesn't provide any of other the third party dependencies. For the long term, will it be useful if SideFX looks at adding more third party library targets to $HT/HoudiniConfig.cmake so that OpenVDBHoudiniSetup.cmake can use them?

Yes absolutely! I assume the reasoning is that there are a subset of libraries which are typically not needed by plugins? As the Houdini CMake only configures a single Houdini target, I imagine this list is as concise as possible to remove unnecessary linkage. Whilst I like the way you can optionally add these libs with _houdini_create_libraries, we shouldn't have to refer directly to the lib names. So if the simplest solution is to just add them all that works too!

e4lam commented 3 years ago

One the problems here is that we can't predict which libraries an HDK plugin might need to link against. If we look at hboost_regex for example, that is needed only because of SOP_OpenVDB_Vector_Merge.cc. So how about something like this HoudiniConfig.cmake? It adds the _houdini_dep_shared_lib_targets list as a new set of Houdini::Dep::* targets that are separate from the Houdini aggregate target. This allows HDK plugin authors to link additional Houdini third-party libraries that they require without needing to hard code file paths.

danrbailey commented 3 years ago

@danrbailey how hard would it be to add Windows CI for Houdini? Can we make the target platform an argument to the download scripts? This wouldn't need to be a commitly job, can be down weekly.

Yeah, everything is hard-coded to linux at the moment (https://github.com/AcademySoftwareFoundation/openvdb/blob/master/ci/download_houdini.py#L148). Doing the Windows check as part of the houdini_cache_update workflow would simplify some things because there would be no need to strip out redundant data from the Houdini download or to cache anything - it would probably make sense to adapt download_houdini.py but add a new "download_houdini_windows" script. I would expect it's a fair bit of work to put this in place, but I would definitely support having it.

Idclip commented 3 years ago

@Idclip The boost library naming convention changed due to the upgrade in Houdini 18.5 to Boost 1.72 from 1.61.

@e4lam is this the corresponding change? https://www.sidefx.com/changelog/?journal=&categories=&body=boost&version=&build_0=&build_1=&show_versions=on&show_compatibility=on&items_per_page=

If so, I'll add a branch on Houdini 18.5.85 in our CMake as a temporary fix.

One the problems here is that we can't predict which libraries an HDK plugin might need to link against. If we look at hboost_regex for example, that is needed only because of SOP_OpenVDB_Vector_Merge.cc. So how about something like this HoudiniConfig.cmake? It adds the _houdini_dep_shared_lib_targets list as a new set of Houdini::Dep::* targets that are separate from the Houdini aggregate target. This allows HDK plugin authors to link additional Houdini third-party libraries that they require without needing to hard code file paths.

This looks like a good solution to me!

e4lam commented 3 years ago

is this the corresponding change? https://www.sidefx.com/changelog/?journal=&categories=&body=boost&version=&build_0=&build_1=&show_versions=on&show_compatibility=on&items_per_page=

Yep.

If so, I'll add a branch on Houdini 18.5.85 in our CMake as a temporary fix.

I do not think that is needed? 18.5.85 was a pre-release build. The first gold release for 18.5 was build 351.

This looks like a good solution to me!

I'll commit my changes and backport to 18.5. Is this something that you think is needed to be backported to 18.0 as well?

Idclip commented 3 years ago

18.5.85 was a pre-release build.

I wasn't sure, I can just branch on 18.5 in that case.

I'll commit my changes and backport to 18.5. Is this something that you think is needed to be backported to 18.0 as well?

Neither is necessary for us, though adding it to the next prod build of 18.5 would be great. We need to patch VDB in any case and I can simply change the lib names on Windows for 18.5 for now. As soon as your changes are available I will switch to them, but I don't mind if I have to branch on a particular Houdini version to do so.

e4lam commented 3 years ago

@Idclip Ok, the change is now committed for tomorrow's 18.5.533 daily build. The current 18.5 prod candidate was today (18.5.532) so it'll be around another month for this change to make it into the next 18.5 prod build.

Idclip commented 3 years ago

No problem, thanks a lot @e4lam! I'll get a PR for this issue up soon.