Skip to content

Conversation

@lgritz
Copy link
Collaborator

@lgritz lgritz commented May 17, 2025

The INSTALL.md Windows building instructions were out of date in two important ways: (1) they neglected to mention OpenColorIO as a (newly) required dependency nor give instructions for how to build it; (2) it doesn't mention the new BUILD_MISSING_DEPS capabilities.

So basically, this PR modifies our Windows build guidance to set OpenImageIO_BUILD_MISSING_DEPS=all and rely on that without needing to document any additional steps for dependencies.

The INSTALL.md Windows building instructions were out of date in two
important ways: (1) they neglected to mention OpenColorIO as a (newly)
required dependency nor give instructions for how to build it; (2) it
doesn't mention the new BUILD_MISSING_DEPS capabilities.

So basically, this PR modifies our Windows build guidance to set
OpenImageIO_BUILD_MISSING_DEPS=all and rely on that without needing to
document any additional steps for dependencies.

Signed-off-by: Larry Gritz <[email protected]>
@lgritz
Copy link
Collaborator Author

lgritz commented May 17, 2025

@aras-p I believe the latest revision of the Windows build instructions (before this PR) was written by you. Can you please look this over and make sure I've said the right things and that none the instructions I removed are still essential and should be left in?

Also, general questions for the gallery to ponder:

  • Is the "auto dependency build" (which we use in CI for the Python wheels, among other places) something we should enable by default on Windows? Everywhere? I think we will always need a way for people to turn that off and force it to use only installed dependencies, but I'm wondering if most users would benefit from the default being to auto-build missing dependencies?
  • The Windows build instructions from Aras specify USE_PYTHON=0. Is that the right choice?

@aras-p
Copy link
Contributor

aras-p commented May 18, 2025

Can you please look this over and make sure I've said the right things and that none the instructions I removed are still essential and should be left in?

Yes, much simpler and easier now with the auto-deps build. I tried the instructions as they are in this PR, everything works fine!

Is the "auto dependency build" (which we use in CI for the Python wheels, among other places) something we should enable by default on Windows? Everywhere?

I would go for having it on by default, at least on windows. Maybe everywhere. Obviously everywhere you need a way to turn it off, but having the simplest out of the box experience would be a big plus. That said, I don't know the usual Linux workflows.

The Windows build instructions from Aras specify USE_PYTHON=0. Is that the right choice?

I frankly forget why I used both Python and Qt off in those instructions. Most likely because for my use case I did not need any of them. The same reason why I had "no shared libs, use static linking". Maybe the readme could say that "this builds statically linked OIIO libraries and utilities, without Qt or Python integrations for simplicity" or similar.

A complete side note: the same readme says that visual studio needs to be 2017-2019 version, but I think that should be extended to 2017-2022.

@lgritz
Copy link
Collaborator Author

lgritz commented May 18, 2025

I would go for having it on by default, at least on windows. Maybe everywhere. Obviously everywhere you need a way to turn it off, but having the simplest out of the box experience would be a big plus. That said, I don't know the usual Linux workflows.

My only worry was that while we rely on it for CI, it's new enough that I'm not sure a lot of users have tried it in the wild or what ways it can fail that we haven't anticipated. I guess at least on Windows, the number of people whose builds will break if it's turned on will probably not be worse than the number of people whose builds break now because it can't find a dependency that it needs.

The Windows build instructions from Aras specify USE_PYTHON=0. Is that the right choice?

I frankly forget why I used both Python and Qt off in those instructions. Most likely because for my use case I did not need any of them.

I think, then, I will change the instructions to re-enable them but point out how they can be disabled.

A complete side note: the same readme says that visual studio needs to be 2017-2019 version, but I think that should be extended to 2017-2022.

Thanks. I will fix.

Signed-off-by: Larry Gritz <[email protected]>
@lgritz
Copy link
Collaborator Author

lgritz commented May 19, 2025

I revised the instructions a bit based on the above discussion.

I'm holding off on changing the default BUILD_MISSING_DEPS for Windows, that feels like it should be a totally separate PR to isolate it.

INSTALL.md Outdated

Note that you can speed up the build by disabling certain components if you
know you won't need them: Adding `-DUSE_PYTHON=0` to the command above will
skip building the Python bindings, `-DUSE_QT=0` will disable looking foor and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

foor -> for maybe

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep

Signed-off-by: Larry Gritz <[email protected]>
@lgritz
Copy link
Collaborator Author

lgritz commented May 21, 2025

Merging these docs improvements, Aras said they worked for him.

@lgritz lgritz merged commit 8b9a130 into AcademySoftwareFoundation:main May 21, 2025
3 checks passed
@lgritz lgritz deleted the lg-winbuild branch May 21, 2025 18:26
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request May 21, 2025
…cademySoftwareFoundation#4769)

The INSTALL.md Windows building instructions were out of date in two
important ways: (1) they neglected to mention OpenColorIO as a (newly)
required dependency nor give instructions for how to build it; (2) it
doesn't mention the new BUILD_MISSING_DEPS capabilities.

So basically, this PR modifies our Windows build guidance to set
OpenImageIO_BUILD_MISSING_DEPS=all and rely on that without needing to
document any additional steps for dependencies.

---------

Signed-off-by: Larry Gritz <[email protected]>
@lgritz lgritz added the build / testing / port / CI Affecting the build system, tests, platform support, porting, or continuous integration. label Jul 13, 2025
zachlewis pushed a commit to zachlewis/OpenImageIO that referenced this pull request Aug 1, 2025
…cademySoftwareFoundation#4769)

The INSTALL.md Windows building instructions were out of date in two
important ways: (1) they neglected to mention OpenColorIO as a (newly)
required dependency nor give instructions for how to build it; (2) it
doesn't mention the new BUILD_MISSING_DEPS capabilities.

So basically, this PR modifies our Windows build guidance to set
OpenImageIO_BUILD_MISSING_DEPS=all and rely on that without needing to
document any additional steps for dependencies.

---------

Signed-off-by: Larry Gritz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build / testing / port / CI Affecting the build system, tests, platform support, porting, or continuous integration.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants