Skip to content

[openimageio] Bump to 2.1.11.2#9744

Closed
theblackunknown wants to merge 2 commits intomicrosoft:masterfrom
theblackunknown:openimageio
Closed

[openimageio] Bump to 2.1.11.2#9744
theblackunknown wants to merge 2 commits intomicrosoft:masterfrom
theblackunknown:openimageio

Conversation

@theblackunknown
Copy link
Contributor

@theblackunknown theblackunknown commented Jan 18, 2020

This pull requests bump OpenImageIO port version to 2.1.10

  • What does your PR fix? N/A

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    I have tested a custom triplet x64-windows-static-release, but I am waiting on CI to iterate and validate others
    I haven't touched the CI baseline

  • Does your PR follow the maintainer guide?
    Yes

@theblackunknown theblackunknown force-pushed the openimageio branch 5 times, most recently from 14a695d to 334be28 Compare January 18, 2020 10:24
@theblackunknown theblackunknown changed the title [openimageio] Bump to 2.1.10 + Feature to build tools [openimageio] Bump to 2.1.10 Jan 18, 2020
@NancyLi1013
Copy link
Contributor

Hi @theblackunknown thanks for this PR.
Could you update the requested changes?

@theblackunknown
Copy link
Contributor Author

Now that I am aware of #8543 from @JackBoosY I would suggest to put this one on hold
I'll rebase mine on top of Jack's one once merged

@Pixel-Minions
Copy link

Good morning, version 2.1.11 released. I was having issues using this latest version with the port. It seems the way the patches have to be applied have changed from 2.1.10 to 2.1.11.

@theblackunknown
Copy link
Contributor Author

Hi @huargovfx , I consider this PR on hold for now
The idea is to rebase my changes - and ideally upgrade to latest OpenImageIO - once #8543 is merged

If possible I would suggest to use this other PR is you don't need the latest OpenImageIO, otherwise please be a bit more patient :)

@JackBoosY
Copy link
Contributor

@theblackunknown My PR #8543 has been merged, you can continue your work now : )

@theblackunknown
Copy link
Contributor Author

Thanks ! Will do once I rebase my work !

@theblackunknown theblackunknown changed the title [openimageio] Bump to 2.1.10 [openimageio] Bump to 2.1.11.1 Feb 12, 2020
@theblackunknown theblackunknown changed the title [openimageio] Bump to 2.1.11.1 [openimageio] Bump to 2.1.11.2 Feb 14, 2020
@theblackunknown theblackunknown marked this pull request as ready for review February 14, 2020 23:15
@NancyLi1013
Copy link
Contributor

/azp run

@JackBoosY
Copy link
Contributor

Need to test features.

@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Mar 4, 2020

Hi @theblackunknown

Sorry for the late reply.

All features except for field3d(doesn't support Windows) passed with the following triplets:

  • x86-windows
  • x64-windows
  • x64-windows-static

All features except for(libraw, pybind11) passed with x64-linux triplet.

Could you please help confirm and check the results about libraw and pybind11 on Linux?

@theblackunknown
Copy link
Contributor Author

theblackunknown commented Mar 7, 2020

Hey @NancyLi1013 , I have just tested those features on a Linux OS
I have not encountered any trouble for libraw, however I did for pybind11.

In my opinion, the main issue is how pybind11 port manage its dependencies :

  • On Windows, it relies on python3 from vcpkg port : this provide a consisten, controlled and reproducable behavior
  • On other OSes, it relies on whatever system python is present : this is very inconsistent, and error prone when trying to have reproducable builds on different installations

Now, even if I found the correct package to install to make it work on Linux, I had to further tweak the portfile to make sure openimageio will use the python shared library and not the static one because otherwise the port will fail (because the former is compiled with -fPIC which is what openimageio python bindings requires)

So my last commit is hacky but make it pass for me,
I would suggest to fix the following ports :

What do you think ?
Whether we put this PR in draft/hold, or accept/reject the hack is up to you

@JackBoosY
Copy link
Contributor

@theblackunknown
I think we must guarantee to use the built-in tools (git, python ...) of vcpkg to ensure that the actual effect in each triplet is consistent. And update these tools to make changes in all triplets at the same time.

@theblackunknown
Copy link
Contributor Author

@JackBoosY
Just to be sure to understand, in the current case what are you suggesting to fix the issue ?
Make pybind11 relies on vcpkg port ? vcpkg downloaded python ? or something else ?

@JackBoosY
Copy link
Contributor

@theblackunknown

  • If OIIO depends on pybind, use vcpkg port pybind11 (Add Build-Depends: pybind11 to CONTROL)
  • If OIIO use python3, use vcpkg installed tools python3(Use vcpkg_find_acquire_program)

@theblackunknown
Copy link
Contributor Author

Ok, we agree on this and this is what the current port is currently doing

However the pybind11 port needs an upgrade because the wrong python libraries gets picked up in its current state :

  • pybind11 relies on system python instead of vcpkg python
  • we need to have python with shared library

I am currently tackling this issue in #9979

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theblackunknown
Copy link
Contributor Author

@NancyLi1013 @JackBoosY same as my other PR, this one is also on hold as it requires the ones that are now closed
I am closing it for now until I go back to it

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.

4 participants