Re: [Patch] BaseTools: Add python3-distutils Ubuntu package checking

[Laszlo Ersek <lersek@redhat.com>]

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
>