public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation
@ 2023-05-06 19:30 Rebecca Cran
  2023-05-06 19:30 ` [PATCH v3 1/4] BaseTools: Remove Python2/Python3 detection from toolset.bat Rebecca Cran
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Rebecca Cran @ 2023-05-06 19:30 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Liming Gao,
	Bob Feng, Yuwei Chen
  Cc: Rebecca Cran

There are remnants of Python 2 support in BaseTools/toolsetup.bat that it's
probably time to remove since we only support Python 3.6 and newer these days.
So, remove the variables that enable Python3 support and simplify the batch
script. I've also seen errors where after running edksetup.bat the build
command isn't available because PYTHONPATH wasn't being set, so fix that
when the Pip BaseTools are being used.

At the same time, let's add a check that we meet the minimum version
requirement so we don't end up failing with an obscure error.

Building BaseTools causes a warning about threading.currentThread being
deprecated, so update code in NmakeSubdirs.py to switch to
threading.current_thread.

There needs to be further work, because if PYTHON_COMMAND isn't specified then
it defaults to "py -3", where py is C:\Windows\py.exe, which doesn't work if
you're using a virtualenv since it installs python.exe and pythonw.exe in
venv\Scripts. toolsetup.bat therefore fails to detect the Pip BaseTools and
uses the in-source Basetools.

GitHub PR: https://github.com/tianocore/edk2/pull/4302
GitHub branch: https://github.com/bcran/edk2/tree/py3

Changes between v1 and v2
=========================

- Require Python 3.6 or newer: 3.6 was when PEP 526 was added, which we use.
- Fix Tests/RunTests.py on Windows.

Rebecca Cran (4):
  BaseTools: Remove Python2/Python3 detection from toolset.bat
  BaseTools: use threading.current_thread in NmakeSubdirs.py
  edksetup.bat: if toolsetup.bat fails, just exit
  BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver

 BaseTools/Source/C/Makefiles/NmakeSubdirs.py |   2 +-
 BaseTools/Tests/PythonTest.py                |  22 +++-
 BaseTools/toolsetup.bat                      | 119 ++++++++------------
 edksetup.bat                                 |   1 +
 4 files changed, 69 insertions(+), 75 deletions(-)

-- 
2.40.0.windows.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/4] BaseTools: Remove Python2/Python3 detection from toolset.bat
  2023-05-06 19:30 [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation Rebecca Cran
@ 2023-05-06 19:30 ` Rebecca Cran
  2023-05-06 19:30 ` [PATCH v3 2/4] BaseTools: use threading.current_thread in NmakeSubdirs.py Rebecca Cran
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Rebecca Cran @ 2023-05-06 19:30 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Liming Gao,
	Bob Feng, Yuwei Chen
  Cc: Rebecca Cran

Since Python3 is now required, we can remove the checks for PYTHON3_ENABLE
and PYTHON3 and simplify the code in toolsetup.bat. Also, remove the
leftover from when we supported freezing Python code.

While here, fix a couple of typos and improve error messages.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 BaseTools/toolsetup.bat | 64 +++++---------------
 1 file changed, 16 insertions(+), 48 deletions(-)

diff --git a/BaseTools/toolsetup.bat b/BaseTools/toolsetup.bat
index 25d13d559cd6..3d13e9fad286 100755
--- a/BaseTools/toolsetup.bat
+++ b/BaseTools/toolsetup.bat
@@ -305,18 +305,8 @@ goto check_build_environment
   )
 
 :defined_python
-if defined PYTHON_COMMAND if not defined PYTHON3_ENABLE (
-  goto check_python_available
-)
-if defined PYTHON3_ENABLE (
-  if "%PYTHON3_ENABLE%" EQU "TRUE" (
-    set PYTHON_COMMAND=py -3
-    goto check_python_available
-  ) else (
-    goto check_python2
-  )
-)
-if not defined PYTHON_COMMAND if not defined PYTHON3_ENABLE (
+
+if not defined PYTHON_COMMAND (
   set PYTHON_COMMAND=py -3
   py -3 %BASE_TOOLS_PATH%\Tests\PythonTest.py >PythonCheck.txt 2>&1
   setlocal enabledelayedexpansion
@@ -328,56 +318,40 @@ if not defined PYTHON_COMMAND if not defined PYTHON3_ENABLE (
       set PYTHON_COMMAND=
       echo.
       echo !!! ERROR !!! Binary python tools are missing.
-      echo PYTHON_COMMAND, PYTHON3_ENABLE or PYTHON_HOME
-      echo Environment variable is not set successfully.
-      echo They is required to build or execute the python tools.
+      echo PYTHON_COMMAND or PYTHON_HOME
+      echo Environment variable is not set correctly.
+      echo They are required to build or execute the python tools.
       echo.
       goto end
-    ) else (
-      goto check_python2
     )
-  ) else (
-    goto check_freezer_path
   )
 )
 
-:check_python2
 endlocal
+
 if defined PYTHON_HOME (
   if EXIST "%PYTHON_HOME%" (
     set PYTHON_COMMAND=%PYTHON_HOME%\python.exe
-    goto check_python_available
+  ) else (
+    echo .
+    echo !!! ERROR !!!  PYTHON_HOME="%PYTHON_HOME%" does not exist.
+    echo .
+    goto end
   )
 )
-if defined PYTHONHOME (
-  if EXIST "%PYTHONHOME%" (
-    set PYTHON_HOME=%PYTHONHOME%
-    set PYTHON_COMMAND=%PYTHON_HOME%\python.exe
-    goto check_python_available
-  )
-)
-echo.
-echo !!! ERROR !!!  PYTHON_HOME is not defined or The value of this variable does not exist
-echo.
-goto end
-:check_python_available
+
 %PYTHON_COMMAND% %BASE_TOOLS_PATH%\Tests\PythonTest.py >PythonCheck.txt 2>&1
   setlocal enabledelayedexpansion
   set /p PythonCheck=<"PythonCheck.txt"
   del PythonCheck.txt
   if "!PythonCheck!" NEQ "TRUE" (
     echo.
-    echo ! ERROR !  "%PYTHON_COMMAND%" is not installed or added to environment variables
+    echo ! ERROR !  PYTHON_COMMAND="%PYTHON_COMMAND%" is not installed or added to environment variables
     echo.
     goto end
-  ) else (
-    goto check_freezer_path
-  )
+)
 
-
-
-:check_freezer_path
-  endlocal
+endlocal
 
   %PYTHON_COMMAND% -c "import edk2basetools" >NUL 2>NUL
   if %ERRORLEVEL% EQU 0 (
@@ -404,13 +378,7 @@ goto end
 
 :print_python_info
   echo                PATH = %PATH%
-  if defined PYTHON3_ENABLE if "%PYTHON3_ENABLE%" EQU "TRUE" (
-    echo      PYTHON3_ENABLE = %PYTHON3_ENABLE%
-    echo             PYTHON3 = %PYTHON_COMMAND%
-  ) else (
-    echo      PYTHON3_ENABLE = FALSE
-    echo      PYTHON_COMMAND = %PYTHON_COMMAND%
-  )
+  echo      PYTHON_COMMAND = %PYTHON_COMMAND%
   echo          PYTHONPATH = %PYTHONPATH%
   echo.
 
-- 
2.40.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/4] BaseTools: use threading.current_thread in NmakeSubdirs.py
  2023-05-06 19:30 [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation Rebecca Cran
  2023-05-06 19:30 ` [PATCH v3 1/4] BaseTools: Remove Python2/Python3 detection from toolset.bat Rebecca Cran
