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

build(windows): Do not try to build minizip before zlib. #1965

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SlawekNowy
Copy link

No description provided.

Signed-off-by: Sławomir Śpiewak <sosw.slawomir.spiewak@gmail.com>
@doug-walker
Copy link
Collaborator

@SlawekNowy , thanks for the PR. Would you please provide more detail about what scenarios require this fix? In other words, most Windows builds succeed already without this, so it must be something more specific about the way you are building.

@SlawekNowy
Copy link
Author

This is the local windows build. Fetched the repository and built it using Visual Studio generator. Both minizip and zlib are managed by opencolorio. Visual studio 2022 Community x64.

@SlawekNowy
Copy link
Author

Oh. I forgot to mention I run --parallel at cmake build step.

Also it seems that did not fix my problems?

@SlawekNowy SlawekNowy marked this pull request as draft April 18, 2024 14:29
@SlawekNowy SlawekNowy marked this pull request as ready for review April 29, 2024 13:49
@SlawekNowy
Copy link
Author

The build is done using cmake .. from build directory (from ocio dir), and then building generated project.

@SlawekNowy
Copy link
Author

I'll admit this is not standard setup, but this repo's CMakeLists does fetch deps...
And besides that is standard setup on Linux...

@SlawekNowy SlawekNowy marked this pull request as draft May 7, 2024 13:55
@SlawekNowy
Copy link
Author

From parent project's discord:

ok cmake. You get a race condition without the deps' root set, but you don't if I do set the root directories of deps? what?

@stephen-yee-adsk
Copy link

Hi Slawek,

I encountered the same problem on Windows. It seems that minizip-ng sometimes builds before zlib, which causes a build error, because minizip-ng depends on zlib. I was able to fix it locally by adding a dependency on Zlib in the ExternalProject_add call in Installminizip-ng.cmake.

i.e.

    ExternalProject_Add(minizip-ng_install
        GIT_REPOSITORY "https://github.com/zlib-ng/minizip-ng.git"
        GIT_TAG "${minizip-ng_VERSION}"
        GIT_CONFIG advice.detachedHead=false
        GIT_SHALLOW TRUE
        PREFIX "${_EXT_BUILD_ROOT}/libminizip-ng"
        BUILD_BYPRODUCTS ${minizip-ng_LIBRARY}
        CMAKE_ARGS ${minizip-ng_CMAKE_ARGS}
        EXCLUDE_FROM_ALL TRUE
        BUILD_COMMAND ""
        INSTALL_COMMAND
            ${CMAKE_COMMAND} --build .
                            --config ${CMAKE_BUILD_TYPE}
                            --target install
                            --parallel
        DEPENDS ZLIB::ZLIB        # minizip-ng depends on zlib
    )

Could you try this and see if it fixes your build?

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

Successfully merging this pull request may close these issues.

None yet

3 participants