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

OCIO won't compile with Visual Studio 2022 #1634

Closed
doug-walker opened this issue Apr 22, 2022 · 8 comments
Closed

OCIO won't compile with Visual Studio 2022 #1634

doug-walker opened this issue Apr 22, 2022 · 8 comments

Comments

@doug-walker
Copy link
Collaborator

There may be a number of problems, but one has to do with PyBind 11:
pybind/pybind11#3497

@remia
Copy link
Collaborator

remia commented Apr 22, 2022

Hi @doug-walker,

Do you have a build log to share? I'm a bit surprised as the analysis nightly build currently pass on Visual Studio 2022 as far as I know.

@doug-walker
Copy link
Collaborator Author

@remia , good point! I did not realize that the Analysis is now using VS 2022, unlike CI which is still on VS 2019. However, the Analysis is not building the Python binding:

-DOCIO_BUILD_PYTHON=OFF \

So that's why it doesn't seem to be running into the PyBind 11 problem I mentioned.

I notice the Analysis is installing PyBind 11 on Windows but then not using it (like it seems to for Linux and Mac). Do you think Analysis should be building and testing the binding on Windows too? @michdolan , any thoughts?

(We are working on trying to improve the Windows install instructions, so that's how we ran into the VS 2022 issue.)

@remia
Copy link
Collaborator

remia commented Apr 25, 2022

Yes that's a good catch, I'm not sure why it's setup like this but that was probably me during testing that noticed some issues with Python then forgot to go back and fixing it. I'll look into it when I get some time.

The next release, assuming it's a bump of VFX platform, will probably be a good opportunity to review all dependencies and increase minimal version where appropriate.

@remia
Copy link
Collaborator

remia commented May 4, 2022

I'm currently stuck trying to enable Python testing on the analysis build, there is a DLL load import error while running the Python unit test and I'm not able to understand where it comes from yet, it's very hard to debug using only Github Actions jobs. Note that it manages to successfully compile with pybind 2.6.1 on Visual Studio 22 so I assume it's a different issue than what you saw.

@doug-walker
Copy link
Collaborator Author

Yes, definitely a different issue. The Pybind issue we encountered was only in Debug mode and the analysis builds in Release mode.
pybind/pybind11#3497
microsoft/onnxruntime#9735

@remia
Copy link
Collaborator

remia commented May 11, 2022

Should we bump the CI workflow to Visual Studio 2022 and update pybind11?

@doug-walker
Copy link
Collaborator Author

doug-walker commented May 16, 2022

@remia , we have updated pybind in PR #1647.

Regarding bumping CI to 2022, I think we should. Especially since the draft CY2023 VFX Platform lists that as the minimum version: https://vfxplatform.com/

@michdolan, any thoughts?

If no one objects, we will bump the CI to use Visual Studio 2022.

cedrik-fuoco-adsk added a commit to autodesk-forks/OpenColorIO that referenced this issue May 17, 2022
doug-walker added a commit that referenced this issue May 18, 2022
…22 (#1647)

* Fix issues when building OCIO with MS Visual Studio 2022

Signed-off-by: Cedrik Fuoco <[email protected]>

* Removing GLEW_LIBRARY variable as it is not needed

Signed-off-by: Cedrik Fuoco <[email protected]>

* Bumping CI to use VC 2022 as per Doug Walker request (see #1634)

Signed-off-by: Cedrik Fuoco <[email protected]>

* Reverting the changes on ci_workflow.yml and adding a check for MSVC for the Glew fix.

Signed-off-by: Cedrik Fuoco <[email protected]>

* Typo for the windows action in ci workflow

Signed-off-by: Cedrik Fuoco <[email protected]>

* Typo for the windows action in ci workflow

Signed-off-by: Cedrik Fuoco <[email protected]>

Co-authored-by: Doug Walker <[email protected]>
@doug-walker
Copy link
Collaborator Author

Note: We tried updating CI to VS 2022 but encountered a problem because the Python 2.7 build was no longer supported. Therefore, we will look into making the CI upgrade to VS 2022 later and did not try to include it in PR #1647.

But PR #1647 does fix the compilation issue in VS 2022 (Debug) and so I'm closing this issue.

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

2 participants