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

[python3] Some patches up for discussion #31279

Closed
wants to merge 7 commits into from

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented May 5, 2023

  • [?] Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@MonicaLiu0311 MonicaLiu0311 added category:port-bug The issue is with a library, which is something the port should already support requires:discussion labels May 6, 2023
@BillyONeal
Copy link
Member

The vcpkg team (@dan-shaw @vicroms @JavierMatosD @ras0219-msft and I) discussed this today.

@ras0219-msft points out that in a vcpkg context we can add the bin directory of the host triplet to the %PATH%, so the add_search_path is only relevant to external-to-vcpkg callers. That makes the goal here somewhat suspicious. "We are not yet opening the can of worms that means becoming an application deployment platform, and that includes Python."

But we acknowledge that we as a team have a big blind spot in our understanding of the python / plugins ... situation. We probably need a better understanding of what you're trying to do here before pceeding.

Robert also points out that we could solve the package ownership problem by creating ports that own just the copy from bin to the right plugins place. (That isn't to say that we want to do that, just pointing it out for the sake of discussion)

@Neumann-A
Copy link
Contributor Author

Neumann-A commented Jun 8, 2023

@ras0219-msft points out that in a vcpkg context we can add the bin directory of the host triplet to the %PATH%,

the problem is that python doesn't care about PATH anymore.

Robert also points out that we could solve the package ownership problem by creating ports that own just the copy from bin to the right plugins place. (That isn't to say that we want to do that, just pointing it out for the sake of discussion)

Worst case scenario would mean an extra port for every port in vcpkg (just for python).

Related:
Info about DLL search paths: https://docs.python.org/3/library/os.html#os.add_dll_directory
https://stackoverflow.com/questions/67805339/is-the-function-os-add-dll-directory-adding-directories-permanently

Basis for this PR: https://github.com/conda-forge/python-feedstock/blob/main/recipe/patches/0012-Add-CondaEcosystemModifyDllSearchPath.patch

(I also tried to embed add_dll_directory in init statements however if you start deploying pythons scripts and the path being added becomes invalid you get a hard error. )

@MonicaLiu0311
Copy link
Contributor

@Neumann-A
Do you want to continue this discussion? If not, I'll remove the requires:discussion tag and convert to draft.

@Neumann-A
Copy link
Contributor Author

Yes this is still up for discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support requires:discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants