On Wed, Oct 16, 2019 at 03:26:15PM +0200, Michael Olbrich wrote:
> On Wed, Oct 16, 2019 at 01:58:07PM +0200, Roland Hieber wrote:
> > On Wed, Oct 16, 2019 at 01:38:56PM +0200, Michael Olbrich wrote:
> > > On Wed, Oct 16, 2019 at 12:53:10PM +0200, Roland Hieber wrote:
> > > > Michael, any comments? It does not seem to be applied yet.
> > > 
> > > Right, this got lost. I'm not sure I like this. Why would there be an 
> > > empty
> > > PTXDIST env variable?
> > 
> > I don't know, but somehow it happened to me, and the script can be more 
> > robust in that case. Also when that happens it is not very
> > easy to find out what is the fault just from the Python stacktrace: 
> > 
> >   Traceback (most recent call last):
> >     File "./scripts/configure_helper.py", line 497, in <module>
> >       (tool, d, pkg_subdir, pkg_conf_opt, sysroot_host) = 
> > ask_ptxdist(ptx_PKG)
> >     File "./scripts/configure_helper.py", line 165, in ask_ptxdist
> >       universal_newlines=True)
> >     File "/usr/lib/python3.7/subprocess.py", line 775, in __init__
> >       restore_signals, start_new_session)
> >     File "/usr/lib/python3.7/subprocess.py", line 1522, in _execute_child
> >       raise child_exception_type(errno_num, err_msg, err_filename)
> >   PermissionError: [Errno 13] Permission denied: ''
> 
> I think, catching the exception and printing an error with the failing
> commandline would be better. Other bogus, non-empty strings will cause
> exceptions as well.

Yeah, after having a night of sleep over this, I think this is also the
better approach.

 - Roland


> 
> Michael
> 
> > > 
> > > > On Wed, Sep 25, 2019 at 03:13:40PM +0200, Roland Hieber wrote:
> > > > > When the environment variable exists, but is empty, 
> > > > > os.environment.get()
> > > > > will return its value instead of using the supplied default. Check for
> > > > > cases like that to prevent calling an empty command.
> > > > > 
> > > > > Signed-off-by: Roland Hieber <[email protected]>
> > > > > ---
> > > > >  v1 -> v2:
> > > > >   - prevent "AttributeError: 'NoneType' object has no attribute 
> > > > > 'strip'"
> > > > >     if none of the checked environment variables are set
> > > > > 
> > > > >  scripts/configure_helper.py | 7 ++++++-
> > > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/scripts/configure_helper.py b/scripts/configure_helper.py
> > > > > index c7b46f3b3846..73dd4a2add1c 100755
> > > > > --- a/scripts/configure_helper.py
> > > > > +++ b/scripts/configure_helper.py
> > > > > @@ -151,7 +151,12 @@ def abort(message):
> > > > >       exit(1)
> > > > >  
> > > > >  def ask_ptxdist(pkg):
> > > > > -     ptxdist = os.environ.get("PTXDIST", os.environ.get("ptxdist", 
> > > > > "ptxdist"))
> > > > > +     ptxdist = os.environ.get("PTXDIST")
> > > > > +     if not ptxdist or not ptxdist.strip():
> > > > > +             ptxdist = os.environ.get("ptxdist")
> > > > > +     if not ptxdist or not ptxdist.strip():
> > > > > +             ptxdist = "ptxdist"
> > > > > +     
> > > > >       p = subprocess.Popen([ ptxdist, "-k", "make",
> > > > >               "/print-%s_DIR" % pkg,
> > > > >               "/print-%s_SUBDIR" % pkg,
> > > > > -- 
> > > > > 2.23.0

-- 
Roland Hieber                     | [email protected]     |
Pengutronix e.K.                  | https://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
[email protected]

Reply via email to