* [PATCH] BaseTools:The code used to test python module is moved to edksetup
@ 2019-04-30 2:16 Fan, ZhijuX
2019-05-07 3:53 ` Bob Feng
0 siblings, 1 reply; 4+ messages in thread
From: Fan, ZhijuX @ 2019-04-30 2:16 UTC (permalink / raw)
To: devel@edk2.groups.io; +Cc: Gao, Liming, Feng, Bob C
[-- Attachment #1: Type: text/plain, Size: 1556 bytes --]
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1582
testing for presence of python modules should be done in edksetup
to reduce impact on subsequent build times.
This code currently exists in BaseTools/Tests/RunTest.py.
This patch is going to fix this issue.
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
---
BaseTools/Tests/RunTests.py | 8 --------
edksetup.sh | 15 +++++++++++++--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py
index 81af736cd8..e8acf1b348 100644
--- a/BaseTools/Tests/RunTests.py
+++ b/BaseTools/Tests/RunTests.py
@@ -12,14 +12,6 @@
import os
import sys
import unittest
-
-try:
- import distutils.util
-except ModuleNotFoundError:
- sys.exit('''
-Python reported: "No module named 'distutils.util"
-''')
-
import TestTools
def GetCTestSuite():
diff --git a/edksetup.sh b/edksetup.sh
index c7b2e1e201..add18ca7c0 100755
--- a/edksetup.sh
+++ b/edksetup.sh
@@ -177,11 +177,22 @@ function SetupPython()
SetupPython3
}
+function TestUtilModule()
+{
+ if ( $PYTHON_COMMAND -c "import distutils.util" >/dev/null 2>&1 );then
+ return 1
+ else
+ echo Error: "No module named 'distutils.util"
+ return 0
+ fi
+}
+
function SourceEnv()
{
SetWorkspace &&
- SetupEnv
- SetupPython
+ SetupEnv &&
+ SetupPython &&
+ TestUtilModule
}
I=$#
--
2.14.1.windows.1
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 4111 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] BaseTools:The code used to test python module is moved to edksetup
2019-04-30 2:16 [PATCH] BaseTools:The code used to test python module is moved to edksetup Fan, ZhijuX
@ 2019-05-07 3:53 ` Bob Feng
2019-05-08 11:38 ` [edk2-devel] " Leif Lindholm
0 siblings, 1 reply; 4+ messages in thread
From: Bob Feng @ 2019-05-07 3:53 UTC (permalink / raw)
To: Fan, ZhijuX, devel@edk2.groups.io; +Cc: Gao, Liming
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
-----Original Message-----
From: Fan, ZhijuX
Sent: Tuesday, April 30, 2019 10:16 AM
To: devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
Subject: [PATCH] BaseTools:The code used to test python module is moved to edksetup
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1582
testing for presence of python modules should be done in edksetup to reduce impact on subsequent build times.
This code currently exists in BaseTools/Tests/RunTest.py.
This patch is going to fix this issue.
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
---
BaseTools/Tests/RunTests.py | 8 --------
edksetup.sh | 15 +++++++++++++--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py index 81af736cd8..e8acf1b348 100644
--- a/BaseTools/Tests/RunTests.py
+++ b/BaseTools/Tests/RunTests.py
@@ -12,14 +12,6 @@
import os
import sys
import unittest
-
-try:
- import distutils.util
-except ModuleNotFoundError:
- sys.exit('''
-Python reported: "No module named 'distutils.util"
-''')
-
import TestTools
def GetCTestSuite():
diff --git a/edksetup.sh b/edksetup.sh
index c7b2e1e201..add18ca7c0 100755
--- a/edksetup.sh
+++ b/edksetup.sh
@@ -177,11 +177,22 @@ function SetupPython()
SetupPython3
}
+function TestUtilModule()
+{
+ if ( $PYTHON_COMMAND -c "import distutils.util" >/dev/null 2>&1 );then
+ return 1
+ else
+ echo Error: "No module named 'distutils.util"
+ return 0
+ fi
+}
+
function SourceEnv()
{
SetWorkspace &&
- SetupEnv
- SetupPython
+ SetupEnv &&
+ SetupPython &&
+ TestUtilModule
}
I=$#
--
2.14.1.windows.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] BaseTools:The code used to test python module is moved to edksetup
2019-05-07 3:53 ` Bob Feng
@ 2019-05-08 11:38 ` Leif Lindholm
2019-05-08 11:45 ` Leif Lindholm
0 siblings, 1 reply; 4+ messages in thread
From: Leif Lindholm @ 2019-05-08 11:38 UTC (permalink / raw)
To: devel, Fan, ZhijuX; +Cc: Gao, Liming, bob.c.feng
Hi guys,
This patch (now committed) break our ci (which runs with 'set -e').
This seems to be caused by TestUtilModule() returning error (1 -
non-zero) when it actually finds the module, and success (0) when it
does not.
While debugging, I found another side effect that I have not had time
to track down, and does not go away with resolving this incorrect
behaviour. When I run from the command line:
$ set -e
$ . edks<tab>
to tab-complete the filename in bash, this terminates the current
shell.
Unless someone can find a solution to the latter quickly, can we
revert this patch please?
One further comment below.
On Tue, May 07, 2019 at 03:53:18AM +0000, Bob Feng wrote:
> Reviewed-by: Bob Feng <bob.c.feng@intel.com>
>
> -----Original Message-----
> From: Fan, ZhijuX
> Sent: Tuesday, April 30, 2019 10:16 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
> Subject: [PATCH] BaseTools:The code used to test python module is moved to edksetup
>
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1582
>
> testing for presence of python modules should be done in edksetup to reduce impact on subsequent build times.
> This code currently exists in BaseTools/Tests/RunTest.py.
>
> This patch is going to fix this issue.
>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
> ---
> BaseTools/Tests/RunTests.py | 8 --------
> edksetup.sh | 15 +++++++++++++--
> 2 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py index 81af736cd8..e8acf1b348 100644
> --- a/BaseTools/Tests/RunTests.py
> +++ b/BaseTools/Tests/RunTests.py
> @@ -12,14 +12,6 @@
> import os
> import sys
> import unittest
> -
> -try:
> - import distutils.util
> -except ModuleNotFoundError:
> - sys.exit('''
> -Python reported: "No module named 'distutils.util"
> -''')
> -
> import TestTools
>
> def GetCTestSuite():
> diff --git a/edksetup.sh b/edksetup.sh
> index c7b2e1e201..add18ca7c0 100755
> --- a/edksetup.sh
> +++ b/edksetup.sh
> @@ -177,11 +177,22 @@ function SetupPython()
> SetupPython3
> }
>
> +function TestUtilModule()
> +{
> + if ( $PYTHON_COMMAND -c "import distutils.util" >/dev/null 2>&1 );then
> + return 1
> + else
> + echo Error: "No module named 'distutils.util"
> + return 0
> + fi
> +}
> +
> function SourceEnv()
> {
> SetWorkspace &&
> - SetupEnv
> - SetupPython
> + SetupEnv &&
Not adding this && in 9c2d68c0a299 ("BaseTools: Update windows and
linux run scripts file to use Python3") when SetupPython was added was
clearly an oversight, but that is not something to quietly fix up in
this completely unrelated patch.
When resending a new version, after revert, please do that as a
separate patch.
Best Regards,
Leif
> + SetupPython &&
> + TestUtilModule
> }
>
> I=$#
> --
> 2.14.1.windows.1
>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] BaseTools:The code used to test python module is moved to edksetup
2019-05-08 11:38 ` [edk2-devel] " Leif Lindholm
@ 2019-05-08 11:45 ` Leif Lindholm
0 siblings, 0 replies; 4+ messages in thread
From: Leif Lindholm @ 2019-05-08 11:45 UTC (permalink / raw)
To: devel, Fan, ZhijuX; +Cc: Gao, Liming, bob.c.feng
On Wed, May 08, 2019 at 12:38:24PM +0100, Leif Lindholm wrote:
> Hi guys,
>
> This patch (now committed) break our ci (which runs with 'set -e').
> This seems to be caused by TestUtilModule() returning error (1 -
> non-zero) when it actually finds the module, and success (0) when it
> does not.
>
> While debugging, I found another side effect that I have not had time
> to track down, and does not go away with resolving this incorrect
> behaviour. When I run from the command line:
> $ set -e
> $ . edks<tab>
> to tab-complete the filename in bash, this terminates the current
> shell.
>
> Unless someone can find a solution to the latter quickly, can we
> revert this patch please?
Err... Never mind, my brain engaged shortly after sending. As usual.
There are many reasons why the various scripts invoked by bash for its
command line completion may sometimes return non-zero, and the issue
is not specific to this script.
Still, due to the && issue pointed out below, I would still like to
see a revert, a separate && fix to the pre-existing version, and a new
version of this patch with corrected logic.
Best Regards,
Leif
> One further comment below.
>
> On Tue, May 07, 2019 at 03:53:18AM +0000, Bob Feng wrote:
> > Reviewed-by: Bob Feng <bob.c.feng@intel.com>
> >
> > -----Original Message-----
> > From: Fan, ZhijuX
> > Sent: Tuesday, April 30, 2019 10:16 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
> > Subject: [PATCH] BaseTools:The code used to test python module is moved to edksetup
> >
> > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1582
> >
> > testing for presence of python modules should be done in edksetup to reduce impact on subsequent build times.
> > This code currently exists in BaseTools/Tests/RunTest.py.
> >
> > This patch is going to fix this issue.
> >
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
> > ---
> > BaseTools/Tests/RunTests.py | 8 --------
> > edksetup.sh | 15 +++++++++++++--
> > 2 files changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py index 81af736cd8..e8acf1b348 100644
> > --- a/BaseTools/Tests/RunTests.py
> > +++ b/BaseTools/Tests/RunTests.py
> > @@ -12,14 +12,6 @@
> > import os
> > import sys
> > import unittest
> > -
> > -try:
> > - import distutils.util
> > -except ModuleNotFoundError:
> > - sys.exit('''
> > -Python reported: "No module named 'distutils.util"
> > -''')
> > -
> > import TestTools
> >
> > def GetCTestSuite():
> > diff --git a/edksetup.sh b/edksetup.sh
> > index c7b2e1e201..add18ca7c0 100755
> > --- a/edksetup.sh
> > +++ b/edksetup.sh
> > @@ -177,11 +177,22 @@ function SetupPython()
> > SetupPython3
> > }
> >
> > +function TestUtilModule()
> > +{
> > + if ( $PYTHON_COMMAND -c "import distutils.util" >/dev/null 2>&1 );then
> > + return 1
> > + else
> > + echo Error: "No module named 'distutils.util"
> > + return 0
> > + fi
> > +}
> > +
> > function SourceEnv()
> > {
> > SetWorkspace &&
> > - SetupEnv
> > - SetupPython
> > + SetupEnv &&
>
> Not adding this && in 9c2d68c0a299 ("BaseTools: Update windows and
> linux run scripts file to use Python3") when SetupPython was added was
> clearly an oversight, but that is not something to quietly fix up in
> this completely unrelated patch.
>
> When resending a new version, after revert, please do that as a
> separate patch.
>
> Best Regards,
>
> Leif
>
> > + SetupPython &&
> > + TestUtilModule
> > }
> >
> > I=$#
> > --
> > 2.14.1.windows.1
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-08 11:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-30 2:16 [PATCH] BaseTools:The code used to test python module is moved to edksetup Fan, ZhijuX
2019-05-07 3:53 ` Bob Feng
2019-05-08 11:38 ` [edk2-devel] " Leif Lindholm
2019-05-08 11:45 ` Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox