Allow loading dlls from PATH on windows and python>=3.8#1668
Allow loading dlls from PATH on windows and python>=3.8#1668anderslanglands wants to merge 2 commits into
Conversation
|
|
|
Is an analogous change required on all projects that have Python bindings? |
It directly related to that: AcademySoftwareFoundation/Imath#238 |
|
I’m working on a PR for you to review right now Larry.
…On Sun, 10 Jul 2022 at 16:27, Larry Gritz ***@***.***> wrote:
Is an analogous change required on all projects that have Python bindings?
—
Reply to this email directly, view it on GitHub
<#1668 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOYQXKUCFQHVIJPSLI3RVLVTJGKPANCNFSM53EJSL5Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thanks for the PR @anderslanglands! Looks like the commits are missing the sign-off to make GitHub action happy. |
|
Hi Rémi, how do I get that?
…On Wed, 13 Jul 2022 at 03:13, Rémi Achard ***@***.***> wrote:
Thanks for the PR @anderslanglands <https://github.com/anderslanglands>!
Looks like the commits are missing the sign-off to make GitHub action happy.
—
Reply to this email directly, view it on GitHub
<#1668 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOYQXIQCJUHPUWMD5XT6X3VTWDQ5ANCNFSM53EJSL5Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@anderslanglands you can use the |
|
Ahh gotcha, thanks. Sorry I’ve not done that before.
I actually was going to push another commit anyway: OIIO recently merged
the same PR, but in that case we decided to invert the logic of the feature
flag to load DLLs from PATH by default (ie match the Python <3.8
behaviour), and let the user disable that with the env var if they want the
3.8 behaviour.
This is also more similar to how USD does it (where they just do the path
loading thing without a way to disable it).
Cheers,
Anders.
…On Wed, 13 Jul 2022 at 21:01, Rémi Achard ***@***.***> wrote:
@anderslanglands <https://github.com/anderslanglands> you can use the
--signoff flag when you commit from the CLI, if you want to sign-off the
last commit you can also use git commit --amend --signoff. GUI tools will
also typically provide a way to do this, I use git-gui which have a
button next to the commit. You will probably need to force push to remove
the un-signed commit from the PR entirely.
—
Reply to this email directly, view it on GitHub
<#1668 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOYQXLNQDGAQ6UNKIEJG63VT2AXJANCNFSM53EJSL5Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hi @anderslanglands, just checking again, do you still plan to update this PR like you mentioned? |
|
@anderslanglands , thanks for identifying this problem and proposing a solution. We fixed this issue in PR #1759, so I'll go ahead and close this PR now. Thanks again for your help! |
|
Thanks Doug!
…On Sat, 18 Mar 2023 at 12:40, Doug Walker ***@***.***> wrote:
@anderslanglands <https://github.com/anderslanglands> , thanks for
identifying this problem and proposing a solution. We fixed this issue in
PR #1759
<#1759>, so
I'll go ahead and close this PR now. Thanks again for your help!
—
Reply to this email directly, view it on GitHub
<#1668 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOYQXM46USUMLBDOFKYOLDW4TY7DANCNFSM53EJSL5Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
As a side note, I think it's important to note that it should be opt-in, not opt-out. This can break a lot of things in an environment and should only be activated manually. Basically, it's forcing a behavior in the entire environment, which could potentially break packages which were not expecting that. |
|
@JeanChristopheMorinPerso, thank you for your comment. We discussed this during the TSC meeting today and the group agreed that having the default be to emulate the Python 3.7 behavior is not what we want as it could be a security risk. I've created issue #1785 for continuing this discussion, if you or Anders have any further suggestions. |
|
Great! Thanks for the update @doug-walker! |
6 participants
python 3.8 stopped loading DLLs from PATH, which breaks PyOpenColorIO when it and its dependencies have been built as shared libraries. This PR provides an optional (gated behind an env var) fix to shim the paths in PATH back in using
os.add_dll_directory(). This is the same approach as taken by USD.