[OE-core] [PATCH 1/6] oe.scriptutils.run_editor: ditch the error-prone argument quoting

2018-06-21 Thread Christopher Larson 
From: Christopher Larson 

Rather than trying to construct a string by quoting the files in an
error-prone way, parse $EDITOR to pass a list to subprocess rather than
a string.

Signed-off-by: Christopher Larson 
---
 scripts/lib/scriptutils.py | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/scripts/lib/scriptutils.py b/scripts/lib/scriptutils.py
index 85b1c949bf5..470f76995f0 100644
--- a/scripts/lib/scriptutils.py
+++ b/scripts/lib/scriptutils.py
@@ -15,16 +15,17 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
-import sys
-import os
-import logging
-import glob
 import argparse
-import subprocess
-import tempfile
-import shutil
+import glob
+import logging
+import os
 import random
+import shlex
+import shutil
 import string
+import subprocess
+import sys
+import tempfile
 
 def logger_create(name, stream=None):
 logger = logging.getLogger(name)
@@ -214,15 +215,14 @@ def fetch_url(tinfoil, srcuri, srcrev, destdir, logger, 
preserve_tmp=False, mirr
 
 def run_editor(fn, logger=None):
 if isinstance(fn, str):
-params = '"%s"' % fn
+files = [fn]
 else:
-params = ''
-for fnitem in fn:
-params += ' "%s"' % fnitem
+files = fn
 
 editor = os.getenv('VISUAL', os.getenv('EDITOR', 'vi'))
 try:
-return subprocess.check_call('%s %s' % (editor, params), shell=True)
+print(shlex.split(editor) + files)
+return subprocess.check_call(shlex.split(editor) + files)
 except subprocess.CalledProcessError as exc:
 logger.error("Execution of '%s' failed: %s" % (editor, exc))
 return 1
-- 
2.11.1

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH 1/6] oe.scriptutils.run_editor: ditch the error-prone argument quoting

2018-06-27 Thread Richard Purdie 
On Fri, 2018-06-22 at 02:07 +0500, Christopher Larson wrote:
> From: Christopher Larson 
> 
> Rather than trying to construct a string by quoting the files in an
> error-prone way, parse $EDITOR to pass a list to subprocess rather
> than
> a string.
> 
> Signed-off-by: Christopher Larson 
> ---
>  scripts/lib/scriptutils.py | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/lib/scriptutils.py b/scripts/lib/scriptutils.py
> index 85b1c949bf5..470f76995f0 100644
> --- a/scripts/lib/scriptutils.py
> +++ b/scripts/lib/scriptutils.py
> @@ -214,15 +215,14 @@ def fetch_url(tinfoil, srcuri, srcrev, destdir,
> logger, preserve_tmp=False, mirr
>  
>  def run_editor(fn, logger=None):
>  if isinstance(fn, str):
> -params = '"%s"' % fn
> +files = [fn]
>  else:
> -params = ''
> -for fnitem in fn:
> -params += ' "%s"' % fnitem
> +files = fn
>  
>  editor = os.getenv('VISUAL', os.getenv('EDITOR', 'vi'))
>  try:
> -return subprocess.check_call('%s %s' % (editor, params), shell=True)
> +print(shlex.split(editor) + files)
> +return subprocess.check_call(shlex.split(editor) + files)
>  except subprocess.CalledProcessError as exc:
>  logger.error("Execution of '%s' failed: %s" % (editor, exc))
>  return 1

I'm guessing you didn't mean to leave the print() above in?

If the print is there, it breaks:

"oe-selftest -r devtool.DevtoolTests.test_devtool_add"

I can comment it out...

Cheers,

Richard


-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH 1/6] oe.scriptutils.run_editor: ditch the error-prone argument quoting

2018-06-27 Thread Christopher Larson 
On Wed, Jun 27, 2018 at 5:52 AM Richard Purdie <
richard.pur...@linuxfoundation.org> wrote:

> On Fri, 2018-06-22 at 02:07 +0500, Christopher Larson wrote:
> > From: Christopher Larson 
> >
> > Rather than trying to construct a string by quoting the files in an
> > error-prone way, parse $EDITOR to pass a list to subprocess rather
> > than
> > a string.
> >
> > Signed-off-by: Christopher Larson 
> > ---
> >  scripts/lib/scriptutils.py | 24 
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/scripts/lib/scriptutils.py b/scripts/lib/scriptutils.py
> > index 85b1c949bf5..470f76995f0 100644
> > --- a/scripts/lib/scriptutils.py
> > +++ b/scripts/lib/scriptutils.py
> > @@ -214,15 +215,14 @@ def fetch_url(tinfoil, srcuri, srcrev, destdir,
> > logger, preserve_tmp=False, mirr
> >
> >  def run_editor(fn, logger=None):
> >  if isinstance(fn, str):
> > -params = '"%s"' % fn
> > +files = [fn]
> >  else:
> > -params = ''
> > -for fnitem in fn:
> > -params += ' "%s"' % fnitem
> > +files = fn
> >
> >  editor = os.getenv('VISUAL', os.getenv('EDITOR', 'vi'))
> >  try:
> > -return subprocess.check_call('%s %s' % (editor, params),
> shell=True)
> > +print(shlex.split(editor) + files)
> > +return subprocess.check_call(shlex.split(editor) + files)
> >  except subprocess.CalledProcessError as exc:
> >  logger.error("Execution of '%s' failed: %s" % (editor, exc))
> >  return 1
>
> I'm guessing you didn't mean to leave the print() above in?
>
> If the print is there, it breaks:
>
> "oe-selftest -r devtool.DevtoolTests.test_devtool_add"
>
> I can comment it out...
>

Yeah, that was a debug remnant, sorry about that. :\
-- 
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core