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] add helper to manage python and pip #23089

Merged
merged 21 commits into from
Apr 5, 2022

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Feb 14, 2022

ports which could use this:

  • duktape
  • mesa (changed to use it)
  • opencv2 / opencv3 / opencv4
  • qtinterfaceframework
  • qtwebengine (in 6.3)
  • tensorflow (?)

@Hoikas
Copy link
Contributor

Hoikas commented Feb 15, 2022

This looks way too complicated to me. There is a lot of code for determining the paths of pip executables that could be simplified by simply calling <path_to_python> -m pip [pip arguments here]. If you need to install pip itself, use <path_to_python> -m ensurepip.

@JackBoosY JackBoosY added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Feb 15, 2022
@JackBoosY
Copy link
Contributor

I think we can accept this. Needs more review.

@JackBoosY JackBoosY added requires:author-response requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Feb 15, 2022
@Neumann-A
Copy link
Contributor Author

@Hoikas The script is directly out of the mesa portfile which adapted it out of the duktape/opencv ports. If you think this is too complicated provide a working example for mesa. Be aware that vcpkg_find_acquire_program(PYTHON3) only downloads and extracts python and does no setup it at all.

@vicroms vicroms removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. requires:author-response labels Feb 16, 2022
@ras0219-msft ras0219-msft changed the title add vcpkg-get-python-packages [vcpkg-get-python-packages] add helper to manage python and pip Feb 16, 2022
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to take a look at virtualenv and post back

if (WIN32)
set(PYTHON_OPTION "")
else()
set(PYTHON_OPTION "--user")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--user Install to the Python user install directory for your platform.
Typically ~/.local/, or %APPDATA%\Python on Windows.
(See the Python documentation for site.USER_BASE for full details.)

This still modifies the user's system. I think we should be using a virtual environment instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script is not installing anything if python3 outside of vcpkg is used.
venv could only help installing packages if python3 outside of vcpkg is used. So we wouldn't throw an error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, this should be:

Suggested change
set(PYTHON_OPTION "--user")
vcpkg_list(SET python_user_param "--user")

@JackBoosY JackBoosY requested a review from strega-nil-ms March 17, 2022 03:04
@strega-nil-ms
Copy link
Contributor

Can we do this instead?

From 7c3f98be33bf07770636a93bf98ea479975c33a7 Mon Sep 17 00:00:00 2001
From: nicole mazzuca <mazzucan@outlook.com>
Date: Wed, 23 Mar 2022 15:31:26 -0700
Subject: [PATCH] actually install stuff locally

This installs pip packages per-port
---
 ports/mesa/portfile.cmake                     | 12 ++--
 .../x_vcpkg_get_python_packages.cmake         | 61 ++++++-------------
 2 files changed, 23 insertions(+), 50 deletions(-)

diff --git a/ports/mesa/portfile.cmake b/ports/mesa/portfile.cmake
index 5fa919888..2dcc52247 100644
--- a/ports/mesa/portfile.cmake
+++ b/ports/mesa/portfile.cmake
@@ -36,7 +36,7 @@ vcpkg_add_to_path("${PYTHON3_DIR}")
 vcpkg_add_to_path("${PYTHON3_DIR}/Scripts")
 set(ENV{PYTHON} "${PYTHON3}")
 
-x_vcpkg_get_python_packages(PYTHON_EXECUTABLE "${PYTHON3}" PACKAGES setuptools mako)
+x_vcpkg_get_python_packages(PACKAGES setuptools mako)
 
 vcpkg_find_acquire_program(FLEX)
 get_filename_component(FLEX_DIR "${FLEX}" DIRECTORY )
@@ -118,12 +118,12 @@ list(APPEND MESA_OPTIONS -Dshared-glapi=enabled)  #shared GLAPI required when bu
 if(VCPKG_TARGET_IS_WINDOWS)
     list(APPEND MESA_OPTIONS -Dplatforms=['windows'])
     list(APPEND MESA_OPTIONS -Dmicrosoft-clc=disabled)
-    if(NOT VCPKG_TARGET_IS_MINGW)
-        set(VCPKG_CXX_FLAGS "/D_CRT_DECLARE_NONSTDC_NAMES ${VCPKG_CXX_FLAGS}")
-        set(VCPKG_C_FLAGS "/D_CRT_DECLARE_NONSTDC_NAMES ${VCPKG_C_FLAGS}")
-    endif()
+    if(NOT VCPKG_TARGET_IS_MINGW)
+        set(VCPKG_CXX_FLAGS "/D_CRT_DECLARE_NONSTDC_NAMES ${VCPKG_CXX_FLAGS}")
+        set(VCPKG_C_FLAGS "/D_CRT_DECLARE_NONSTDC_NAMES ${VCPKG_C_FLAGS}")
+    endif()
 endif()
-
+
 vcpkg_configure_meson(
     SOURCE_PATH "${SOURCE_PATH}"
     OPTIONS 
diff --git a/ports/vcpkg-get-python-packages/x_vcpkg_get_python_packages.cmake b/ports/vcpkg-get-python-packages/x_vcpkg_get_python_packages.cmake
index 1fee61281..8e5e960dc 100644
--- a/ports/vcpkg-get-python-packages/x_vcpkg_get_python_packages.cmake
+++ b/ports/vcpkg-get-python-packages/x_vcpkg_get_python_packages.cmake
@@ -23,11 +23,14 @@ List of python packages to acquire
 include_guard(GLOBAL)
 
 function(x_vcpkg_get_python_packages)
-    cmake_parse_arguments(PARSE_ARGV 0 arg "" "PYTHON_EXECUTABLE" "PACKAGES")
+    cmake_parse_arguments(PARSE_ARGV 0 arg "" "PYTHON_VERSION" "PACKAGES")
     
-    if(NOT DEFINED arg_PYTHON_EXECUTABLE)
-        message(FATAL_ERROR "PYTHON_EXECUTABLE must be specified.")
+    if(NOT DEFINED arg_PYTHON_VERSION)
+        set(arg_PYTHON_VERSION 3)
+    elseif(NOT (arg_PYTHON_VERSION EQUAL "2" OR arg_PYTHON_VERSION EQUAL "3"))
+        message(FATAL_ERROR "Unsupported python version ${arg_PYTHON_VERSION}")
     endif()
+
     if(NOT DEFINED arg_PACKAGES)
         message(FATAL_ERROR "PACKAGES must be specified.")
     endif()
@@ -35,47 +38,17 @@ function(x_vcpkg_get_python_packages)
         message(FATAL_ERROR "${CMAKE_CURRENT_FUNCTION} was passed extra arguments: ${arg_UNPARSED_ARGUMENTS}")
     endif()
 
-    get_filename_component(python_dir "${arg_PYTHON_EXECUTABLE}" DIRECTORY)
-
-    if (WIN32)
-        set(PYTHON_OPTION "")
-    else()
-        set(PYTHON_OPTION "--user")
-    endif()
+    set(python_variable "PYTHON${arg_PYTHON_VERSION}")
+    vcpkg_find_acquire_program("${python_variable}")
+    set(python_program "${${python_variable}}")
 
-    if("${python_dir}" MATCHES "(${DOWNLOADS}|${CURRENT_HOST_INSTALLED_DIR})") # inside vcpkg
-        if(NOT EXISTS "${python_dir}/easy_install${VCPKG_HOST_EXECUTABLE_SUFFIX}")
-            if(NOT EXISTS "${python_dir}/Scripts/pip${VCPKG_HOST_EXECUTABLE_SUFFIX}")
-                vcpkg_from_github(
-                    OUT_SOURCE_PATH PYFILE_PATH
-                    REPO pypa/get-pip
-                    REF 309a56c5fd94bd1134053a541cb4657a4e47e09d #2019-08-25
-                    SHA512 bb4b0745998a3205cd0f0963c04fb45f4614ba3b6fcbe97efe8f8614192f244b7ae62705483a5305943d6c8fedeca53b2e9905aed918d2c6106f8a9680184c7a
-                )
-                vcpkg_execute_required_process(COMMAND "${arg_PYTHON_EXECUTABLE}" "${PYFILE_PATH}/get-pip.py" ${PYTHON_OPTION}
-                                               WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}")
-            endif()
-            foreach(_package IN LISTS arg_PACKAGES)
-                vcpkg_execute_required_process(COMMAND "${python_dir}/Scripts/pip${VCPKG_HOST_EXECUTABLE_SUFFIX}" install ${_package} ${PYTHON_OPTION}
-                                               WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}")
-            endforeach()
-        else()
-            foreach(_package IN LISTS arg_PACKAGES)
-                vcpkg_execute_required_process(COMMAND "${python_dir}/easy_install${VCPKG_HOST_EXECUTABLE_SUFFIX}" ${_package}
-                                               WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}")
-            endforeach()
-        endif()
-        if(NOT VCPKG_TARGET_IS_WINDOWS)
-            vcpkg_execute_required_process(COMMAND pip3 install ${arg_PACKAGES})
-        endif()
-    else() # outside vcpkg
-        foreach(package IN LISTS arg_PACKAGES)
-            vcpkg_execute_in_download_mode(COMMAND ${arg_PYTHON_EXECUTABLE} -c "import ${package}" RESULT_VARIABLE HAS_ERROR)
+    set(ENV{PYTHONUSERBASE} "${CURRENT_BUILDTREES_DIR}/vcpkg-python-packages")
 
-            if(HAS_ERROR)
-                message(FATAL_ERROR "Python package '${package}' needs to be installed for port '${PORT}'.\nComplete list of required python packages: ${arg_PACKAGES}")
-
-            endif()
-        endforeach()
-    endif()
+    foreach(package IN LISTS arg_PACKAGES)
+        vcpkg_execute_required_process(
+            COMMAND "${python_program}" -m pip install --user ${package}
+            WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}"
+						LOGNAME "pip-install-${package}"
+        )
+    endforeach()
 endfunction()