@ 2023-05-06 19:30 ` Rebecca Cran
  2023-05-06 19:30 ` [PATCH v3 3/4] edksetup.bat: if toolsetup.bat fails, just exit Rebecca Cran
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Rebecca Cran @ 2023-05-06 19:30 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Liming Gao,
	Bob Feng, Yuwei Chen
  Cc: Rebecca Cran

threading.currentThread is a deprecated alias for
threading.current_thread, and causes a warning to be displayed when it's
called. Update NmakeSubdirs.py to use the latter method instead.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 BaseTools/Source/C/Makefiles/NmakeSubdirs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/Makefiles/NmakeSubdirs.py b/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
index 1f4a45004f4b..7860c040afa0 100644
--- a/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
+++ b/BaseTools/Source/C/Makefiles/NmakeSubdirs.py
@@ -132,7 +132,7 @@ class ThreadControl(object):
                 break
 
         self.runningLock.acquire(True)
-        self.running.remove(threading.currentThread())
+        self.running.remove(threading.current_thread())
         self.runningLock.release()
 
 def Run():
-- 
2.40.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/4] edksetup.bat: if toolsetup.bat fails, just exit
  2023-05-06 19:30 [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation Rebecca Cran
  2023-05-06 19:30 ` [PATCH v3 1/4] BaseTools: Remove Python2/Python3 detection from toolset.bat Rebecca Cran
  2023-05-06 19:30 ` [PATCH v3 2/4] BaseTools: use threading.current_thread in NmakeSubdirs.py Rebecca Cran
@ 2023-05-06 19:30 ` Rebecca Cran
  2023-05-06 19:30 ` [PATCH v3 4/4] BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver Rebecca Cran
  2023-05-08  1:30 ` 回复: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation gaoliming
  4 siblings, 0 replies; 9+ messages in thread
From: Rebecca Cran @ 2023-05-06 19:30 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Liming Gao,
	Bob Feng, Yuwei Chen
  Cc: Rebecca Cran

If toolsetup.bat fails (i.e. exits with a non-zero %ERRORLEVEL%), don't
try and carry on but just quit.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 edksetup.bat | 1 +
 1 file changed, 1 insertion(+)

diff --git a/edksetup.bat b/edksetup.bat
index 2fdf130e00e2..71ceefb32742 100755
--- a/edksetup.bat
+++ b/edksetup.bat
@@ -86,6 +86,7 @@ if exist %EDK_TOOLS_PATH%\Source set BASE_TOOLS_PATH=%EDK_TOOLS_PATH%
 :checkBaseTools
 IF NOT EXIST "%EDK_TOOLS_PATH%\toolsetup.bat" goto BadBaseTools
 call %EDK_TOOLS_PATH%\toolsetup.bat %*
+if %ERRORLEVEL% NEQ 0 goto end
 if /I "%1"=="Reconfig" shift
 goto check_NASM
 goto check_cygwin
-- 
2.40.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 4/4] BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver
  2023-05-06 19:30 [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation Rebecca Cran
                   ` (2 preceding siblings ...)
  2023-05-06 19:30 ` [PATCH v3 3/4] edksetup.bat: if toolsetup.bat fails, just exit Rebecca Cran
@ 2023-05-06 19:30 ` Rebecca Cran
  2023-05-08  1:30 ` 回复: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation gaoliming
  4 siblings, 0 replies; 9+ messages in thread
From: Rebecca Cran @ 2023-05-06 19:30 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Liming Gao,
	Bob Feng, Yuwei Chen
  Cc: Rebecca Cran

Update toolsetup.bat and Tests/PythonTest.py to check if we're running a
version of Python that's compatible with BaseTools and the Pip
BaseTools.

BaseTools uses syntax from Python 3.6 or newer, so set that as the minimum
version EDK2 requires.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
 BaseTools/Tests/PythonTest.py | 22 ++++++-
 BaseTools/toolsetup.bat       | 61 +++++++++++---------
 2 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/BaseTools/Tests/PythonTest.py b/BaseTools/Tests/PythonTest.py
index ec44c7947086..b87c78570eae 100644
--- a/BaseTools/Tests/PythonTest.py
+++ b/BaseTools/Tests/PythonTest.py
@@ -1,9 +1,27 @@
 ## @file
-# Test whether PYTHON_COMMAND is available
+# Test whether PYTHON_COMMAND is available and the
+# minimum Python version is installed.
 #
 # Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 
+import sys
+
 if __name__ == '__main__':
-    print('TRUE')
+    # Check if the major and minor versions required were specified.
+    if len(sys.argv) >= 3:
+        req_major_version = int(sys.argv[1])
+        req_minor_version = int(sys.argv[2])
+    else:
+        # If the minimum version wasn't specified on the command line,
+        # default to 3.6 because BaseTools uses syntax from PEP 526
+        # (https://peps.python.org/pep-0526/)
+        req_major_version = 3
+        req_minor_version = 6
+
+    if sys.version_info.major == req_major_version and \
+       sys.version_info.minor >= req_minor_version:
+        sys.exit(0)
+    else:
+        sys.exit(1)
diff --git a/BaseTools/toolsetup.bat b/BaseTools/toolsetup.bat
index 3d13e9fad286..dc6288effd7d 100755
--- a/BaseTools/toolsetup.bat
+++ b/BaseTools/toolsetup.bat
@@ -12,6 +12,8 @@
 @echo off
 pushd .
 set SCRIPT_ERROR=0
+set PYTHON_VER_MAJOR=3
+set PYTHON_VER_MINOR=6
 
 @REM ##############################################################
 @REM # You should not have to modify anything below this line
@@ -304,17 +306,19 @@ goto check_build_environment
      )
   )
 
-:defined_python
+@REM Check Python environment
 
 if not defined PYTHON_COMMAND (
   set PYTHON_COMMAND=py -3
-  py -3 %BASE_TOOLS_PATH%\Tests\PythonTest.py >PythonCheck.txt 2>&1
-  setlocal enabledelayedexpansion
-  set /p PythonCheck=<"PythonCheck.txt"
-  del PythonCheck.txt
-  if "!PythonCheck!" NEQ "TRUE" (
+  py -3 %BASE_TOOLS_PATH%\Tests\PythonTest.py %PYTHON_VER_MAJOR% %PYTHON_VER_MINOR% >NUL 2>NUL
+  if %ERRORLEVEL% EQU 1 (
+    echo.
+    echo !!! ERROR !!! Python %PYTHON_VER_MAJOR%.%PYTHON_VER_MINOR% or newer is required.
+    echo.
+    goto end
+  )
+  if %ERRORLEVEL% NEQ 0 (
     if not defined PYTHON_HOME if not defined PYTHONHOME (
-      endlocal
       set PYTHON_COMMAND=
       echo.
       echo !!! ERROR !!! Binary python tools are missing.
@@ -327,8 +331,6 @@ if not defined PYTHON_COMMAND (
   )
 )
 
-endlocal
-
 if defined PYTHON_HOME (
   if EXIST "%PYTHON_HOME%" (
     set PYTHON_COMMAND=%PYTHON_HOME%\python.exe
@@ -340,27 +342,30 @@ if defined PYTHON_HOME (
   )
 )
 
-%PYTHON_COMMAND% %BASE_TOOLS_PATH%\Tests\PythonTest.py >PythonCheck.txt 2>&1
-  setlocal enabledelayedexpansion
-  set /p PythonCheck=<"PythonCheck.txt"
-  del PythonCheck.txt
-  if "!PythonCheck!" NEQ "TRUE" (
-    echo.
-    echo ! ERROR !  PYTHON_COMMAND="%PYTHON_COMMAND%" is not installed or added to environment variables
-    echo.
-    goto end
+%PYTHON_COMMAND% %BASE_TOOLS_PATH%\Tests\PythonTest.py %PYTHON_VER_MAJOR% %PYTHON_VER_MINOR% >NUL 2>NUL
+if %ERRORLEVEL% EQU 1 (
+  echo.
+  echo !!! ERROR !!! Python %PYTHON_VER_MAJOR%.%PYTHON_VER_MINOR% or newer is required.
+  echo.
+  goto end
+)
+if %ERRORLEVEL% NEQ 0 (
+  echo.
+  echo !!! ERROR !!!  PYTHON_COMMAND="%PYTHON_COMMAND%" does not exist or is not a Python interpreter.
+  echo.
+  goto end
 )
 
 endlocal
 
-  %PYTHON_COMMAND% -c "import edk2basetools" >NUL 2>NUL
-  if %ERRORLEVEL% EQU 0 (
-    goto use_pip_basetools
-  ) else (
-    REM reset ERRORLEVEL
-    type nul>nul
-    goto use_builtin_basetools
-  )
+%PYTHON_COMMAND% -c "import edk2basetools" >NUL 2>NUL
+if %ERRORLEVEL% EQU 0 (
+  goto use_pip_basetools
+) else (
+  REM reset ERRORLEVEL
+  type nul>nul
+  goto use_builtin_basetools
+)
 
 :use_builtin_basetools
   @echo Using EDK2 in-source Basetools
@@ -444,5 +449,7 @@ set VS2019=
 set VS2017=
 set VS2015=
 set VSTool=
+set PYTHON_VER_MAJOR=
+set PYTHON_VER_MINOR=
+set SCRIPT_ERROR=
 popd
-
-- 
2.40.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* 回复: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation
  2023-05-06 19:30 [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation Rebecca Cran
                   ` (3 preceding siblings ...)
  2023-05-06 19:30 ` [PATCH v3 4/4] BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver Rebecca Cran
@ 2023-05-08  1:30 ` gaoliming
  2023-05-08  1:32   ` [edk2-devel] " Rebecca Cran
  4 siblings, 1 reply; 9+ messages in thread
From: gaoliming @ 2023-05-08  1:30 UTC (permalink / raw)
  To: 'Rebecca Cran', devel, 'Andrew Fish',
	'Leif Lindholm', 'Michael D Kinney',
	'Bob Feng', 'Yuwei Chen'

Rebecca:
  This change is good to me. Reviewed-by: Liming Gao <gaoliming@byosoft.com.
cn>
  
  But, I see edksetup.sh also has python2 check. I think the same clean up
can be done in edksetup.sh.

Thanks
Liming
> -----邮件原件-----
> 发件人: Rebecca Cran <rebecca@bsdio.com>
> 发送时间: 2023年5月7日 3:31
> 收件人: devel@edk2.groups.io; Andrew Fish <afish@apple.com>; Leif
> Lindholm <quic_llindhol@quicinc.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Bob Feng <bob.c.feng@intel.com>; Yuwei Chen <yuwei.chen@intel.com>
> 抄送: Rebecca Cran <rebecca@bsdio.com>
> 主题: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows
> environment setup and BaseTools C compilation
> 
> There are remnants of Python 2 support in BaseTools/toolsetup.bat that
it's
> probably time to remove since we only support Python 3.6 and newer these
> days.
> So, remove the variables that enable Python3 support and simplify the
batch
> script. I've also seen errors where after running edksetup.bat the build
> command isn't available because PYTHONPATH wasn't being set, so fix that
> when the Pip BaseTools are being used.
> 
> At the same time, let's add a check that we meet the minimum version
> requirement so we don't end up failing with an obscure error.
> 
> Building BaseTools causes a warning about threading.currentThread being
> deprecated, so update code in NmakeSubdirs.py to switch to
> threading.current_thread.
> 
> There needs to be further work, because if PYTHON_COMMAND isn't
> specified then
> it defaults to "py -3", where py is C:\Windows\py.exe, which doesn't work
if
> you're using a virtualenv since it installs python.exe and pythonw.exe in
> venv\Scripts. toolsetup.bat therefore fails to detect the Pip BaseTools
and
> uses the in-source Basetools.
> 
> GitHub PR: https://github.com/tianocore/edk2/pull/4302
> GitHub branch: https://github.com/bcran/edk2/tree/py3
> 
> Changes between v1 and v2
> =========================
> 
> - Require Python 3.6 or newer: 3.6 was when PEP 526 was added, which we
> use.
> - Fix Tests/RunTests.py on Windows.
> 
> Rebecca Cran (4):
>   BaseTools: Remove Python2/Python3 detection from toolset.bat
>   BaseTools: use threading.current_thread in NmakeSubdirs.py
>   edksetup.bat: if toolsetup.bat fails, just exit
>   BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver
> 
>  BaseTools/Source/C/Makefiles/NmakeSubdirs.py |   2 +-
>  BaseTools/Tests/PythonTest.py                |  22 +++-
>  BaseTools/toolsetup.bat                      | 119
> ++++++++------------
>  edksetup.bat                                 |   1 +
>  4 files changed, 69 insertions(+), 75 deletions(-)
> 
> --
> 2.40.0.windows.1




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] 回复: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation
  2023-05-08  1:30 ` 回复: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation gaoliming
@ 2023-05-08  1:32   ` Rebecca Cran
  2023-05-08  1:43     ` 回复: " gaoliming
  0 siblings, 1 reply; 9+ messages in thread
From: Rebecca Cran @ 2023-05-08  1:32 UTC (permalink / raw)
  To: devel, gaoliming, Andrew Fish, Leif Lindholm, Kinney, Michael D,
	'Bob Feng', 'Yuwei Chen'

I’ve removed the python 2 checks from edksetup.sh in the patch “Remove bashisms from edksetup.sh and BaseTools/BuildEnv” that I sent out a few days ago.

Rebecca

On Sun, May 7, 2023, at 7:30 PM, gaoliming via groups.io wrote:
> Rebecca:
>   This change is good to me. Reviewed-by: Liming Gao <gaoliming@byosoft.com.
> cn>
>  
>   But, I see edksetup.sh also has python2 check. I think the same clean up
> can be done in edksetup.sh.
>
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: Rebecca Cran <rebecca@bsdio.com>
>> 发送时间: 2023年5月7日 3:31
>> 收件人: devel@edk2.groups.io; Andrew Fish <afish@apple.com>; Leif
>> Lindholm <quic_llindhol@quicinc.com>; Michael D Kinney
>> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
>> Bob Feng <bob.c.feng@intel.com>; Yuwei Chen <yuwei.chen@intel.com>
>> 抄送: Rebecca Cran <rebecca@bsdio.com>
>> 主题: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows
>> environment setup and BaseTools C compilation
>> 
>> There are remnants of Python 2 support in BaseTools/toolsetup.bat that
> it's
>> probably time to remove since we only support Python 3.6 and newer these
>> days.
>> So, remove the variables that enable Python3 support and simplify the
> batch
>> script. I've also seen errors where after running edksetup.bat the build
>> command isn't available because PYTHONPATH wasn't being set, so fix that
>> when the Pip BaseTools are being used.
>> 
>> At the same time, let's add a check that we meet the minimum version
>> requirement so we don't end up failing with an obscure error.
>> 
>> Building BaseTools causes a warning about threading.currentThread being
>> deprecated, so update code in NmakeSubdirs.py to switch to
>> threading.current_thread.
>> 
>> There needs to be further work, because if PYTHON_COMMAND isn't
>> specified then
>> it defaults to "py -3", where py is C:\Windows\py.exe, which doesn't work
> if
>> you're using a virtualenv since it installs python.exe and pythonw.exe in
>> venv\Scripts. toolsetup.bat therefore fails to detect the Pip BaseTools
> and
>> uses the in-source Basetools.
>> 
>> GitHub PR: https://github.com/tianocore/edk2/pull/4302
>> GitHub branch: https://github.com/bcran/edk2/tree/py3
>> 
>> Changes between v1 and v2
>> =========================
>> 
>> - Require Python 3.6 or newer: 3.6 was when PEP 526 was added, which we
>> use.
>> - Fix Tests/RunTests.py on Windows.
>> 
>> Rebecca Cran (4):
>>   BaseTools: Remove Python2/Python3 detection from toolset.bat
>>   BaseTools: use threading.current_thread in NmakeSubdirs.py
>>   edksetup.bat: if toolsetup.bat fails, just exit
>>   BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver
>> 
>>  BaseTools/Source/C/Makefiles/NmakeSubdirs.py |   2 +-
>>  BaseTools/Tests/PythonTest.py                |  22 +++-
>>  BaseTools/toolsetup.bat                      | 119
>> ++++++++------------
>>  edksetup.bat                                 |   1 +
>>  4 files changed, 69 insertions(+), 75 deletions(-)
>> 
>> --
>> 2.40.0.windows.1
>
>
>
>
>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* 回复: [edk2-devel] 回复: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation
  2023-05-08  1:32   ` [edk2-devel] " Rebecca Cran
@ 2023-05-08  1:43     ` gaoliming
  2023-05-08  1:49       ` Rebecca Cran
  0 siblings, 1 reply; 9+ messages in thread
From: gaoliming @ 2023-05-08  1:43 UTC (permalink / raw)
  To: devel, rebecca, 'Andrew Fish', 'Leif Lindholm',
	'Kinney, Michael D', 'Bob Feng',
	'Yuwei Chen'

Rebecca:
  I see this patch. It removes python2. But, it doesn't add python version check. Will you add this check in edksetup.sh?

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Rebecca Cran
> 发送时间: 2023年5月8日 9:32
> 收件人: devel@edk2.groups.io; gaoliming <gaoliming@byosoft.com.cn>;
> Andrew Fish <afish@apple.com>; Leif Lindholm <quic_llindhol@quicinc.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; 'Bob Feng'
> <bob.c.feng@intel.com>; 'Yuwei Chen' <yuwei.chen@intel.com>
> 主题: Re: [edk2-devel] 回复: [PATCH v3 0/4] edksetup.bat, BaseTools:
> Improve Windows environment setup and BaseTools C compilation
> 
> I’ve removed the python 2 checks from edksetup.sh in the patch “Remove
> bashisms from edksetup.sh and BaseTools/BuildEnv” that I sent out a few
> days ago.
> 
> Rebecca
> 
> On Sun, May 7, 2023, at 7:30 PM, gaoliming via groups.io wrote:
> > Rebecca:
> >   This change is good to me. Reviewed-by: Liming Gao
> <gaoliming@byosoft.com.
> > cn>
> >
> >   But, I see edksetup.sh also has python2 check. I think the same clean up
> > can be done in edksetup.sh.
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: Rebecca Cran <rebecca@bsdio.com>
> >> 发送时间: 2023年5月7日 3:31
> >> 收件人: devel@edk2.groups.io; Andrew Fish <afish@apple.com>; Leif
> >> Lindholm <quic_llindhol@quicinc.com>; Michael D Kinney
> >> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> >> Bob Feng <bob.c.feng@intel.com>; Yuwei Chen <yuwei.chen@intel.com>
> >> 抄送: Rebecca Cran <rebecca@bsdio.com>
> >> 主题: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows
> >> environment setup and BaseTools C compilation
> >>
> >> There are remnants of Python 2 support in BaseTools/toolsetup.bat that
> > it's
> >> probably time to remove since we only support Python 3.6 and newer
> these
> >> days.
> >> So, remove the variables that enable Python3 support and simplify the
> > batch
> >> script. I've also seen errors where after running edksetup.bat the build
> >> command isn't available because PYTHONPATH wasn't being set, so fix that
> >> when the Pip BaseTools are being used.
> >>
> >> At the same time, let's add a check that we meet the minimum version
> >> requirement so we don't end up failing with an obscure error.
> >>
> >> Building BaseTools causes a warning about threading.currentThread being
> >> deprecated, so update code in NmakeSubdirs.py to switch to
> >> threading.current_thread.
> >>
> >> There needs to be further work, because if PYTHON_COMMAND isn't
> >> specified then
> >> it defaults to "py -3", where py is C:\Windows\py.exe, which doesn't work
> > if
> >> you're using a virtualenv since it installs python.exe and pythonw.exe in
> >> venv\Scripts. toolsetup.bat therefore fails to detect the Pip BaseTools
> > and
> >> uses the in-source Basetools.
> >>
> >> GitHub PR: https://github.com/tianocore/edk2/pull/4302
> >> GitHub branch: https://github.com/bcran/edk2/tree/py3
> >>
> >> Changes between v1 and v2
> >> =========================
> >>
> >> - Require Python 3.6 or newer: 3.6 was when PEP 526 was added, which
> we
> >> use.
> >> - Fix Tests/RunTests.py on Windows.
> >>
> >> Rebecca Cran (4):
> >>   BaseTools: Remove Python2/Python3 detection from toolset.bat
> >>   BaseTools: use threading.current_thread in NmakeSubdirs.py
> >>   edksetup.bat: if toolsetup.bat fails, just exit
> >>   BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver
> >>
> >>  BaseTools/Source/C/Makefiles/NmakeSubdirs.py |   2 +-
> >>  BaseTools/Tests/PythonTest.py                |  22 +++-
> >>  BaseTools/toolsetup.bat                      | 119
> >> ++++++++------------
> >>  edksetup.bat                                 |   1 +
> >>  4 files changed, 69 insertions(+), 75 deletions(-)
> >>
> >> --
> >> 2.40.0.windows.1
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] 回复: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation
  2023-05-08  1:43     ` 回复: " gaoliming
@ 2023-05-08  1:49       ` Rebecca Cran
  0 siblings, 0 replies; 9+ messages in thread
From: Rebecca Cran @ 2023-05-08  1:49 UTC (permalink / raw)
  To: devel, gaoliming, Andrew Fish, Leif Lindholm, Kinney, Michael D,
	'Bob Feng', 'Yuwei Chen'

Oh that’s a good point. I also noticed that on Windows we don’t run the python test in BaseTools/Tests/RunTests.py.

I’ll get the current patches merged and work on adding the python version check on Linux and the python functionally tests on Windows.

On Sun, May 7, 2023, at 7:43 PM, gaoliming via groups.io wrote:
> Rebecca:
>   I see this patch. It removes python2. But, it doesn't add python 
> version check. Will you add this check in edksetup.sh?
>
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Rebecca Cran
>> 发送时间: 2023年5月8日 9:32
>> 收件人: devel@edk2.groups.io; gaoliming <gaoliming@byosoft.com.cn>;
>> Andrew Fish <afish@apple.com>; Leif Lindholm <quic_llindhol@quicinc.com>;
>> Kinney, Michael D <michael.d.kinney@intel.com>; 'Bob Feng'
>> <bob.c.feng@intel.com>; 'Yuwei Chen' <yuwei.chen@intel.com>
>> 主题: Re: [edk2-devel] 回复: [PATCH v3 0/4] edksetup.bat, BaseTools:
>> Improve Windows environment setup and BaseTools C compilation
>> 
>> I’ve removed the python 2 checks from edksetup.sh in the patch “Remove
>> bashisms from edksetup.sh and BaseTools/BuildEnv” that I sent out a few
>> days ago.
>> 
>> Rebecca
>> 
>> On Sun, May 7, 2023, at 7:30 PM, gaoliming via groups.io wrote:
>> > Rebecca:
>> >   This change is good to me. Reviewed-by: Liming Gao
>> <gaoliming@byosoft.com.
>> > cn>
>> >
>> >   But, I see edksetup.sh also has python2 check. I think the same clean up
>> > can be done in edksetup.sh.
>> >
>> > Thanks
>> > Liming
>> >> -----邮件原件-----
>> >> 发件人: Rebecca Cran <rebecca@bsdio.com>
>> >> 发送时间: 2023年5月7日 3:31
>> >> 收件人: devel@edk2.groups.io; Andrew Fish <afish@apple.com>; Leif
>> >> Lindholm <quic_llindhol@quicinc.com>; Michael D Kinney
>> >> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
>> >> Bob Feng <bob.c.feng@intel.com>; Yuwei Chen <yuwei.chen@intel.com>
>> >> 抄送: Rebecca Cran <rebecca@bsdio.com>
>> >> 主题: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows
>> >> environment setup and BaseTools C compilation
>> >>
>> >> There are remnants of Python 2 support in BaseTools/toolsetup.bat that
>> > it's
>> >> probably time to remove since we only support Python 3.6 and newer
>> these
>> >> days.
>> >> So, remove the variables that enable Python3 support and simplify the
>> > batch
>> >> script. I've also seen errors where after running edksetup.bat the build
>> >> command isn't available because PYTHONPATH wasn't being set, so fix that
>> >> when the Pip BaseTools are being used.
>> >>
>> >> At the same time, let's add a check that we meet the minimum version
>> >> requirement so we don't end up failing with an obscure error.
>> >>
>> >> Building BaseTools causes a warning about threading.currentThread being
>> >> deprecated, so update code in NmakeSubdirs.py to switch to
>> >> threading.current_thread.
>> >>
>> >> There needs to be further work, because if PYTHON_COMMAND isn't
>> >> specified then
>> >> it defaults to "py -3", where py is C:\Windows\py.exe, which doesn't work
>> > if
>> >> you're using a virtualenv since it installs python.exe and pythonw.exe in
>> >> venv\Scripts. toolsetup.bat therefore fails to detect the Pip BaseTools
>> > and
>> >> uses the in-source Basetools.
>> >>
>> >> GitHub PR: https://github.com/tianocore/edk2/pull/4302
>> >> GitHub branch: https://github.com/bcran/edk2/tree/py3
>> >>
>> >> Changes between v1 and v2
>> >> =========================
>> >>
>> >> - Require Python 3.6 or newer: 3.6 was when PEP 526 was added, which
>> we
>> >> use.
>> >> - Fix Tests/RunTests.py on Windows.
>> >>
>> >> Rebecca Cran (4):
>> >>   BaseTools: Remove Python2/Python3 detection from toolset.bat
>> >>   BaseTools: use threading.current_thread in NmakeSubdirs.py
>> >>   edksetup.bat: if toolsetup.bat fails, just exit
>> >>   BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver
>> >>
>> >>  BaseTools/Source/C/Makefiles/NmakeSubdirs.py |   2 +-
>> >>  BaseTools/Tests/PythonTest.py                |  22 +++-
>> >>  BaseTools/toolsetup.bat                      | 119
>> >> ++++++++------------
>> >>  edksetup.bat                                 |   1 +
>> >>  4 files changed, 69 insertions(+), 75 deletions(-)
>> >>
>> >> --
>> >> 2.40.0.windows.1
>> >
>> >
>> >
>> >
>> >
>> >
>> 
>> 
>> 
>> 
>
>
>
>
>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-05-08  1:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-06 19:30 [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation Rebecca Cran
2023-05-06 19:30 ` [PATCH v3 1/4] BaseTools: Remove Python2/Python3 detection from toolset.bat Rebecca Cran
2023-05-06 19:30 ` [PATCH v3 2/4] BaseTools: use threading.current_thread in NmakeSubdirs.py Rebecca Cran
2023-05-06 19:30 ` [PATCH v3 3/4] edksetup.bat: if toolsetup.bat fails, just exit Rebecca Cran
2023-05-06 19:30 ` [PATCH v3 4/4] BaseTools: Update toolsetup.bat and Tests/PythonTest.py to check ver Rebecca Cran
2023-05-08  1:30 ` 回复: [PATCH v3 0/4] edksetup.bat, BaseTools: Improve Windows environment setup and BaseTools C compilation gaoliming
2023-05-08  1:32   ` [edk2-devel] " Rebecca Cran
2023-05-08  1:43     ` 回复: " gaoliming
2023-05-08  1:49       ` Rebecca Cran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox