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

[vcpkg-get-python-packages] Fix exported variable #36354

Merged
merged 6 commits into from
Jan 31, 2024

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jan 24, 2024

Noticed while trying to use another variable name then PYTHON3 with lensfun.
In addition:

  • fix typos
  • make precompiled archive optional.

@MonicaLiu0311 MonicaLiu0311 self-assigned this Jan 25, 2024
MonicaLiu0311
MonicaLiu0311 previously approved these changes Jan 25, 2024
@MonicaLiu0311 MonicaLiu0311 added the category:port-bug The issue is with a library, which is something the port should already support label Jan 25, 2024
@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 25, 2024

if(arg_PYTHON_VERSION STREQUAL 3)
file(COPY "${python_dir}/python3${PYTHON_VERSION_MINOR}.zip" DESTINATION "${venv_path}/Scripts")
endif()

This file isn't installed by python3:x64-windows. So the function still breaks when used with the executable from that port.
https://dev.azure.com/vcpkg/public/_build/results?buildId=98885&view=logs&j=7922e5c4-0103-5f8f-ad17-45ce9bb98e80&t=66eef212-08d6-5e6c-a560-7d01149d0188
CC @Neumann-A

@dg0yt dg0yt marked this pull request as draft January 25, 2024 07:17
@Neumann-A
Copy link
Contributor

file(COPY "${python_dir}/python3${PYTHON_VERSION_MINOR}.zip" DESTINATION "${venv_path}/Scripts")

probably needs to be generated by the python port itself. Ran already into several issues with embedded python not finding the default scripts but need a good way to compose other modules into that.

@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 28, 2024

Hm. I wonder if the actual problem with this port/function is that pip install bypasses asset caching.

@Neumann-A
Copy link
Contributor

It should only install into the venv not into the installed tree.

@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 28, 2024

It should only install into the venv not into the installed tree.

Yes. And still it is a problem for disconnected builds after --only-downloads. Unlike most tools which also aren't installed into the installed tree.

Pip has its own caching. And it is also possible to pre-download to a local downloads dir and to install later from that dir:
https://pip.pypa.io/en/stable/user_guide/#installing-from-local-packages
Maybe this will eventually need an injection point and instruction on how to prepare and pass in the local dir. Which may require to build python3 first to get matching downloads.

(I'm also looking at jinja2: Repeatedly popping up now as a libsystemd requirement since this port was allowed to enter this registry, https://github.com/microsoft/vcpkg/issues?q=is%3Aissue+libsystemd+jinja2)

@dg0yt dg0yt marked this pull request as ready for review January 30, 2024 20:24
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Jan 31, 2024
@data-queue data-queue merged commit d5e173c into microsoft:master Jan 31, 2024
16 checks passed
@dg0yt dg0yt deleted the python-packages branch February 1, 2024 05:07
TomKatom pushed a commit to TomKatom/vcpkg that referenced this pull request Feb 23, 2024
* [vcpkg-get-python-packages] Update manifest

* [vcpkg-get-python-packages] Fix typos

* [vcpkg-get-python-packages] Fix exported variable

* [vcpkg-get-python-packages] Make python3 zip optional

* versions
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 info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants