[PATCH 0/6] devtool/recipetool fixes

[PATCH 0/6] devtool/recipetool fixes

[Paul Eggleton]

A bunch of fixes and minor improvements for devtool & recipetool.


The following changes since commit 53fcfe4348a2ca727844f2b0bd3fca2902cbdda0:

  lib/oeqa: add Galculator to SDK and runtime tests (2016-07-01 16:08:54 +0100)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib paule/devtool18
  http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=paule/devtool18

Paul Eggleton (6):
  recipetool: create: fix handling of github URLs
  recipetool: create: support specifying a file as the local source
  lib/oe/patch: handle encoding differences in patch files
  devtool: update-recipe: fix --initial-rev option
  devtool: reset: allow reset to work if the recipe file has been deleted
  scripts/contrib/devtool-stress: exclude more recipes by default

 meta/lib/oe/patch.py              | 100 ++++++++++++++++++++++----------------
 scripts/contrib/devtool-stress.py |   2 +-
 scripts/lib/devtool/standard.py   |  19 ++++----
 scripts/lib/recipetool/create.py  |  17 ++++---
 4 files changed, 79 insertions(+), 59 deletions(-)

-- 
2.5.5

[PATCH 1/6] recipetool: create: fix handling of github URLs

[Paul Eggleton]

For a while now, Github hasn't been advertising a specific repository
URL since cloning the web URL with git works. Armed with this knowledge
and fully expecting people to just paste the github URL, we need to
handle this situation specially. If it looks like a github URL to the
root of a repository then treat it as a git repository instead of a
normal https URL to be fetched by the wget fetcher.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/recipetool/create.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index 1297428..c6b9d36 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -324,7 +324,7 @@ def supports_srcrev(uri):
 def reformat_git_uri(uri):
     '''Convert any http[s]://....git URI into git://...;protocol=http[s]'''
     checkuri = uri.split(';', 1)[0]
-    if checkuri.endswith('.git') or '/git/' in checkuri:
+    if checkuri.endswith('.git') or '/git/' in checkuri or re.match('https?://github.com/[^/]+/[^/]+/?', checkuri):
         res = re.match('(https?)://([^;]+(\.git)?)(;.*)?$', uri)
         if res:
             # Need to switch the URI around so that the git fetcher is used
-- 
2.5.5

[PATCH 2/6] recipetool: create: support specifying a file as the local source

[Paul Eggleton]

It is currently possible to specify a file (e.g. a tarball) on the local
disk as the source, but you have to know to put file:// in front of it.
There's really no need to force users to jump through that hoop if they
really want to do this so check if the specified source is a file and
prefix it with file:// if that's the case.

Also ensure the same works for "devtool add" at the same time.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/devtool/standard.py  |  4 ++++
 scripts/lib/recipetool/create.py | 15 ++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index ed49a93..737ecc1 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -65,6 +65,10 @@ def add(args, config, basepath, workspace):
         elif os.path.isdir(args.recipename):
             logger.warn('Ambiguous argument %s - assuming you mean it to be the recipe name')
 
+    if args.srctree and os.path.isfile(args.srctree):
+        args.fetchuri = 'file://' + os.path.abspath(args.srctree)
+        args.srctree = ''
+
     if args.fetch:
         if args.fetchuri:
             raise DevtoolError('URI specified as positional argument as well as -f/--fetch')
diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index c6b9d36..12a25c6 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -351,11 +351,16 @@ def create_recipe(args):
     extravalues = {}
     checksums = (None, None)
     tempsrc = ''
+    source = args.source
     srcsubdir = ''
     srcrev = '${AUTOREV}'
-    if '://' in args.source:
+
+    if os.path.isfile(source):
+        source = 'file://%s' % os.path.abspath(source)
+
+    if '://' in source:
         # Fetch a URL
-        fetchuri = reformat_git_uri(urldefrag(args.source)[0])
+        fetchuri = reformat_git_uri(urldefrag(source)[0])
         if args.binary:
             # Assume the archive contains the directory structure verbatim
             # so we need to extract to a subdirectory
@@ -426,10 +431,10 @@ def create_recipe(args):
         if args.extract_to:
             logger.error('--extract-to cannot be specified if source is a directory')
             sys.exit(1)
-        if not os.path.isdir(args.source):
-            logger.error('Invalid source directory %s' % args.source)
+        if not os.path.isdir(source):
+            logger.error('Invalid source directory %s' % source)
             sys.exit(1)
-        srctree = args.source
+        srctree = source
         srcuri = ''
         if os.path.exists(os.path.join(srctree, '.git')):
             # Try to get upstream repo location from origin remote
-- 
2.5.5

[PATCH 3/6] lib/oe/patch: handle encoding differences in patch files

[Paul Eggleton]

With Python 3, the encoding of a file is significant; several recipes in
OE-Core have patches which are not fully utf-8 decodable e.g. man,
lrzsz, and gstreamer1.0-libav, leading to errors when using devtool's
modify, upgrade or extract subcommands on these recipes. To work around
this, try reading the patch file as utf-8 first and if that fails try
latin-1 before giving up.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oe/patch.py | 100 +++++++++++++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 43 deletions(-)

diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index 4a0d3f7..af3adec 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -117,43 +117,50 @@ class PatchSet(object):
                 return None
             return os.sep.join(filesplit[striplevel:])
 
-        copiedmode = False
-        filelist = []
-        with open(patchfile) as f:
-            for line in f:
-                if line.startswith('--- '):
-                    patchpth = patchedpath(line)
-                    if not patchpth:
-                        break
-                    if copiedmode:
-                        addedfile = patchpth
-                    else:
-                        removedfile = patchpth
-                elif line.startswith('+++ '):
-                    addedfile = patchedpath(line)
-                    if not addedfile:
-                        break
-                elif line.startswith('*** '):
-                    copiedmode = True
-                    removedfile = patchedpath(line)
-                    if not removedfile:
-                        break
-                else:
-                    removedfile = None
-                    addedfile = None
-
-                if addedfile and removedfile:
-                    if removedfile == '/dev/null':
-                        mode = 'A'
-                    elif addedfile == '/dev/null':
-                        mode = 'D'
-                    else:
-                        mode = 'M'
-                    if srcdir:
-                        fullpath = os.path.abspath(os.path.join(srcdir, addedfile))
-                    else:
-                        fullpath = addedfile
-                    filelist.append((fullpath, mode))
+        for encoding in ['utf-8', 'latin-1']:
+            try:
+                copiedmode = False
+                filelist = []
+                with open(patchfile) as f:
+                    for line in f:
+                        if line.startswith('--- '):
+                            patchpth = patchedpath(line)
+                            if not patchpth:
+                                break
+                            if copiedmode:
+                                addedfile = patchpth
+                            else:
+                                removedfile = patchpth
+                        elif line.startswith('+++ '):
+                            addedfile = patchedpath(line)
+                            if not addedfile:
+                                break
+                        elif line.startswith('*** '):
+                            copiedmode = True
+                            removedfile = patchedpath(line)
+                            if not removedfile:
+                                break
+                        else:
+                            removedfile = None
+                            addedfile = None
+
+                        if addedfile and removedfile:
+                            if removedfile == '/dev/null':
+                                mode = 'A'
+                            elif addedfile == '/dev/null':
+                                mode = 'D'
+                            else:
+                                mode = 'M'
+                            if srcdir:
+                                fullpath = os.path.abspath(os.path.join(srcdir, addedfile))
+                            else:
+                                fullpath = addedfile
+                            filelist.append((fullpath, mode))
+            except UnicodeDecodeError:
+                continue
+            break
+        else:
+            raise PatchError('Unable to decode %s' % patchfile)
 
         return filelist
 
@@ -280,12 +287,19 @@ class GitApplyTree(PatchTree):
         """
         Extract just the header lines from the top of a patch file
         """
-        lines = []
-        with open(patchfile, 'r') as f:
-            for line in f.readlines():
-                if line.startswith('Index: ') or line.startswith('diff -') or line.startswith('---'):
-                    break
-                lines.append(line)
+        for encoding in ['utf-8', 'latin-1']:
+            lines = []
+            try:
+                with open(patchfile, 'r', encoding=encoding) as f:
+                    for line in f:
+                        if line.startswith('Index: ') or line.startswith('diff -') or line.startswith('---'):
+                            break
+                        lines.append(line)
+            except UnicodeDecodeError:
+                continue
+            break
+        else:
+            raise PatchError('Unable to find a character encoding to decode %s' % patchfile)
         return lines
 
     @staticmethod
-- 
2.5.5

[PATCH 4/6] devtool: update-recipe: fix --initial-rev option

[Paul Eggleton]

In OE-Core revision 7baf57ad896112cf2258b3e2c2a1f8b756fb39bc I changed
the default update-recipe behaviour to only update patches for commits
that were changed; unfortunately I failed to handle the --initial-rev
option which was broken after that point. Rework how the initial
revision is passed in so that it now operates correctly.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 737ecc1..a7e365f 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -815,22 +815,19 @@ def modify(args, config, basepath, workspace):
 
     return 0
 
-def _get_patchset_revs(args, srctree, recipe_path):
+def _get_patchset_revs(srctree, recipe_path, initial_rev=None):
     """Get initial and update rev of a recipe. These are the start point of the
     whole patchset and start point for the patches to be re-generated/updated.
     """
     import bb
 
-    if args.initial_rev:
-        return args.initial_rev, args.initial_rev
-
-    # Parse initial rev from recipe
+    # Parse initial rev from recipe if not specified
     commits = []
-    initial_rev = None
     with open(recipe_path, 'r') as f:
         for line in f:
             if line.startswith('# initial_rev:'):
-                initial_rev = line.split(':')[-1].strip()
+                if not initial_rev:
+                    initial_rev = line.split(':')[-1].strip()
             elif line.startswith('# commit:'):
                 commits.append(line.split(':')[-1].strip())
 
@@ -1129,7 +1126,7 @@ def _update_recipe_patch(args, config, workspace, srctree, rd, config_data):
         raise DevtoolError('unable to find workspace bbappend for recipe %s' %
                            args.recipename)
 
-    initial_rev, update_rev, changed_revs = _get_patchset_revs(args, srctree, append)
+    initial_rev, update_rev, changed_revs = _get_patchset_revs(srctree, append, args.initial_rev)
     if not initial_rev:
         raise DevtoolError('Unable to find initial revision - please specify '
                            'it with --initial-rev')
-- 
2.5.5

[PATCH 5/6] devtool: reset: allow reset to work if the recipe file has been deleted

[Paul Eggleton]

We were attempting to open the recipe file unconditionally here - we
need to account for the possibility that the recipe file has been
deleted or moved away by the user.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index a7e365f..8236dd5 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -1327,7 +1327,7 @@ def reset(args, config, basepath, workspace):
         for recipe in recipes:
             targets.append(recipe)
             recipefile = workspace[recipe]['recipefile']
-            if recipefile:
+            if recipefile and os.path.exists(recipefile):
                 targets.extend(get_bbclassextend_targets(recipefile, recipe))
         try:
             exec_build_env_command(config.init_path, basepath, 'bitbake -c clean %s' % ' '.join(targets))
-- 
2.5.5

[PATCH 6/6] scripts/contrib/devtool-stress: exclude more recipes by default

[Paul Eggleton]

These recipes can't be used with devtool because they can't be unpacked
in the normal way.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/contrib/devtool-stress.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/contrib/devtool-stress.py b/scripts/contrib/devtool-stress.py
index ab77a2d..5f396aa 100755
--- a/scripts/contrib/devtool-stress.py
+++ b/scripts/contrib/devtool-stress.py
@@ -210,7 +210,7 @@ def main():
     parser.add_argument('-d', '--debug', help='Enable debug output', action='store_true')
     parser.add_argument('-r', '--resume-from', help='Resume from specified recipe', metavar='PN')
     parser.add_argument('-o', '--only', help='Only test specified recipes (comma-separated without spaces, wildcards allowed)', metavar='PNLIST')
-    parser.add_argument('-s', '--skip', help='Skip specified recipes (comma-separated without spaces, wildcards allowed)', metavar='PNLIST')
+    parser.add_argument('-s', '--skip', help='Skip specified recipes (comma-separated without spaces, wildcards allowed)', metavar='PNLIST', default='gcc-source-*,kernel-devsrc,package-index,perf,meta-world-pkgdata,glibc-locale,glibc-mtrace,glibc-scripts,os-release')
     parser.add_argument('-c', '--skip-classes', help='Skip recipes inheriting specified classes (comma-separated) - default %(default)s', metavar='CLASSLIST', default='native,nativesdk,cross,cross-canadian,image,populate_sdk,meta,packagegroup')
     subparsers = parser.add_subparsers(title='subcommands', metavar='<subcommand>')
     subparsers.required = True
-- 
2.5.5

Re: [PATCH 3/6] lib/oe/patch: handle encoding differences in patch files

[Khem Raj]

On Wed, Jul 6, 2016 at 4:57 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> With Python 3, the encoding of a file is significant; several recipes in
> OE-Core have patches which are not fully utf-8 decodable e.g. man,
> lrzsz, and gstreamer1.0-libav, leading to errors when using devtool's
> modify, upgrade or extract subcommands on these recipes. To work around
> this, try reading the patch file as utf-8 first and if that fails try
> latin-1 before giving up.


while this makes devtool robust. I think it will also be a good
opportunity to inform
the user/developer of the utf-8 non compliant file and may be a hint
on how to fix it if possible

>
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> ---
>  meta/lib/oe/patch.py | 100 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 57 insertions(+), 43 deletions(-)
>
> diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
> index 4a0d3f7..af3adec 100644
> --- a/meta/lib/oe/patch.py
> +++ b/meta/lib/oe/patch.py
> @@ -117,43 +117,50 @@ class PatchSet(object):
>                  return None
>              return os.sep.join(filesplit[striplevel:])
>
> -        copiedmode = False
> -        filelist = []
> -        with open(patchfile) as f:
> -            for line in f:
> -                if line.startswith('--- '):
> -                    patchpth = patchedpath(line)
> -                    if not patchpth:
> -                        break
> -                    if copiedmode:
> -                        addedfile = patchpth
> -                    else:
> -                        removedfile = patchpth
> -                elif line.startswith('+++ '):
> -                    addedfile = patchedpath(line)
> -                    if not addedfile:
> -                        break
> -                elif line.startswith('*** '):
> -                    copiedmode = True
> -                    removedfile = patchedpath(line)
> -                    if not removedfile:
> -                        break
> -                else:
> -                    removedfile = None
> -                    addedfile = None
> -
> -                if addedfile and removedfile:
> -                    if removedfile == '/dev/null':
> -                        mode = 'A'
> -                    elif addedfile == '/dev/null':
> -                        mode = 'D'
> -                    else:
> -                        mode = 'M'
> -                    if srcdir:
> -                        fullpath = os.path.abspath(os.path.join(srcdir, addedfile))
> -                    else:
> -                        fullpath = addedfile
> -                    filelist.append((fullpath, mode))
> +        for encoding in ['utf-8', 'latin-1']:
> +            try:
> +                copiedmode = False
> +                filelist = []
> +                with open(patchfile) as f:
> +                    for line in f:
> +                        if line.startswith('--- '):
> +                            patchpth = patchedpath(line)
> +                            if not patchpth:
> +                                break
> +                            if copiedmode:
> +                                addedfile = patchpth
> +                            else:
> +                                removedfile = patchpth
> +                        elif line.startswith('+++ '):
> +                            addedfile = patchedpath(line)
> +                            if not addedfile:
> +                                break
> +                        elif line.startswith('*** '):
> +                            copiedmode = True
> +                            removedfile = patchedpath(line)
> +                            if not removedfile:
> +                                break
> +                        else:
> +                            removedfile = None
> +                            addedfile = None
> +
> +                        if addedfile and removedfile:
> +                            if removedfile == '/dev/null':
> +                                mode = 'A'
> +                            elif addedfile == '/dev/null':
> +                                mode = 'D'
> +                            else:
> +                                mode = 'M'
> +                            if srcdir:
> +                                fullpath = os.path.abspath(os.path.join(srcdir, addedfile))
> +                            else:
> +                                fullpath = addedfile
> +                            filelist.append((fullpath, mode))
> +            except UnicodeDecodeError:
> +                continue
> +            break
> +        else:
> +            raise PatchError('Unable to decode %s' % patchfile)
>
>          return filelist
>
> @@ -280,12 +287,19 @@ class GitApplyTree(PatchTree):
>          """
>          Extract just the header lines from the top of a patch file
>          """
> -        lines = []
> -        with open(patchfile, 'r') as f:
> -            for line in f.readlines():
> -                if line.startswith('Index: ') or line.startswith('diff -') or line.startswith('---'):
> -                    break
> -                lines.append(line)
> +        for encoding in ['utf-8', 'latin-1']:
> +            lines = []
> +            try:
> +                with open(patchfile, 'r', encoding=encoding) as f:
> +                    for line in f:
> +                        if line.startswith('Index: ') or line.startswith('diff -') or line.startswith('---'):
> +                            break
> +                        lines.append(line)
> +            except UnicodeDecodeError:
> +                continue
> +            break
> +        else:
> +            raise PatchError('Unable to find a character encoding to decode %s' % patchfile)
>          return lines
>
>      @staticmethod
> --
> 2.5.5
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH 6/6] scripts/contrib/devtool-stress: exclude more recipes by default

[Khem Raj]

On Wed, Jul 6, 2016 at 4:57 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> These recipes can't be used with devtool because they can't be unpacked
> in the normal way.

May be we should have a devtoop blacklist variable that can be added
to the recipe

>
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> ---
>  scripts/contrib/devtool-stress.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/contrib/devtool-stress.py b/scripts/contrib/devtool-stress.py
> index ab77a2d..5f396aa 100755
> --- a/scripts/contrib/devtool-stress.py
> +++ b/scripts/contrib/devtool-stress.py
> @@ -210,7 +210,7 @@ def main():
>      parser.add_argument('-d', '--debug', help='Enable debug output', action='store_true')
>      parser.add_argument('-r', '--resume-from', help='Resume from specified recipe', metavar='PN')
>      parser.add_argument('-o', '--only', help='Only test specified recipes (comma-separated without spaces, wildcards allowed)', metavar='PNLIST')
> -    parser.add_argument('-s', '--skip', help='Skip specified recipes (comma-separated without spaces, wildcards allowed)', metavar='PNLIST')
> +    parser.add_argument('-s', '--skip', help='Skip specified recipes (comma-separated without spaces, wildcards allowed)', metavar='PNLIST', default='gcc-source-*,kernel-devsrc,package-index,perf,meta-world-pkgdata,glibc-locale,glibc-mtrace,glibc-scripts,os-release')
>      parser.add_argument('-c', '--skip-classes', help='Skip recipes inheriting specified classes (comma-separated) - default %(default)s', metavar='CLASSLIST', default='native,nativesdk,cross,cross-canadian,image,populate_sdk,meta,packagegroup')
>      subparsers = parser.add_subparsers(title='subcommands', metavar='<subcommand>')
>      subparsers.required = True
> --
> 2.5.5
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH 6/6] scripts/contrib/devtool-stress: exclude more recipes by default

[Paul Eggleton]

Hi Khem,

devtool itself already understands not to try to deal with these recipes - it 
will print out a reasonable error for the user. The exception list is just for 
the devtool-stress script so that it produces clean output.

Cheers,
Paul

On Wed, 06 Jul 2016 17:42:22 Khem Raj wrote:
> On Wed, Jul 6, 2016 at 4:57 PM, Paul Eggleton
> 
> <paul.eggleton@linux.intel.com> wrote:
> > These recipes can't be used with devtool because they can't be unpacked
> > in the normal way.
> 
> May be we should have a devtoop blacklist variable that can be added
> to the recipe
> 
> > Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> > ---
> > 
> >  scripts/contrib/devtool-stress.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/contrib/devtool-stress.py
> > b/scripts/contrib/devtool-stress.py index ab77a2d..5f396aa 100755
> > --- a/scripts/contrib/devtool-stress.py
> > +++ b/scripts/contrib/devtool-stress.py
> > 
> > @@ -210,7 +210,7 @@ def main():
> >      parser.add_argument('-d', '--debug', help='Enable debug output',
> >      action='store_true') parser.add_argument('-r', '--resume-from',
> >      help='Resume from specified recipe', metavar='PN')
> >      parser.add_argument('-o', '--only', help='Only test specified
> >      recipes (comma-separated without spaces, wildcards allowed)',
> >      metavar='PNLIST')> 
> > -    parser.add_argument('-s', '--skip', help='Skip specified recipes
> > (comma-separated without spaces, wildcards allowed)', metavar='PNLIST') +
> >    parser.add_argument('-s', '--skip', help='Skip specified recipes
> > (comma-separated without spaces, wildcards allowed)', metavar='PNLIST',
> > default='gcc-source-*,kernel-devsrc,package-index,perf,meta-world-pkgdata
> > ,glibc-locale,glibc-mtrace,glibc-scripts,os-release')> 
> >      parser.add_argument('-c', '--skip-classes', help='Skip recipes
> >      inheriting specified classes (comma-separated) - default
> >      %(default)s', metavar='CLASSLIST',
> >      default='native,nativesdk,cross,cross-canadian,image,populate_sdk,me
> >      ta,packagegroup') subparsers =
> >      parser.add_subparsers(title='subcommands', metavar='<subcommand>')
> >      subparsers.required = True
> > 
> > --
> > 2.5.5
> > 
> > --
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 3/6] lib/oe/patch: handle encoding differences in patch files

[Paul Eggleton]

On Wed, 06 Jul 2016 17:40:03 Khem Raj wrote:
> On Wed, Jul 6, 2016 at 4:57 PM, Paul Eggleton
> <paul.eggleton@linux.intel.com> wrote:
> > With Python 3, the encoding of a file is significant; several recipes in
> > OE-Core have patches which are not fully utf-8 decodable e.g. man,
> > lrzsz, and gstreamer1.0-libav, leading to errors when using devtool's
> > modify, upgrade or extract subcommands on these recipes. To work around
> > this, try reading the patch file as utf-8 first and if that fails try
> > latin-1 before giving up.
> 
> while this makes devtool robust. I think it will also be a good
> opportunity to inform
> the user/developer of the utf-8 non compliant file and may be a hint
> on how to fix it if possible

Sure, I could easily have it print a warning, but I'm not sure this is going 
to be fixable for all cases. If the patch header is the source of the non-UTF8 
characters then it's easy, but what about if the code being patched has non-
UTF8 encoding? Surely in that case the patch has to correspond?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 6/6] scripts/contrib/devtool-stress: exclude more recipes by default

[Khem Raj]

On Wed, Jul 6, 2016 at 6:39 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> Hi Khem,
>
> devtool itself already understands not to try to deal with these recipes - it
> will print out a reasonable error for the user. The exception list is just for
> the devtool-stress script so that it produces clean output.

OK understood the case, I was just thinking of cases where recipes are external
and for some reason dont yet work with devtool, may be can be
blacklisted in recipes
instead of having a central list.

>
> Cheers,
> Paul
>
> On Wed, 06 Jul 2016 17:42:22 Khem Raj wrote:
>> On Wed, Jul 6, 2016 at 4:57 PM, Paul Eggleton
>>
>> <paul.eggleton@linux.intel.com> wrote:
>> > These recipes can't be used with devtool because they can't be unpacked
>> > in the normal way.
>>
>> May be we should have a devtoop blacklist variable that can be added
>> to the recipe
>>
>> > Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
>> > ---
>> >
>> >  scripts/contrib/devtool-stress.py | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/scripts/contrib/devtool-stress.py
>> > b/scripts/contrib/devtool-stress.py index ab77a2d..5f396aa 100755
>> > --- a/scripts/contrib/devtool-stress.py
>> > +++ b/scripts/contrib/devtool-stress.py
>> >
>> > @@ -210,7 +210,7 @@ def main():
>> >      parser.add_argument('-d', '--debug', help='Enable debug output',
>> >      action='store_true') parser.add_argument('-r', '--resume-from',
>> >      help='Resume from specified recipe', metavar='PN')
>> >      parser.add_argument('-o', '--only', help='Only test specified
>> >      recipes (comma-separated without spaces, wildcards allowed)',
>> >      metavar='PNLIST')>
>> > -    parser.add_argument('-s', '--skip', help='Skip specified recipes
>> > (comma-separated without spaces, wildcards allowed)', metavar='PNLIST') +
>> >    parser.add_argument('-s', '--skip', help='Skip specified recipes
>> > (comma-separated without spaces, wildcards allowed)', metavar='PNLIST',
>> > default='gcc-source-*,kernel-devsrc,package-index,perf,meta-world-pkgdata
>> > ,glibc-locale,glibc-mtrace,glibc-scripts,os-release')>
>> >      parser.add_argument('-c', '--skip-classes', help='Skip recipes
>> >      inheriting specified classes (comma-separated) - default
>> >      %(default)s', metavar='CLASSLIST',
>> >      default='native,nativesdk,cross,cross-canadian,image,populate_sdk,me
>> >      ta,packagegroup') subparsers =
>> >      parser.add_subparsers(title='subcommands', metavar='<subcommand>')
>> >      subparsers.required = True
>> >
>> > --
>> > 2.5.5
>> >
>> > --
>> > _______________________________________________
>> > Openembedded-core mailing list
>> > Openembedded-core@lists.openembedded.org
>> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
> --
>
> Paul Eggleton
> Intel Open Source Technology Centre

Re: [PATCH 3/6] lib/oe/patch: handle encoding differences in patch files

[Khem Raj]

On Wed, Jul 6, 2016 at 6:45 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> On Wed, 06 Jul 2016 17:40:03 Khem Raj wrote:
>> On Wed, Jul 6, 2016 at 4:57 PM, Paul Eggleton
>> <paul.eggleton@linux.intel.com> wrote:
>> > With Python 3, the encoding of a file is significant; several recipes in
>> > OE-Core have patches which are not fully utf-8 decodable e.g. man,
>> > lrzsz, and gstreamer1.0-libav, leading to errors when using devtool's
>> > modify, upgrade or extract subcommands on these recipes. To work around
>> > this, try reading the patch file as utf-8 first and if that fails try
>> > latin-1 before giving up.
>>
>> while this makes devtool robust. I think it will also be a good
>> opportunity to inform
>> the user/developer of the utf-8 non compliant file and may be a hint
>> on how to fix it if possible
>
> Sure, I could easily have it print a warning, but I'm not sure this is going
> to be fixable for all cases. If the patch header is the source of the non-UTF8
> characters then it's easy, but what about if the code being patched has non-
> UTF8 encoding? Surely in that case the patch has to correspond?

yes if the patch is patching a non UTF code for some reason then that
may not be flagged.

>
> Cheers,
> Paul
>
> --
>
> Paul Eggleton
> Intel Open Source Technology Centre

Re: [PATCH 6/6] scripts/contrib/devtool-stress: exclude more recipes by default

[Paul Eggleton]

On Wed, 06 Jul 2016 18:54:37 Khem Raj wrote:
> On Wed, Jul 6, 2016 at 6:39 PM, Paul Eggleton
> <paul.eggleton@linux.intel.com> wrote:
> > devtool itself already understands not to try to deal with these recipes -
> > it will print out a reasonable error for the user. The exception list is
> > just for the devtool-stress script so that it produces clean output.
> 
> OK understood the case, I was just thinking of cases where recipes are
> external and for some reason dont yet work with devtool, may be can be
> blacklisted in recipes
> instead of having a central list.

Let's break this down a bit:

devtool itself looks at various things to determine whether the recipe can be 
used with "devtool modify" - the implementation is in 
_check_compatible_recipe() in scripts/lib/devtool/standard.py. I don't think 
it would make sense to break this up.

The devtool-stress.py script is just for testing. I'm not even sure if anyone 
else but me actually runs it. To go and put things in a bunch of recipes 
solely for the purpose of this script seems a bit excessive.

Thinking about it, I guess we could have devtool modify return a particular 
exit code for incompatible recipes and have the stress script consider those 
"skipped" rather than "failed". That to me would be a better alternative.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 6/6] scripts/contrib/devtool-stress: exclude more recipes by default

[Khem Raj]

On Wed, Jul 6, 2016 at 7:20 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> On Wed, 06 Jul 2016 18:54:37 Khem Raj wrote:
>> On Wed, Jul 6, 2016 at 6:39 PM, Paul Eggleton
>> <paul.eggleton@linux.intel.com> wrote:
>> > devtool itself already understands not to try to deal with these recipes -
>> > it will print out a reasonable error for the user. The exception list is
>> > just for the devtool-stress script so that it produces clean output.
>>
>> OK understood the case, I was just thinking of cases where recipes are
>> external and for some reason dont yet work with devtool, may be can be
>> blacklisted in recipes
>> instead of having a central list.
>
> Let's break this down a bit:
>
> devtool itself looks at various things to determine whether the recipe can be
> used with "devtool modify" - the implementation is in
> _check_compatible_recipe() in scripts/lib/devtool/standard.py. I don't think
> it would make sense to break this up.
>
> The devtool-stress.py script is just for testing. I'm not even sure if anyone
> else but me actually runs it. To go and put things in a bunch of recipes
> solely for the purpose of this script seems a bit excessive.

OK if devtool has explicit logic for detecting this its fine,
hopefully it will cover
all cases and it can be extended if dubious errors are found over time.

>
> Thinking about it, I guess we could have devtool modify return a particular
> exit code for incompatible recipes and have the stress script consider those
> "skipped" rather than "failed". That to me would be a better alternative.

Certainly it will improve the diagnostics

>
> Cheers,
> Paul
>
> --
>
> Paul Eggleton
> Intel Open Source Technology Centre

Re: [PATCH 3/6] lib/oe/patch: handle encoding differences in patch files

[Paul Eggleton]

On Wed, 06 Jul 2016 18:56:09 Khem Raj wrote:
> On Wed, Jul 6, 2016 at 6:45 PM, Paul Eggleton
> <paul.eggleton@linux.intel.com> wrote:
> > On Wed, 06 Jul 2016 17:40:03 Khem Raj wrote:
> >> On Wed, Jul 6, 2016 at 4:57 PM, Paul Eggleton
> >> 
> >> <paul.eggleton@linux.intel.com> wrote:
> >> > With Python 3, the encoding of a file is significant; several recipes
> >> > in
> >> > OE-Core have patches which are not fully utf-8 decodable e.g. man,
> >> > lrzsz, and gstreamer1.0-libav, leading to errors when using devtool's
> >> > modify, upgrade or extract subcommands on these recipes. To work around
> >> > this, try reading the patch file as utf-8 first and if that fails try
> >> > latin-1 before giving up.
> >> 
> >> while this makes devtool robust. I think it will also be a good
> >> opportunity to inform
> >> the user/developer of the utf-8 non compliant file and may be a hint
> >> on how to fix it if possible
> > 
> > Sure, I could easily have it print a warning, but I'm not sure this is
> > going to be fixable for all cases. If the patch header is the source of
> > the non-UTF8 characters then it's easy, but what about if the code being
> > patched has non- UTF8 encoding? Surely in that case the patch has to
> > correspond?
> 
> yes if the patch is patching a non UTF code for some reason then that
> may not be flagged.

I'm not about to dissect the patch in this context just to give a warning 
though - either we succeed in decoding it or we don't. If we want to do so as 
a test on incoming patches, we could, though I suspect we have bigger fish to 
fry.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre