Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows debug builds link issue #1570

Closed
aloysbaillet opened this issue Jul 26, 2021 · 8 comments
Closed

Windows debug builds link issue #1570

aloysbaillet opened this issue Jul 26, 2021 · 8 comments

Comments

@aloysbaillet
Copy link
Contributor

Description of Issue

On Windows, debug builds link against debug python libraries, but boost python is configured to link against non-debug python libraries (see https://www.boost.org/doc/libs/1_70_0/libs/python/doc/html/building/python_debugging_builds.html for info).

I think the Boost default makes sense for USD as well: to properly debug USD started from a debug python (python_d.exe) it means recompiling PySide and numpy and many other python libraries!

I couldn't find a way to tell CMake FindPython* macros to ignore the debug libraries, but this patch makes debug build work fine on windows:

diff --git a/cmake/defaults/Packages.cmake b/cmake/defaults/Packages.cmake
index 79677059d..b4cc9f4e9 100644
--- a/cmake/defaults/Packages.cmake
+++ b/cmake/defaults/Packages.cmake
@@ -76,6 +76,7 @@ if(PXR_ENABLE_PYTHON_SUPPORT)
     if(PXR_PY_UNDEFINED_DYNAMIC_LOOKUP AND NOT WIN32 )
         set(PYTHON_LIBRARIES "")
     endif()
+    string(REPLACE "_d" "" PYTHON_LIBRARIES "${PYTHON_LIBRARIES}")

     find_package(Boost
         COMPONENTS

This patch simply removes the _d from the python library path, which solves the problem.
The patch is rather hacky unfortunately, but I couldnt' find a nice cmake way... Happy to propose this as a PR if you think it is acceptable!

Steps to Reproduce

  1. Build USD with the build script on Windows using the --debug flag.

System Information (OS, Hardware)

Windows, any hardware

Package Versions

Repro with USD 21.08

Build Flags

--debug

@meshula
Copy link
Member

meshula commented Jul 26, 2021

This would need to be an option, having a fully built out python_d environment is a legit and normal thing to do. USD debug on windows, should link debug libs all the way through by default. You can't tell the cmake macros to ignore the _d libs, but what you can do is let cmake do its thing, then make a new variable where you resolve the libraries according to your preference. That would probably be more robust overall than regexing out the _d. Consuming libraries would then need to reference this resolved libs variable of course.

@aloysbaillet
Copy link
Contributor Author

Thanks @meshula , I agree this should ideally be an option for the build script. That said I tried for a few hours to build everything with debug mode and despite turning off most plugins and imaging I never got usdcat to work in full debug mode... So I'm expecting that not many people would ever try this, and whoever really needs it would use the cmake build directly?

@meshula
Copy link
Member

meshula commented Jul 27, 2021

LOL maybe :) I haven't done a full debug build in some time because of the sorts of difficulties you are mentioning, but in the early days of the port I used it for usdview debugging. These days I almost exclusively use the RelWithDebInfo as a debugging target and selectively disable optimizations.

My main concern is that mixing and matching debug and release libraries is known to be problematic on windows because the CRT and CRTD libraries don't share heaps, and it's all to easy to alloc in one free in the other. Also C++ containers and iterators aren't compatible between debug and release (although has that been addressed in VS2019 perhaps? And if so, I think it's via some preprocessor directives which also need to be set up?). It's often not a problem when concerns (Python embedding and C++) are well separated, but USD's implementation reaches into the Python internals in several places in Vt and Tf, and mixing the library kinds therefore worries me. I guess I'm surprised/not surprised that the boost libraries default in that manner.

At least if it's an option that's deliberately enabled, you know where to start looking for issues if problems such as heap corruptions arise.

So that's where my caution on the defaulted behavior is coming from. To be clear, I'm not opposed to this proposal, because people who know what they're doing such as yourself are going to know what to expect, but I do think caution on behalf of more naive users of the library is probably warranted.

@jilliene
Copy link

Filed as internal issue #USD-6806

@sunyab
Copy link
Contributor

sunyab commented Aug 19, 2021

Hey @aloysbaillet, I think this will be addressed with PR #1478 which I'm testing now. However, I was trying to repro this issue by building on Windows using the --debug flag and everything seemed to work OK. Did you get a build error when doing this on your side? Or did something break at runtime?

@sunyab
Copy link
Contributor

sunyab commented Aug 19, 2021

Oh, or did you have a debug build of Python available on your system? I haven't gotten as far as building out debug Python on Windows yet.

@aloysbaillet
Copy link
Contributor Author

Thanks Sunya! I was going full debug indeed, I even got Qt and PySide2 compiled in debug to make sure I could load everything. You don't need to go that far if you skip imaging of course:)

@sunyab
Copy link
Contributor

sunyab commented Nov 12, 2021

I believe this issue should be addressed in the 21.11 release with the fix from PR #1478 (commit 8cf99d8). Closing this out, thanks!

@sunyab sunyab closed this as completed Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants