public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Feng, Bob C" <bob.c.feng@intel.com>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Gao, Liming" <liming.gao@intel.com>
Subject: Re: [Patch] BaseTools: Add python3-distutils Ubuntu package checking
Date: Wed, 27 Feb 2019 08:52:08 +0000	[thread overview]
Message-ID: <08650203BA1BD64D8AD9B6D5D74A85D1600901EC@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20190227082600.7g6m6a6uwmp4sfuj@bivouac.eciton.net>

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


  reply	other threads:[~2019-02-27  8:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2019-02-27  8:59       ` Leif Lindholm
2019-02-27 12:38       ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=08650203BA1BD64D8AD9B6D5D74A85D1600901EC@SHSMSX101.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox