public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch] BaseTools: Add python3-distutils Ubuntu package checking
@ 2019-02-26  1:05 Feng, Bob C
  2019-02-27  8:07 ` Ard Biesheuvel
  0 siblings, 1 reply; 6+ messages in thread
From: Feng, Bob C @ 2019-02-26  1:05 UTC (permalink / raw)
  To: edk2-devel; +Cc: Bob Feng, Liming Gao

https://bugzilla.tianocore.org/show_bug.cgi?id=1509

Add python3-distutils Ubuntu package checking.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
---
 BaseTools/Tests/RunTests.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py
index 0dd65632d0..64778db981 100644
--- a/BaseTools/Tests/RunTests.py
+++ b/BaseTools/Tests/RunTests.py
@@ -17,10 +17,24 @@
 #
 import os
 import sys
 import unittest
 
+distutils_exist = True
+try:
+    import distutils.util
+except:
+    distutils_exist = False
+
+if not distutils_exist:
+    print("""
+python3-distutil packages is missing. Please install it with the following command:
+
+bash$ sudo apt-get install python3-distutil
+""")
+    sys.exit(-1)
+
 import TestTools
 
 def GetCTestSuite():
     import CToolsTests
     return CToolsTests.TheTestSuite()
-- 
2.20.1.windows.1



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

* Re: [Patch] BaseTools: Add python3-distutils Ubuntu package checking
  2019-02-26  1:05 [Patch] BaseTools: Add python3-distutils Ubuntu package checking Feng, Bob C
@ 2019-02-27  8:07 ` Ard Biesheuvel
  2019-02-27  8:26   ` Leif Lindholm
  0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2019-02-27  8:07 UTC (permalink / raw)
  To: Feng, Bob C; +Cc: edk2-devel@lists.01.org, Liming Gao

On Tue, 26 Feb 2019 at 02:05, Feng, Bob C <bob.c.feng@intel.com> wrote:
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1509
>
> Add python3-distutils Ubuntu package checking.
>

Hi Bob,

This assumes that all Linux systems are Ubuntu based, which is not
true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and
Suse all use something else.

In general, I don't think we should validate the Python environment to
this extent, since we cannot fix the problem for the user anyway, only
flag it, and since python explodes rather loudly in this case, I think
we should be able to leave it up to developers that are savvy enough
to build EDK2 to also find the python distutils package for their
platform.

Note that that doesn't mean we shouldn't document this, and not just
for Ubuntu. But I think putting it in the script is overkill.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> ---
>  BaseTools/Tests/RunTests.py | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py
> index 0dd65632d0..64778db981 100644
> --- a/BaseTools/Tests/RunTests.py
> +++ b/BaseTools/Tests/RunTests.py
> @@ -17,10 +17,24 @@
>  #
>  import os
>  import sys
>  import unittest
>
> +distutils_exist = True
> +try:
> +    import distutils.util
> +except:
> +    distutils_exist = False
> +
> +if not distutils_exist:
> +    print("""
> +python3-distutil packages is missing. Please install it with the following command:
> +
> +bash$ sudo apt-get install python3-distutil
> +""")
> +    sys.exit(-1)
> +
>  import TestTools
>
>  def GetCTestSuite():
>      import CToolsTests
>      return CToolsTests.TheTestSuite()
> --
> 2.20.1.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Patch] BaseTools: Add python3-distutils Ubuntu package checking
  2019-02-27  8:07 ` Ard Biesheuvel
@ 2019-02-27  8:26   ` Leif Lindholm
  2019-02-27  8:52     ` Feng, Bob C
  0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2019-02-27  8:26 UTC (permalink / raw)
  To: Feng, Bob C; +Cc: Ard Biesheuvel, edk2-devel@lists.01.org, Liming Gao

On Wed, Feb 27, 2019 at 09:07:49AM +0100, Ard Biesheuvel wrote:
> On Tue, 26 Feb 2019 at 02:05, Feng, Bob C <bob.c.feng@intel.com> wrote:
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1509
> >
> > Add python3-distutils Ubuntu package checking.
> >
> 
> Hi Bob,
> 
> This assumes that all Linux systems are Ubuntu based, which is not
> true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and
> Suse all use something else.
> 
> In general, I don't think we should validate the Python environment to
> this extent, since we cannot fix the problem for the user anyway, only
> flag it, and since python explodes rather loudly in this case, I think
> we should be able to leave it up to developers that are savvy enough
> to build EDK2 to also find the python distutils package for their
> platform.
> 
> Note that that doesn't mean we shouldn't document this, and not just
> for Ubuntu. But I think putting it in the script is overkill.

Yes, I agree

It is also worth noting that python3-distutils is the current
debian/ubuntu package name. So if we *do* print a message...

> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Bob Feng <bob.c.feng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > ---
> >  BaseTools/Tests/RunTests.py | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/BaseTools/Tests/RunTests.py b/BaseTools/Tests/RunTests.py
> > index 0dd65632d0..64778db981 100644
> > --- a/BaseTools/Tests/RunTests.py
> > +++ b/BaseTools/Tests/RunTests.py
> > @@ -17,10 +17,24 @@
> >  #
> >  import os
> >  import sys
> >  import unittest
> >
> > +distutils_exist = True
> > +try:
> > +    import distutils.util
> > +except:
> > +    distutils_exist = False
> > +
> > +if not distutils_exist:
> > +    print("""
> > +python3-distutil packages is missing. Please install it with the following command:

... printing "missing python distutils package" and possibly python
version would be more reliable.

But as Ard points out - this is effectively what python itself will
say.

/
    Leif

> > +
> > +bash$ sudo apt-get install python3-distutil
> > +""")
> > +    sys.exit(-1)
> > +
> >  import TestTools
> >
> >  def GetCTestSuite():
> >      import CToolsTests
> >      return CToolsTests.TheTestSuite()
> > --
> > 2.20.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Patch] BaseTools: Add python3-distutils Ubuntu package checking
  2019-02-27  8:26   ` Leif Lindholm
@ 2019-02-27  8:52     ` Feng, Bob C
  2019-02-27  8:59       ` Leif Lindholm
  2019-02-27 12:38       ` Laszlo Ersek
  0 siblings, 2 replies; 6+ messages in thread
From: Feng, Bob C @ 2019-02-27  8:52 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: Ard Biesheuvel, edk2-devel@lists.01.org, Gao, Liming

Thanks for comments. I think the print message is not good. It's based on Ubutun OS. It's not right. 

I think the import error need to be caught and then print some messages, otherwise the build tool will break and print the call stack which is not friendly to user.

Thanks,
Bob 

-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@linaro.org] 
Sent: Wednesday, February 27, 2019 4:26 PM
To: Feng, Bob C <bob.c.feng@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking

On Wed, Feb 27, 2019 at 09:07:49AM +0100, Ard Biesheuvel wrote:
> On Tue, 26 Feb 2019 at 02:05, Feng, Bob C <bob.c.feng@intel.com> wrote:
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1509
> >
> > Add python3-distutils Ubuntu package checking.
> >
> 
> Hi Bob,
> 
> This assumes that all Linux systems are Ubuntu based, which is not 
> true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and 
> Suse all use something else.
> 
> In general, I don't think we should validate the Python environment to 
> this extent, since we cannot fix the problem for the user anyway, only 
> flag it, and since python explodes rather loudly in this case, I think 
> we should be able to leave it up to developers that are savvy enough 
> to build EDK2 to also find the python distutils package for their 
> platform.
> 
> Note that that doesn't mean we shouldn't document this, and not just 
> for Ubuntu. But I think putting it in the script is overkill.

Yes, I agree

It is also worth noting that python3-distutils is the current debian/ubuntu package name. So if we *do* print a message...

> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Bob Feng <bob.c.feng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > ---
> >  BaseTools/Tests/RunTests.py | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/BaseTools/Tests/RunTests.py 
> > b/BaseTools/Tests/RunTests.py index 0dd65632d0..64778db981 100644
> > --- a/BaseTools/Tests/RunTests.py
> > +++ b/BaseTools/Tests/RunTests.py
> > @@ -17,10 +17,24 @@
> >  #
> >  import os
> >  import sys
> >  import unittest
> >
> > +distutils_exist = True
> > +try:
> > +    import distutils.util
> > +except:
> > +    distutils_exist = False
> > +
> > +if not distutils_exist:
> > +    print("""
> > +python3-distutil packages is missing. Please install it with the following command:

... printing "missing python distutils package" and possibly python version would be more reliable.

But as Ard points out - this is effectively what python itself will say.

/
    Leif

> > +
> > +bash$ sudo apt-get install python3-distutil
> > +""")
> > +    sys.exit(-1)
> > +
> >  import TestTools
> >
> >  def GetCTestSuite():
> >      import CToolsTests
> >      return CToolsTests.TheTestSuite()
> > --
> > 2.20.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Patch] BaseTools: Add python3-distutils Ubuntu package checking
  2019-02-27  8:52     ` Feng, Bob C
@ 2019-02-27  8:59       ` Leif Lindholm
  2019-02-27 12:38       ` Laszlo Ersek
  1 sibling, 0 replies; 6+ messages in thread
From: Leif Lindholm @ 2019-02-27  8:59 UTC (permalink / raw)
  To: Feng, Bob C; +Cc: Ard Biesheuvel, edk2-devel@lists.01.org, Gao, Liming

On Wed, Feb 27, 2019 at 08:52:08AM +0000, Feng, Bob C wrote:
> Thanks for comments. I think the print message is not good. It's based on Ubutun OS. It's not right. 
> 
> I think the import error need to be caught and then print some
> messages, otherwise the build tool will break and print the call
> stack which is not friendly to user.

I agree printing the call stack is not useful for import errors.
Is there no way to suppress that for basic environment issues?

Surely a "failed to import..." message would also be printed?

Regards,

Leif

> Thanks,
> Bob 
> 
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org] 
> Sent: Wednesday, February 27, 2019 4:26 PM
> To: Feng, Bob C <bob.c.feng@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking
> 
> On Wed, Feb 27, 2019 at 09:07:49AM +0100, Ard Biesheuvel wrote:
> > On Tue, 26 Feb 2019 at 02:05, Feng, Bob C <bob.c.feng@intel.com> wrote:
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=1509
> > >
> > > Add python3-distutils Ubuntu package checking.
> > >
> > 
> > Hi Bob,
> > 
> > This assumes that all Linux systems are Ubuntu based, which is not 
> > true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and 
> > Suse all use something else.
> > 
> > In general, I don't think we should validate the Python environment to 
> > this extent, since we cannot fix the problem for the user anyway, only 
> > flag it, and since python explodes rather loudly in this case, I think 
> > we should be able to leave it up to developers that are savvy enough 
> > to build EDK2 to also find the python distutils package for their 
> > platform.
> > 
> > Note that that doesn't mean we shouldn't document this, and not just 
> > for Ubuntu. But I think putting it in the script is overkill.
> 
> Yes, I agree
> 
> It is also worth noting that python3-distutils is the current debian/ubuntu package name. So if we *do* print a message...
> 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Bob Feng <bob.c.feng@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > ---
> > >  BaseTools/Tests/RunTests.py | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/BaseTools/Tests/RunTests.py 
> > > b/BaseTools/Tests/RunTests.py index 0dd65632d0..64778db981 100644
> > > --- a/BaseTools/Tests/RunTests.py
> > > +++ b/BaseTools/Tests/RunTests.py
> > > @@ -17,10 +17,24 @@
> > >  #
> > >  import os
> > >  import sys
> > >  import unittest
> > >
> > > +distutils_exist = True
> > > +try:
> > > +    import distutils.util
> > > +except:
> > > +    distutils_exist = False
> > > +
> > > +if not distutils_exist:
> > > +    print("""
> > > +python3-distutil packages is missing. Please install it with the following command:
> 
> ... printing "missing python distutils package" and possibly python version would be more reliable.
> 
> But as Ard points out - this is effectively what python itself will say.
> 
> /
>     Leif
> 
> > > +
> > > +bash$ sudo apt-get install python3-distutil
> > > +""")
> > > +    sys.exit(-1)
> > > +
> > >  import TestTools
> > >
> > >  def GetCTestSuite():
> > >      import CToolsTests
> > >      return CToolsTests.TheTestSuite()
> > > --
> > > 2.20.1.windows.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Patch] BaseTools: Add python3-distutils Ubuntu package checking
  2019-02-27  8:52     ` Feng, Bob C
  2019-02-27  8:59       ` Leif Lindholm
@ 2019-02-27 12:38       ` Laszlo Ersek
  1 sibling, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2019-02-27 12:38 UTC (permalink / raw)
  To: Feng, Bob C, Leif Lindholm; +Cc: edk2-devel@lists.01.org, Gao, Liming

On 02/27/19 09:52, Feng, Bob C wrote:
> Thanks for comments. I think the print message is not good. It's based on Ubutun OS. It's not right. 
> 
> I think the import error need to be caught and then print some messages, otherwise the build tool will break and print the call stack which is not friendly to user.

I agree. OS or package name specific references should not be printed,
but a user-friendly, concise message about the missing Python feature
should be.

As soon as I see a Python (or Java, for that matter) stack dump dropped
on me, my first instinct is to close the terminal window. Obviously I
won't do that most of the time, but it takes effort to start deciphering
the stack dump. Give me a usable one-liner error message, and
*optionally* a stack dump to dig down.

Given that the situation can be remedied by a google search ("what
package on my system provides this Python feature") plus a package
installation, the stack dump is entirely irrelevant in this case.

Thanks
Laszlo


> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org] 
> Sent: Wednesday, February 27, 2019 4:26 PM
> To: Feng, Bob C <bob.c.feng@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [Patch] BaseTools: Add python3-distutils Ubuntu package checking
> 
> On Wed, Feb 27, 2019 at 09:07:49AM +0100, Ard Biesheuvel wrote:
>> On Tue, 26 Feb 2019 at 02:05, Feng, Bob C <bob.c.feng@intel.com> wrote:
>>>
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=1509
>>>
>>> Add python3-distutils Ubuntu package checking.
>>>
>>
>> Hi Bob,
>>
>> This assumes that all Linux systems are Ubuntu based, which is not 
>> true. The apt tool is specific to Debian/Ubuntu, Fedora/Redhat and 
>> Suse all use something else.
>>
>> In general, I don't think we should validate the Python environment to 
>> this extent, since we cannot fix the problem for the user anyway, only 
>> flag it, and since python explodes rather loudly in this case, I think 
>> we should be able to leave it up to developers that are savvy enough 
>> to build EDK2 to also find the python distutils package for their 
>> platform.
>>
>> Note that that doesn't mean we shouldn't document this, and not just 
>> for Ubuntu. But I think putting it in the script is overkill.
> 
> Yes, I agree
> 
> It is also worth noting that python3-distutils is the current debian/ubuntu package name. So if we *do* print a message...
> 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Bob Feng <bob.c.feng@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> ---
>>>  BaseTools/Tests/RunTests.py | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/BaseTools/Tests/RunTests.py 
>>> b/BaseTools/Tests/RunTests.py index 0dd65632d0..64778db981 100644
>>> --- a/BaseTools/Tests/RunTests.py
>>> +++ b/BaseTools/Tests/RunTests.py
>>> @@ -17,10 +17,24 @@
>>>  #
>>>  import os
>>>  import sys
>>>  import unittest
>>>
>>> +distutils_exist = True
>>> +try:
>>> +    import distutils.util
>>> +except:
>>> +    distutils_exist = False
>>> +
>>> +if not distutils_exist:
>>> +    print("""
>>> +python3-distutil packages is missing. Please install it with the following command:
> 
> ... printing "missing python distutils package" and possibly python version would be more reliable.
> 
> But as Ard points out - this is effectively what python itself will say.
> 
> /
>     Leif
> 
>>> +
>>> +bash$ sudo apt-get install python3-distutil
>>> +""")
>>> +    sys.exit(-1)
>>> +
>>>  import TestTools
>>>
>>>  def GetCTestSuite():
>>>      import CToolsTests
>>>      return CToolsTests.TheTestSuite()
>>> --
>>> 2.20.1.windows.1
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 



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

end of thread, other threads:[~2019-02-27 12:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-26  1:05 [Patch] BaseTools: Add python3-distutils Ubuntu package checking Feng, Bob C
2019-02-27  8:07 ` Ard Biesheuvel
2019-02-27  8:26   ` Leif Lindholm
2019-02-27  8:52     ` Feng, Bob C
2019-02-27  8:59       ` Leif Lindholm
2019-02-27 12:38       ` Laszlo Ersek

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