-- 
2.32.0 (Apple Git-132)

@Neumann-A
Copy link
Contributor Author

Neumann-A commented Mar 23, 2022

Can we do this instead?

Can we leave this up to another PR ?
The original python stuff was added in #5937 by @vicroms in the duktape port I assume.

@strega-nil-ms
Copy link
Contributor

@Neumann-A I mean we could, but I don't see what the point of this PR is if we don't actually make the helper any different.

@Neumann-A
Copy link
Contributor Author

Neumann-A commented Mar 23, 2022

I mean we could, but I don't see what the point of this PR is if we don't actually make the helper any different.

The point of this PR is to provide a port with commonly used code by other ports without the need to duplicate the code itself. I mean I can alternatively keep duplicating that specific code for qtwebengine in the upcoming Qt 6.3 release .....

I also want to point to: #23089 (comment)

BillyONeal added a commit to BillyONeal/vcpkg that referenced this pull request Mar 28, 2022
* [Most recent nightly build failed](https://dev.azure.com/vcpkg/public/_build/results?buildId=69427)
* [Validation of this tool update failed](https://dev.azure.com/vcpkg/public/_build/results?buildId=69417)

## Common to both:

PASSING, REMOVE FROM FAIL LIST: chartdir:x64-windows (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: chartdir:x64-windows-static-md (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: chartdir:x86-windows (.\scripts\ci.baseline.txt)

Probably fixed by microsoft#23701

PASSING, REMOVE FROM FAIL LIST: gmp:x64-uwp (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: gmp:x64-windows-static-md (.\scripts\ci.baseline.txt)

Probably fixed by microsoft#23466 ?

REGRESSION: colmap:x64-windows-static-md failed with BUILD_FAILED. If expected, add colmap:x64-windows-static-md=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

I don't know exactly what changed. I observe that
* this thing depends on a *lot* of stuff
* on March 14 we didn't even attempt to build this
* the x64-windows ones are already in the baseline

so I skipped it.

REGRESSION: qtdeclarative:x64-windows. If expected, add qtdeclarative:x64-windows=fail to .\scripts\ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

This is a reporting change: The new world order also includes host build failures which is why it's duplicated.

See also microsoft#23714
See also microsoft#23490

I'm nervous about baslining this because it seems most of the qt world is built on top of this port

I filed microsoft#23824 about this and @Neumann-A indicated this should be fixed by microsoft#23755

REGRESSION: nettle:x64-uwp. If expected, add nettle:x64-uwp=fail to .\scripts\ci.baseline.txt.
REGRESSION: nettle:x64-windows-static-md. If expected, add nettle:x64-windows-static-md=fail to .\scripts\ci.baseline.txt.
REGRESSION: nettle:x64-uwp failed with BUILD_FAILED. If expected, add nettle:x64-uwp=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: nettle:x64-windows-static-md failed with POST_BUILD_CHECKS_FAILED. If expected, add nettle:x64-windows-static-md=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

Didn't analyze, probably fixed by microsoft#23519 ?

REGRESSION: libgpg-error:x64-uwp. If expected, add libgpg-error:x64-uwp=fail to .\scripts\ci.baseline.txt.
REGRESSION: libgpg-error:x64-uwp failed with BUILD_FAILED. If expected, add libgpg-error:x64-uwp=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

This was broken by VS2022 update:
```
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VisualStudio\v17.0\AppxPackage\Microsoft.AppXPackage.Targets(892,25): error MSB4086: A numeric comparison was attempted on "$(TargetPlatformMinVersion)" that evaluates to "" instead of a number, in condition "'$(TargetPlatformMinVersion)' >= '10.0.17200.0'". [C:\Dev\vcpkg\buildtrees\libgpg-error\x64-uwp-rel\error-1.42-2324ddbc71.clean\SMP\libgpg-error_winrt.vcxproj]
```

REGRESSION: libmikmod:x64-osx. If expected, add libmikmod:x64-osx=fail to .\scripts\ci.baseline.txt.
REGRESSION: libmikmod:x64-osx failed with BUILD_FAILED. If expected, add libmikmod:x64-osx=fail to /Users/vagrant/Data/work/2/s/scripts/azure-pipelines/../ci.baseline.txt.

Broken between [2022-03-16](https://dev.azure.com/vcpkg/public/_build/results?buildId=68947) and [2022-03-18](https://dev.azure.com/vcpkg/public/_build/results?buildId=69051). Unfortunately I don't see obvious reasons why. Nothing else depends on this and nobody has noticed in 2 weeks, so I'm baslining it for now. (Will investigate shortly...)

## Only broken in tool update:

REGRESSION: mesa:x64-windows failed with BUILD_FAILED. If expected, add mesa:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

```
-- Downloading https://gitlab.freedesktop.org/mesa/mesa/-/archive/mesa-21.2.5/mesa-mesa-21.2.5.tar.gz -> mesa-mesa-mesa-21.2.5-1.tar.gz...
-- Extracting source /Users/vagrant/Data/downloads/mesa-mesa-mesa-21.2.5-1.tar.gz
-- Applying patch swravx512-post-static-link.patch
-- Applying patch swr-msvc-2.patch
-- Applying patch swr-llvm13.patch
-- Applying patch radv-msvc-llvm13-2.patch
-- Applying patch d3d10sw.patch
-- Using source at /Users/vagrant/Data/buildtrees/mesa/src/esa-21.2.5-2df234d2b1.clean
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'mako'
CMake Error at ports/mesa/portfile.cmake:85 (message):
  Python package 'mako' needs to be installed for port 'mesa'.

  Complete list of required python packages: setuptools;mako
Call Stack (most recent call first):
  ports/mesa/portfile.cmake:91 (vcpkg_get_python_package)
  scripts/ports.cmake:145 (include)
```

Looks like this is being tracked by microsoft#23089 ; perhaps that we don't have as aggressive a recycling strategy for macos boxes as we do for the others has let different machines give different results?

## Only broken without tool update:

REGRESSION: chromium-base:x64-osx. If expected, add chromium-base:x64-osx=fail to .\scripts\ci.baseline.txt.

This one has been constantly flaky; I baselined it.

REGRESSION: libxml2:x64-osx. If expected, add libxml2:x64-osx=fail to .\scripts\ci.baseline.txt.

This port uses vcpkg_from_git and the upstream server was down during the build.
BillyONeal added a commit that referenced this pull request Mar 28, 2022
* Update vcpkg-tool to 2022-03-24

* Hook up microsoft/vcpkg-tool#345

* Hook up microsoft/vcpkg-tool#442

* Update vcpkg-tool to 2022-03-25

* Analysis of failures.

* [Most recent nightly build failed](https://dev.azure.com/vcpkg/public/_build/results?buildId=69427)
* [Validation of this tool update failed](https://dev.azure.com/vcpkg/public/_build/results?buildId=69417)

## Common to both:

PASSING, REMOVE FROM FAIL LIST: chartdir:x64-windows (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: chartdir:x64-windows-static-md (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: chartdir:x86-windows (.\scripts\ci.baseline.txt)

Probably fixed by #23701

PASSING, REMOVE FROM FAIL LIST: gmp:x64-uwp (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: gmp:x64-windows-static-md (.\scripts\ci.baseline.txt)

Probably fixed by #23466 ?

REGRESSION: colmap:x64-windows-static-md failed with BUILD_FAILED. If expected, add colmap:x64-windows-static-md=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

I don't know exactly what changed. I observe that
* this thing depends on a *lot* of stuff
* on March 14 we didn't even attempt to build this
* the x64-windows ones are already in the baseline

so I skipped it.

REGRESSION: qtdeclarative:x64-windows. If expected, add qtdeclarative:x64-windows=fail to .\scripts\ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

This is a reporting change: The new world order also includes host build failures which is why it's duplicated.

See also #23714
See also #23490

I'm nervous about baslining this because it seems most of the qt world is built on top of this port

I filed #23824 about this and @Neumann-A indicated this should be fixed by #23755

REGRESSION: nettle:x64-uwp. If expected, add nettle:x64-uwp=fail to .\scripts\ci.baseline.txt.
REGRESSION: nettle:x64-windows-static-md. If expected, add nettle:x64-windows-static-md=fail to .\scripts\ci.baseline.txt.
REGRESSION: nettle:x64-uwp failed with BUILD_FAILED. If expected, add nettle:x64-uwp=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: nettle:x64-windows-static-md failed with POST_BUILD_CHECKS_FAILED. If expected, add nettle:x64-windows-static-md=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

Didn't analyze, probably fixed by #23519 ?

REGRESSION: libgpg-error:x64-uwp. If expected, add libgpg-error:x64-uwp=fail to .\scripts\ci.baseline.txt.
REGRESSION: libgpg-error:x64-uwp failed with BUILD_FAILED. If expected, add libgpg-error:x64-uwp=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

This was broken by VS2022 update:
```
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VisualStudio\v17.0\AppxPackage\Microsoft.AppXPackage.Targets(892,25): error MSB4086: A numeric comparison was attempted on "$(TargetPlatformMinVersion)" that evaluates to "" instead of a number, in condition "'$(TargetPlatformMinVersion)' >= '10.0.17200.0'". [C:\Dev\vcpkg\buildtrees\libgpg-error\x64-uwp-rel\error-1.42-2324ddbc71.clean\SMP\libgpg-error_winrt.vcxproj]
```

REGRESSION: libmikmod:x64-osx. If expected, add libmikmod:x64-osx=fail to .\scripts\ci.baseline.txt.
REGRESSION: libmikmod:x64-osx failed with BUILD_FAILED. If expected, add libmikmod:x64-osx=fail to /Users/vagrant/Data/work/2/s/scripts/azure-pipelines/../ci.baseline.txt.

Broken between [2022-03-16](https://dev.azure.com/vcpkg/public/_build/results?buildId=68947) and [2022-03-18](https://dev.azure.com/vcpkg/public/_build/results?buildId=69051). Unfortunately I don't see obvious reasons why. Nothing else depends on this and nobody has noticed in 2 weeks, so I'm baslining it for now. (Will investigate shortly...)

## Only broken in tool update:

REGRESSION: mesa:x64-windows failed with BUILD_FAILED. If expected, add mesa:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

```
-- Downloading https://gitlab.freedesktop.org/mesa/mesa/-/archive/mesa-21.2.5/mesa-mesa-21.2.5.tar.gz -> mesa-mesa-mesa-21.2.5-1.tar.gz...
-- Extracting source /Users/vagrant/Data/downloads/mesa-mesa-mesa-21.2.5-1.tar.gz
-- Applying patch swravx512-post-static-link.patch
-- Applying patch swr-msvc-2.patch
-- Applying patch swr-llvm13.patch
-- Applying patch radv-msvc-llvm13-2.patch
-- Applying patch d3d10sw.patch
-- Using source at /Users/vagrant/Data/buildtrees/mesa/src/esa-21.2.5-2df234d2b1.clean
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'mako'
CMake Error at ports/mesa/portfile.cmake:85 (message):
  Python package 'mako' needs to be installed for port 'mesa'.

  Complete list of required python packages: setuptools;mako
Call Stack (most recent call first):
  ports/mesa/portfile.cmake:91 (vcpkg_get_python_package)
  scripts/ports.cmake:145 (include)
```

Looks like this is being tracked by #23089 ; perhaps that we don't have as aggressive a recycling strategy for macos boxes as we do for the others has let different machines give different results?

## Only broken without tool update:

REGRESSION: chromium-base:x64-osx. If expected, add chromium-base:x64-osx=fail to .\scripts\ci.baseline.txt.

This one has been constantly flaky; I baselined it.

REGRESSION: libxml2:x64-osx. If expected, add libxml2:x64-osx=fail to .\scripts\ci.baseline.txt.

This port uses vcpkg_from_git and the upstream server was down during the build.

* Restore chartdir to the baseline, I thought #23732 had been merged.
@Neumann-A
Copy link
Contributor Author

@strega-nil-ms: Your approach won't work since pip is not available in the embedded version of python3 vcpkg downloads on windows.

@strega-nil-ms
Copy link
Contributor

strega-nil-ms commented Apr 1, 2022

@Neumann-A hmm, after doing some research, it seems to me that we're making a mistake by using the embedded version of python, given that we do want to have access to pip...

Edit: after doing more research, it seems to me that we should be using the nuget package, since it is easily downloadable and unpackable, but does not miss out on pip.

However, until such point as we do that, it's not unreasonable imo to continue the weird BS to get pip working with the python we download. The more important thing is that we shouldn't be installing stuff in user global locations, ever.

@Neumann-A
Copy link
Contributor Author

Edit: after doing more research, it seems to me that we should be using the nuget package, since it is easily downloadable and unpackable, but does not miss out on pip.

I think it doesn't really matter if we download python with pip or without it.
Considering that the main python usage within vcpkg is limited to invoking meson and meson not requiring any additional packages, it is reasonable to not have pip by default. Only a few ports really want to have additional python packages.

Instead of doing:

vcpkg_list(SET post_install_command "${CMAKE_COMMAND}" -E rm python310._pth)

we could string replace within it and changing python310.zip to

python310.zip
Lib
Lib/site-packages

I wrote a batch file this week which mimics what vcpkg does but replaces the python310._pth with a version which contains the above changes:

curl https://www.python.org/ftp/python/3.10.4/python-3.10.4-embed-amd64.zip --output python3.10.4-x64.zip
mkdir python3
cd python3
tar -xf ..\python3.10.4-x64.zip
curl -sSL https://bootstrap.pypa.io/get-pip.py -o get-pip.py
python.exe get-pip.py --no-warn-script-location
rem replace python310._pth with my version
copy python310._pth python310._pth.bak
del python310._pth
copy ..\python310._pth python310._pth
python.exe -m pip install matplotlib numpy opencv-python torch jupyterlab --no-warn-script-location
cd ..

The more important thing is that we shouldn't be installing stuff in user global locations, ever.

we never do. We only download python on windows and only install packages there.
I removed some stuff in 03addea which seems to be unnecessary considering the above.

@strega-nil-ms
Copy link
Contributor

strega-nil-ms commented Apr 1, 2022

Mmh, yeah, I'm much happier with that change.

Alright, I'm approve now.

@strega-nil-ms strega-nil-ms added info:reviewed Pull Request changes follow basic guidelines and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Apr 1, 2022
@strega-nil-ms strega-nil-ms merged commit 5bf1323 into microsoft:master Apr 5, 2022
@strega-nil-ms
Copy link
Contributor

Thanks @Neumann-A

@Neumann-A Neumann-A deleted the vcpkg-get-python-packages branch April 5, 2022 19:11
@Osyotr
Copy link
Contributor

Osyotr commented Apr 5, 2022

meson not requiring any additional packages

I remember getting errors about missing setuptools on a fresh linux VM when building ports with meson.
UPD: https://mesonbuild.com/Quick-guide.html#installation-using-package-manager

@Neumann-A
Copy link
Contributor Author

then we should add a dependency to vcpkg-get-python-packages in vcpkg-tool-meson and get the required python packages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants