* [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