[PATCH 0/7] devtool/recipetool improvements

[PATCH 0/7] devtool/recipetool improvements

[Paul Eggleton]

The following changes since commit b18fa5f2f2f46afc6fdc58f4d29679dea9c36c43:

  Remove LIC_FILES_CHKSUM from recipes without SRC_URI (2016-10-28 11:27:33 +0100)

are available in the git repository at:

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

Paul Eggleton (7):
  classes/license: fix handling of symlinks pointed to in LIC_FILES_CHKSUM
  devtool: add: show recipetool create output
  devtool: finish: fix error if destination layer is not in bblayers.conf
  devtool: finish: warn if moving recipe to unconfigured destination layer
  recipetool: create: separate LICENSE items with & by default
  oe-selftest: devtool: fix error message in _test_recipe_contents()
  devtool: add "rename" subcommand

 meta/classes/license.bbclass      |   2 +
 meta/lib/oeqa/selftest/devtool.py |  67 +++++++++++-
 scripts/lib/devtool/build.py      |   2 +-
 scripts/lib/devtool/standard.py   | 216 +++++++++++++++++++++++++++++++++++++-
 scripts/lib/recipetool/create.py  |  37 +++++--
 5 files changed, 307 insertions(+), 17 deletions(-)

-- 
2.5.5

[PATCH 1/7] classes/license: fix handling of symlinks pointed to in LIC_FILES_CHKSUM

[Paul Eggleton]

If you set LIC_FILES_CHKSUM to point to a relative symlink then you'll
get "Could not copy license file" warnings in copy_license_files() since
the symlink won't be valid after it's copied. If the source is a symlink
then we need to dereference it first.

I encountered this when I used recipetool on the sources for capnproto,
where the c++ directory contains a LICENSE.txt symlink to the LICENSE
file in the parent directory, and this symlink ends up being pointed to
in LIC_FILES_CHKSUM.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/classes/license.bbclass | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index da4fc3e..660b85f 100644
--- a/meta/classes/license.bbclass
+++ b/meta/classes/license.bbclass
@@ -351,6 +351,8 @@ def copy_license_files(lic_files_paths, destdir):
             dst = os.path.join(destdir, basename)
             if os.path.exists(dst):
                 os.remove(dst)
+            if os.path.islink(src):
+                src = os.path.realpath(src)
             canlink = os.access(src, os.W_OK) and (os.stat(src).st_dev == os.stat(destdir).st_dev)
             if canlink:
                 try:
-- 
2.5.5

Re: [PATCH 1/7] classes/license: fix handling of symlinks pointed to in LIC_FILES_CHKSUM

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

On Mon, Oct 31, 2016 at 04:59:43PM +1300, Paul Eggleton wrote:
> If you set LIC_FILES_CHKSUM to point to a relative symlink then you'll
> get "Could not copy license file" warnings in copy_license_files() since
> the symlink won't be valid after it's copied. If the source is a symlink
> then we need to dereference it first.
> 
> I encountered this when I used recipetool on the sources for capnproto,
> where the c++ directory contains a LICENSE.txt symlink to the LICENSE
> file in the parent directory, and this symlink ends up being pointed to
> in LIC_FILES_CHKSUM.

Acked-by: Martin Jansa <Martin.Jansa@gmail.com>

Please cherry-pick this to morty as well.

> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> ---
>  meta/classes/license.bbclass | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
> index da4fc3e..660b85f 100644
> --- a/meta/classes/license.bbclass
> +++ b/meta/classes/license.bbclass
> @@ -351,6 +351,8 @@ def copy_license_files(lic_files_paths, destdir):
>              dst = os.path.join(destdir, basename)
>              if os.path.exists(dst):
>                  os.remove(dst)
> +            if os.path.islink(src):
> +                src = os.path.realpath(src)
>              canlink = os.access(src, os.W_OK) and (os.stat(src).st_dev == os.stat(destdir).st_dev)
>              if canlink:
>                  try:
> -- 
> 2.5.5
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 169 bytes --]

Re: [PATCH 1/7] classes/license: fix handling of symlinks pointed to in LIC_FILES_CHKSUM

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]

Is this sstate safe?

On Oct 30, 2016 9:11 PM, "Paul Eggleton" <paul.eggleton@linux.intel.com>
wrote:

> If you set LIC_FILES_CHKSUM to point to a relative symlink then you'll
> get "Could not copy license file" warnings in copy_license_files() since
> the symlink won't be valid after it's copied. If the source is a symlink
> then we need to dereference it first.
>
> I encountered this when I used recipetool on the sources for capnproto,
> where the c++ directory contains a LICENSE.txt symlink to the LICENSE
> file in the parent directory, and this symlink ends up being pointed to
> in LIC_FILES_CHKSUM.
>
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> ---
>  meta/classes/license.bbclass | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
> index da4fc3e..660b85f 100644
> --- a/meta/classes/license.bbclass
> +++ b/meta/classes/license.bbclass
> @@ -351,6 +351,8 @@ def copy_license_files(lic_files_paths, destdir):
>              dst = os.path.join(destdir, basename)
>              if os.path.exists(dst):
>                  os.remove(dst)
> +            if os.path.islink(src):
> +                src = os.path.realpath(src)
>              canlink = os.access(src, os.W_OK) and (os.stat(src).st_dev ==
> os.stat(destdir).st_dev)
>              if canlink:
>                  try:
> --
> 2.5.5
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>

[-- Attachment #2: Type: text/html, Size: 2334 bytes --]

Re: [PATCH 1/7] classes/license: fix handling of symlinks pointed to in LIC_FILES_CHKSUM

[Paul Eggleton]

Hi Khem,

Why wouldn't it be?

Cheers,
Paul

On Mon, 31 Oct 2016 12:23:04 Khem Raj wrote:
> Is this sstate safe?
> 
> On Oct 30, 2016 9:11 PM, "Paul Eggleton" <paul.eggleton@linux.intel.com>
> 
> wrote:
> > If you set LIC_FILES_CHKSUM to point to a relative symlink then you'll
> > get "Could not copy license file" warnings in copy_license_files() since
> > the symlink won't be valid after it's copied. If the source is a symlink
> > then we need to dereference it first.
> > 
> > I encountered this when I used recipetool on the sources for capnproto,
> > where the c++ directory contains a LICENSE.txt symlink to the LICENSE
> > file in the parent directory, and this symlink ends up being pointed to
> > in LIC_FILES_CHKSUM.
> > 
> > Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> > ---
> > 
> >  meta/classes/license.bbclass | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
> > index da4fc3e..660b85f 100644
> > --- a/meta/classes/license.bbclass
> > +++ b/meta/classes/license.bbclass
> > 
> > @@ -351,6 +351,8 @@ def copy_license_files(lic_files_paths, destdir):
> >              dst = os.path.join(destdir, basename)
> >              
> >              if os.path.exists(dst):
> >                  os.remove(dst)
> > 
> > +            if os.path.islink(src):
> > +                src = os.path.realpath(src)
> > 
> >              canlink = os.access(src, os.W_OK) and (os.stat(src).st_dev ==
> > 
> > os.stat(destdir).st_dev)
> > 
> >              if canlink:
> >                  try:
> > --
> > 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 1/7] classes/license: fix handling of symlinks pointed to in LIC_FILES_CHKSUM

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 2157 bytes --]


> On Oct 31, 2016, at 12:41 PM, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:
> 
> Hi Khem,
> 
> Why wouldn't it be?

Was thinking out loud if the path is expanded to absolute paths then
does it get recorded into task checksums  ?

> 
> Cheers,
> Paul
> 
> On Mon, 31 Oct 2016 12:23:04 Khem Raj wrote:
>> Is this sstate safe?
>> 
>> On Oct 30, 2016 9:11 PM, "Paul Eggleton" <paul.eggleton@linux.intel.com>
>> 
>> wrote:
>>> If you set LIC_FILES_CHKSUM to point to a relative symlink then you'll
>>> get "Could not copy license file" warnings in copy_license_files() since
>>> the symlink won't be valid after it's copied. If the source is a symlink
>>> then we need to dereference it first.
>>> 
>>> I encountered this when I used recipetool on the sources for capnproto,
>>> where the c++ directory contains a LICENSE.txt symlink to the LICENSE
>>> file in the parent directory, and this symlink ends up being pointed to
>>> in LIC_FILES_CHKSUM.
>>> 
>>> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
>>> ---
>>> 
>>> meta/classes/license.bbclass | 2 ++
>>> 1 file changed, 2 insertions(+)
>>> 
>>> diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
>>> index da4fc3e..660b85f 100644
>>> --- a/meta/classes/license.bbclass
>>> +++ b/meta/classes/license.bbclass
>>> 
>>> @@ -351,6 +351,8 @@ def copy_license_files(lic_files_paths, destdir):
>>>             dst = os.path.join(destdir, basename)
>>> 
>>>             if os.path.exists(dst):
>>>                 os.remove(dst)
>>> 
>>> +            if os.path.islink(src):
>>> +                src = os.path.realpath(src)
>>> 
>>>             canlink = os.access(src, os.W_OK) and (os.stat(src).st_dev ==
>>> 
>>> os.stat(destdir).st_dev)
>>> 
>>>             if canlink:
>>>                 try:
>>> --
>>> 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


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: [PATCH 1/7] classes/license: fix handling of symlinks pointed to in LIC_FILES_CHKSUM

[Christopher Larson]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Mon, Oct 31, 2016 at 3:34 PM, Khem Raj <raj.khem@gmail.com> wrote:

> > On Oct 31, 2016, at 12:41 PM, Paul Eggleton <
> paul.eggleton@linux.intel.com> wrote:
> >
> > Hi Khem,
> >
> > Why wouldn't it be?
>
> Was thinking out loud if the path is expanded to absolute paths then
> does it get recorded into task checksums  ?


It’s resolved in the copy_license_files function, not at parse time. Of
course, any functions calling copy_license_files will change checksums due
to the function itself changing, presumably.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 1176 bytes --]

[PATCH 2/7] devtool: add: show recipetool create output

[Paul Eggleton]

When running devtool add, instead of hiding the recipetool create
output, change it so that it's appropriate to show in the devtool
context and show it in real-time. This means that you get status output
such as when a URL is being fetched (though currently no progress
information.) recipetool create now has a hidden --devtool option to
enable this display mode.

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

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 4eff6f8..8dfd538 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -155,7 +155,7 @@ def add(args, config, basepath, workspace):
     try:
         while True:
             try:
-                stdout, _ = exec_build_env_command(config.init_path, basepath, 'recipetool --color=%s create -o %s "%s" %s' % (color, tempdir, source, extracmdopts))
+                stdout, _ = exec_build_env_command(config.init_path, basepath, 'recipetool --color=%s create --devtool -o %s \'%s\' %s' % (color, tempdir, source, extracmdopts), watch=True)
             except bb.process.ExecutionError as e:
                 if e.exitcode == 14:
                     # FIXME this is a horrible hack that is unfortunately
@@ -164,11 +164,12 @@ def add(args, config, basepath, workspace):
                     # with references to it throughout the code, so we have
                     # to exit out and come back here to do it.
                     ensure_npm(config, basepath, args.fixed_setup)
+                    logger.info('Re-running recipe creation process after building nodejs')
                     continue
                 elif e.exitcode == 15:
                     raise DevtoolError('Could not auto-determine recipe name, please specify it on the command line')
                 else:
-                    raise DevtoolError('Command \'%s\' failed:\n%s' % (e.command, e.stdout))
+                    raise DevtoolError('Command \'%s\' failed' % e.command)
             break
 
         recipes = glob.glob(os.path.join(tempdir, '*.bb'))
diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index d427d32..c1819b6 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -32,6 +32,18 @@ logger = logging.getLogger('recipetool')
 tinfoil = None
 plugins = None
 
+def log_error_cond(message, debugonly):
+    if debugonly:
+        logger.debug(message)
+    else:
+        logger.error(message)
+
+def log_info_cond(message, debugonly):
+    if debugonly:
+        logger.debug(message)
+    else:
+        logger.info(message)
+
 def plugin_init(pluginlist):
     # Take a reference to the list so we can use it later
     global plugins
@@ -406,7 +418,7 @@ def create_recipe(args):
         srctree = tempsrc
         if fetchuri.startswith('npm://'):
             # Check if npm is available
-            check_npm(tinfoil.config_data)
+            check_npm(tinfoil.config_data, args.devtool)
         logger.info('Fetching %s...' % srcuri)
         try:
             checksums = scriptutils.fetch_uri(tinfoil.config_data, fetchuri, srctree, srcrev)
@@ -640,7 +652,7 @@ def create_recipe(args):
 
     if not outfile:
         if not pn:
-            logger.error('Unable to determine short program name from source tree - please specify name with -N/--name or output file name with -o/--outfile')
+            log_error_cond('Unable to determine short program name from source tree - please specify name with -N/--name or output file name with -o/--outfile', args.devtool)
             # devtool looks for this specific exit code, so don't change it
             sys.exit(15)
         else:
@@ -736,7 +748,7 @@ def create_recipe(args):
         shutil.move(srctree, args.extract_to)
         if tempsrc == srctree:
             tempsrc = None
-        logger.info('Source extracted to %s' % args.extract_to)
+        log_info_cond('Source extracted to %s' % args.extract_to, args.devtool)
 
     if outfile == '-':
         sys.stdout.write('\n'.join(outlines) + '\n')
@@ -749,7 +761,7 @@ def create_recipe(args):
                     continue
                 f.write('%s\n' % line)
                 lastline = line
-        logger.info('Recipe %s has been created; further editing may be required to make it fully functional' % outfile)
+        log_info_cond('Recipe %s has been created; further editing may be required to make it fully functional' % outfile, args.devtool)
 
     if tempsrc:
         if args.keep_temp:
@@ -1073,9 +1085,9 @@ def convert_rpm_xml(xmlfile):
     return values
 
 
-def check_npm(d):
+def check_npm(d, debugonly=False):
     if not os.path.exists(os.path.join(d.getVar('STAGING_BINDIR_NATIVE', True), 'npm')):
-        logger.error('npm required to process specified source, but npm is not available - you need to build nodejs-native first')
+        log_error_cond('npm required to process specified source, but npm is not available - you need to build nodejs-native first', debugonly)
         sys.exit(14)
 
 def register_commands(subparsers):
@@ -1093,5 +1105,6 @@ def register_commands(subparsers):
     parser_create.add_argument('--src-subdir', help='Specify subdirectory within source tree to use', metavar='SUBDIR')
     parser_create.add_argument('-a', '--autorev', help='When fetching from a git repository, set SRCREV in the recipe to a floating revision instead of fixed', action="store_true")
     parser_create.add_argument('--keep-temp', action="store_true", help='Keep temporary directory (for debugging)')
+    parser_create.add_argument('--devtool', action="store_true", help=argparse.SUPPRESS)
     parser_create.set_defaults(func=create_recipe)
 
-- 
2.5.5

[PATCH 3/7] devtool: finish: fix error if destination layer is not in bblayers.conf

[Paul Eggleton]

If devtool finish is run on a recipe where the recipe file itself is in
the workspace (e.g. where devtool add / devtool upgrade has been used)
and the specified destination layer is not in bblayers.conf, then we
need to avoid running bitbake -c clean at the end because the recipe has
been moved, but the bbappend is still present in the workspace layer at
that point and so if we do it will fail due to the dangling bbappend.
It's difficult to do the clean at the point we'd want to because tinfoil
is holding bitbake.lock for most of the time, but in any case cleaning
the recipe is less important than it used to be since we started
managing the sysroot contents more strictly, so just disable cleaning
under these circumstances to avoid the problem.

Fixes [YOCTO #10484].

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

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 8dfd538..9511d56 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -1479,6 +1479,7 @@ def finish(args, config, basepath, workspace):
 
     check_workspace_recipe(workspace, args.recipename)
 
+    no_clean = False
     tinfoil = setup_tinfoil(basepath=basepath, tracking=True)
     try:
         rd = parse_recipe(config, tinfoil, args.recipename, True)
@@ -1540,6 +1541,7 @@ def finish(args, config, basepath, workspace):
         if origlayerdir == config.workspace_path and destpath:
             # Recipe file itself is in the workspace - need to move it and any
             # associated files to the specified layer
+            no_clean = True
             logger.info('Moving recipe file to %s' % destpath)
             recipedir = os.path.dirname(rd.getVar('FILE', True))
             for root, _, files in os.walk(recipedir):
@@ -1554,7 +1556,7 @@ def finish(args, config, basepath, workspace):
         tinfoil.shutdown()
 
     # Everything else has succeeded, we can now reset
-    _reset([args.recipename], no_clean=False, config=config, basepath=basepath, workspace=workspace)
+    _reset([args.recipename], no_clean=no_clean, config=config, basepath=basepath, workspace=workspace)
 
     return 0
 
-- 
2.5.5

[PATCH 4/7] devtool: finish: warn if moving recipe to unconfigured destination layer

[Paul Eggleton]

If you run devtool finish to move a recipe created in the workspace by
devtool add or devtool upgrade to a layer, and that layer is not
currently included in bblayers.conf (perhaps unintentionally), then the
recipe will no longer be visible to bitbake. In this scenario, show a
warning so that the user isn't surprised by the recipe "going missing".

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

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 9511d56..c15bfef 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -1517,6 +1517,11 @@ def finish(args, config, basepath, workspace):
             destpath = oe.recipeutils.get_bbfile_path(rd, destlayerdir, origrelpath)
             if not destpath:
                 raise DevtoolError("Unable to determine destination layer path - check that %s specifies an actual layer and %s/conf/layer.conf specifies BBFILES. You may also need to specify a more complete path." % (args.destination, destlayerdir))
+            # Warn if the layer isn't in bblayers.conf (the code to create a bbappend will do this in other cases)
+            layerdirs = [os.path.abspath(layerdir) for layerdir in rd.getVar('BBLAYERS', True).split()]
+            if not os.path.abspath(destlayerdir) in layerdirs:
+                bb.warn('Specified destination layer is not currently enabled in bblayers.conf, so the %s recipe will now be unavailable in your current configuration until you add the layer there' % args.recipename)
+
         elif destlayerdir == origlayerdir:
             # Same layer, update the original recipe
             appendlayerdir = None
-- 
2.5.5

[PATCH 5/7] recipetool: create: separate LICENSE items with & by default

[Paul Eggleton]

recipetool sets the LICENSE value based on licenses detected from the
source tree. If there are multiple licenses then they were being
separated by spaces, but this isn't actually legal formatting and if
you're using "devtool add" you get a warning printed when devtool
parses the recipe internally.

Earlier I had made a conscious decision to do it this way since it's up
to the user to figure out whether the multiple licenses should all apply
(in which case they'd be separated with &) or if there is a choice of
license (in which case | is the correct separator). However, I've come
to the conclusion that we can just default to & and then the ugly
warning goes away, and it's the safest alternative of the two (and most
likely to be correct, since it's more common to have a codebase which is
made up of code with different licenses, i.e. all of them apply to the
combined work).

I've tweaked the comment that we add to the recipe to explicitly state
that we've used & and that the user needs to change that if that's not
accurate.

Fixes [YOCTO #10413].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oeqa/selftest/devtool.py |  5 ++++-
 scripts/lib/recipetool/create.py  | 12 +++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/meta/lib/oeqa/selftest/devtool.py b/meta/lib/oeqa/selftest/devtool.py
index e992dcf..713f2c5 100644
--- a/meta/lib/oeqa/selftest/devtool.py
+++ b/meta/lib/oeqa/selftest/devtool.py
@@ -46,7 +46,10 @@ class DevtoolBase(oeSelfTest):
                     if needvalue is None:
                         self.fail('Variable %s should not appear in recipe')
                     if isinstance(needvalue, set):
-                        value = set(value.split())
+                        if var == 'LICENSE':
+                            value = set(value.split(' & '))
+                        else:
+                            value = set(value.split())
                     self.assertEqual(value, needvalue, 'values for %s do not match' % var)
 
 
diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index c1819b6..cb1c804 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -787,10 +787,12 @@ def handle_license_vars(srctree, lines_before, handled, extravalues, d):
         lines_before.append('# your responsibility to verify that the values are complete and correct.')
         if len(licvalues) > 1:
             lines_before.append('#')
-            lines_before.append('# NOTE: multiple licenses have been detected; if that is correct you should separate')
-            lines_before.append('# these in the LICENSE value using & if the multiple licenses all apply, or | if there')
-            lines_before.append('# is a choice between the multiple licenses. If in doubt, check the accompanying')
-            lines_before.append('# documentation to determine which situation is applicable.')
+            lines_before.append('# NOTE: multiple licenses have been detected; they have been separated with &')
+            lines_before.append('# in the LICENSE value for now since it is a reasonable assumption that all')
+            lines_before.append('# of the licenses apply. If instead there is a choice between the multiple')
+            lines_before.append('# licenses then you should change the value to separate the licenses with |')
+            lines_before.append('# instead of &. If there is any doubt, check the accompanying documentation')
+            lines_before.append('# to determine which situation is applicable.')
         if lic_unknown:
             lines_before.append('#')
             lines_before.append('# The following license files were not able to be identified and are')
@@ -814,7 +816,7 @@ def handle_license_vars(srctree, lines_before, handled, extravalues, d):
             licenses = [pkg_license]
         else:
             lines_before.append('# NOTE: Original package metadata indicates license is: %s' % pkg_license)
-    lines_before.append('LICENSE = "%s"' % ' '.join(licenses))
+    lines_before.append('LICENSE = "%s"' % ' & '.join(licenses))
     lines_before.append('LIC_FILES_CHKSUM = "%s"' % ' \\\n                    '.join(lic_files_chksum))
     lines_before.append('')
     handled.append(('license', licvalues))
-- 
2.5.5

[PATCH 6/7] oe-selftest: devtool: fix error message in _test_recipe_contents()

[Paul Eggleton]

If a variable is being set in the recipe when we've explicitly passed
None as the value to _test_recipe_contents() indicating that it
shouldn't be set at all, then we should be printing out the variable
name in the assertion message but it seems like I forgot to do a
substitution. Also include the value for informational purposes.

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

diff --git a/meta/lib/oeqa/selftest/devtool.py b/meta/lib/oeqa/selftest/devtool.py
index 713f2c5..71d205c 100644
--- a/meta/lib/oeqa/selftest/devtool.py
+++ b/meta/lib/oeqa/selftest/devtool.py
@@ -44,7 +44,7 @@ class DevtoolBase(oeSelfTest):
                 if var and var in checkvars:
                     needvalue = checkvars.pop(var)
                     if needvalue is None:
-                        self.fail('Variable %s should not appear in recipe')
+                        self.fail('Variable %s should not appear in recipe, but value is being set to "%s"' % (var, value))
                     if isinstance(needvalue, set):
                         if var == 'LICENSE':
                             value = set(value.split(' & '))
-- 
2.5.5

[PATCH 7/7] devtool: add "rename" subcommand

[Paul Eggleton]

When you run devtool add on a source tree we attempt to figure out the
correct name and version for the recipe. However, despite our best
efforts, sometimes the name and/or version we come up with isn't
correct, and the only way to remedy that up until now was to reset the
recipe, delete the source tree and start again, specifying the name this
time. To avoid this slightly painful procedure, add a "rename"
subcommand that lets you rename the recipe and/or change the version.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oeqa/selftest/devtool.py |  60 +++++++++++
 scripts/lib/devtool/build.py      |   2 +-
 scripts/lib/devtool/standard.py   | 202 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oeqa/selftest/devtool.py b/meta/lib/oeqa/selftest/devtool.py
index 71d205c..46f5a0b 100644
--- a/meta/lib/oeqa/selftest/devtool.py
+++ b/meta/lib/oeqa/selftest/devtool.py
@@ -1349,3 +1349,63 @@ class DevtoolTests(DevtoolBase):
         files.remove(foundpatch)
         if files:
             self.fail('Unexpected file(s) copied next to bbappend: %s' % ', '.join(files))
+
+    def test_devtool_rename(self):
+        # Check preconditions
+        self.assertTrue(not os.path.exists(self.workspacedir), 'This test cannot be run with a workspace directory under the build directory')
+        self.track_for_cleanup(self.workspacedir)
+        self.add_command_to_tearDown('bitbake-layers remove-layer */workspace')
+
+        # First run devtool add
+        # We already have this recipe in OE-Core, but that doesn't matter
+        recipename = 'i2c-tools'
+        recipever = '3.1.2'
+        recipefile = os.path.join(self.workspacedir, 'recipes', recipename, '%s_%s.bb' % (recipename, recipever))
+        url = 'http://downloads.yoctoproject.org/mirror/sources/i2c-tools-%s.tar.bz2' % recipever
+        def add_recipe():
+            result = runCmd('devtool add %s' % url)
+            self.assertTrue(os.path.exists(recipefile), 'Expected recipe file not created')
+            self.assertTrue(os.path.exists(os.path.join(self.workspacedir, 'sources', recipename)), 'Source directory not created')
+            checkvars = {}
+            checkvars['S'] = None
+            checkvars['SRC_URI'] = url.replace(recipever, '${PV}')
+            self._test_recipe_contents(recipefile, checkvars, [])
+        add_recipe()
+        # Now rename it - change both name and version
+        newrecipename = 'mynewrecipe'
+        newrecipever = '456'
+        newrecipefile = os.path.join(self.workspacedir, 'recipes', newrecipename, '%s_%s.bb' % (newrecipename, newrecipever))
+        result = runCmd('devtool rename %s %s -V %s' % (recipename, newrecipename, newrecipever))
+        self.assertTrue(os.path.exists(newrecipefile), 'Recipe file not renamed')
+        self.assertFalse(os.path.exists(os.path.join(self.workspacedir, 'recipes', recipename)), 'Old recipe directory still exists')
+        newsrctree = os.path.join(self.workspacedir, 'sources', newrecipename)
+        self.assertTrue(os.path.exists(newsrctree), 'Source directory not renamed')
+        checkvars = {}
+        checkvars['S'] = '${WORKDIR}/%s-%s' % (recipename, recipever)
+        checkvars['SRC_URI'] = url
+        self._test_recipe_contents(newrecipefile, checkvars, [])
+        # Try again - change just name this time
+        result = runCmd('devtool reset -n %s' % newrecipename)
+        shutil.rmtree(newsrctree)
+        add_recipe()
+        newrecipefile = os.path.join(self.workspacedir, 'recipes', newrecipename, '%s_%s.bb' % (newrecipename, recipever))
+        result = runCmd('devtool rename %s %s' % (recipename, newrecipename))
+        self.assertTrue(os.path.exists(newrecipefile), 'Recipe file not renamed')
+        self.assertFalse(os.path.exists(os.path.join(self.workspacedir, 'recipes', recipename)), 'Old recipe directory still exists')
+        self.assertTrue(os.path.exists(os.path.join(self.workspacedir, 'sources', newrecipename)), 'Source directory not renamed')
+        checkvars = {}
+        checkvars['S'] = '${WORKDIR}/%s-${PV}' % recipename
+        checkvars['SRC_URI'] = url.replace(recipever, '${PV}')
+        self._test_recipe_contents(newrecipefile, checkvars, [])
+        # Try again - change just version this time
+        result = runCmd('devtool reset -n %s' % newrecipename)
+        shutil.rmtree(newsrctree)
+        add_recipe()
+        newrecipefile = os.path.join(self.workspacedir, 'recipes', recipename, '%s_%s.bb' % (recipename, newrecipever))
+        result = runCmd('devtool rename %s -V %s' % (recipename, newrecipever))
+        self.assertTrue(os.path.exists(newrecipefile), 'Recipe file not renamed')
+        self.assertTrue(os.path.exists(os.path.join(self.workspacedir, 'sources', recipename)), 'Source directory no longer exists')
+        checkvars = {}
+        checkvars['S'] = '${WORKDIR}/${BPN}-%s' % recipever
+        checkvars['SRC_URI'] = url
+        self._test_recipe_contents(newrecipefile, checkvars, [])
diff --git a/scripts/lib/devtool/build.py b/scripts/lib/devtool/build.py
index 6be549d..252379e 100644
--- a/scripts/lib/devtool/build.py
+++ b/scripts/lib/devtool/build.py
@@ -80,7 +80,7 @@ def register_commands(subparsers, context):
     """Register devtool subcommands from this plugin"""
     parser_build = subparsers.add_parser('build', help='Build a recipe',
                                          description='Builds the specified recipe using bitbake (up to and including %s)' % ', '.join(_get_build_tasks(context.config)),
-                                         group='working')
+                                         group='working', order=50)
     parser_build.add_argument('recipename', help='Recipe to build')
     parser_build.add_argument('-s', '--disable-parallel-make', action="store_true", help='Disable make parallelism')
     parser_build.set_defaults(func=build)
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index c15bfef..4523048 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -852,6 +852,199 @@ def modify(args, config, basepath, workspace):
 
     return 0
 
+
+def rename(args, config, basepath, workspace):
+    """Entry point for the devtool 'rename' subcommand"""
+    import bb
+    import oe.recipeutils
+
+    check_workspace_recipe(workspace, args.recipename)
+
+    if not (args.newname or args.version):
+        raise DevtoolError('You must specify a new name, a version with -V/--version, or both')
+
+    recipefile = workspace[args.recipename]['recipefile']
+    if not recipefile:
+        raise DevtoolError('devtool rename can only be used where the recipe file itself is in the workspace (e.g. after devtool add)')
+
+    if args.newname and args.newname != args.recipename:
+        reason = oe.recipeutils.validate_pn(args.newname)
+        if reason:
+            raise DevtoolError(reason)
+        newname = args.newname
+    else:
+        newname = args.recipename
+
+    append = workspace[args.recipename]['bbappend']
+    appendfn = os.path.splitext(os.path.basename(append))[0]
+    splitfn = appendfn.split('_')
+    if len(splitfn) > 1:
+        origfnver = appendfn.split('_')[1]
+    else:
+        origfnver = ''
+
+    recipefilemd5 = None
+    tinfoil = setup_tinfoil(basepath=basepath, tracking=True)
+    try:
+        rd = parse_recipe(config, tinfoil, args.recipename, True)
+        if not rd:
+            return 1
+
+        bp = rd.getVar('BP', True)
+        bpn = rd.getVar('BPN', True)
+        if newname != args.recipename:
+            localdata = rd.createCopy()
+            localdata.setVar('PN', newname)
+            newbpn = localdata.getVar('BPN', True)
+        else:
+            newbpn = bpn
+        s = rd.getVar('S', False)
+        src_uri = rd.getVar('SRC_URI', False)
+        pv = rd.getVar('PV', True)
+
+        # Correct variable values that refer to the upstream source - these
+        # values must stay the same, so if the name/version are changing then
+        # we need to fix them up
+        new_s = s
+        new_src_uri = src_uri
+        if newbpn != bpn:
+            # ${PN} here is technically almost always incorrect, but people do use it
+            new_s = new_s.replace('${BPN}', bpn)
+            new_s = new_s.replace('${PN}', bpn)
+            new_s = new_s.replace('${BP}', '%s-${PV}' % bpn)
+            new_src_uri = new_src_uri.replace('${BPN}', bpn)
+            new_src_uri = new_src_uri.replace('${PN}', bpn)
+            new_src_uri = new_src_uri.replace('${BP}', '%s-${PV}' % bpn)
+        if args.version and origfnver == pv:
+            new_s = new_s.replace('${PV}', pv)
+            new_s = new_s.replace('${BP}', '${BPN}-%s' % pv)
+            new_src_uri = new_src_uri.replace('${PV}', pv)
+            new_src_uri = new_src_uri.replace('${BP}', '${BPN}-%s' % pv)
+        patchfields = {}
+        if new_s != s:
+            patchfields['S'] = new_s
+        if new_src_uri != src_uri:
+            patchfields['SRC_URI'] = new_src_uri
+        if patchfields:
+            recipefilemd5 = bb.utils.md5_file(recipefile)
+            oe.recipeutils.patch_recipe(rd, recipefile, patchfields)
+            newrecipefilemd5 = bb.utils.md5_file(recipefile)
+    finally:
+        tinfoil.shutdown()
+
+    if args.version:
+        newver = args.version
+    else:
+        newver = origfnver
+
+    if newver:
+        newappend = '%s_%s.bbappend' % (newname, newver)
+        newfile =  '%s_%s.bb' % (newname, newver)
+    else:
+        newappend = '%s.bbappend' % newname
+        newfile = '%s.bb' % newname
+
+    oldrecipedir = os.path.dirname(recipefile)
+    newrecipedir = os.path.join(config.workspace_path, 'recipes', newname)
+    if oldrecipedir != newrecipedir:
+        bb.utils.mkdirhier(newrecipedir)
+
+    newappend = os.path.join(os.path.dirname(append), newappend)
+    newfile = os.path.join(newrecipedir, newfile)
+
+    # Rename bbappend
+    logger.info('Renaming %s to %s' % (append, newappend))
+    os.rename(append, newappend)
+    # Rename recipe file
+    logger.info('Renaming %s to %s' % (recipefile, newfile))
+    os.rename(recipefile, newfile)
+
+    # Rename source tree if it's the default path
+    appendmd5 = None
+    if not args.no_srctree:
+        srctree = workspace[args.recipename]['srctree']
+        if os.path.abspath(srctree) == os.path.join(config.workspace_path, 'sources', args.recipename):
+            newsrctree = os.path.join(config.workspace_path, 'sources', newname)
+            logger.info('Renaming %s to %s' % (srctree, newsrctree))
+            shutil.move(srctree, newsrctree)
+            # Correct any references (basically EXTERNALSRC*) in the .bbappend
+            appendmd5 = bb.utils.md5_file(newappend)
+            appendlines = []
+            with open(newappend, 'r') as f:
+                for line in f:
+                    appendlines.append(line)
+            with open(newappend, 'w') as f:
+                for line in appendlines:
+                    if srctree in line:
+                        line = line.replace(srctree, newsrctree)
+                    f.write(line)
+            newappendmd5 = bb.utils.md5_file(newappend)
+
+    bpndir = None
+    newbpndir = None
+    if newbpn != bpn:
+        bpndir = os.path.join(oldrecipedir, bpn)
+        if os.path.exists(bpndir):
+            newbpndir = os.path.join(newrecipedir, newbpn)
+            logger.info('Renaming %s to %s' % (bpndir, newbpndir))
+            shutil.move(bpndir, newbpndir)
+
+    bpdir = None
+    newbpdir = None
+    if newver != origfnver or newbpn != bpn:
+        bpdir = os.path.join(oldrecipedir, bp)
+        if os.path.exists(bpdir):
+            newbpdir = os.path.join(newrecipedir, '%s-%s' % (newbpn, newver))
+            logger.info('Renaming %s to %s' % (bpdir, newbpdir))
+            shutil.move(bpdir, newbpdir)
+
+    if oldrecipedir != newrecipedir:
+        # Move any stray files and delete the old recipe directory
+        for entry in os.listdir(oldrecipedir):
+            oldpath = os.path.join(oldrecipedir, entry)
+            newpath = os.path.join(newrecipedir, entry)
+            logger.info('Renaming %s to %s' % (oldpath, newpath))
+            shutil.move(oldpath, newpath)
+        os.rmdir(oldrecipedir)
+
+    # Now take care of entries in .devtool_md5
+    md5entries = []
+    with open(os.path.join(config.workspace_path, '.devtool_md5'), 'r') as f:
+        for line in f:
+            md5entries.append(line)
+
+    if bpndir and newbpndir:
+        relbpndir = os.path.relpath(bpndir, config.workspace_path) + '/'
+    else:
+        relbpndir = None
+    if bpdir and newbpdir:
+        relbpdir = os.path.relpath(bpdir, config.workspace_path) + '/'
+    else:
+        relbpdir = None
+
+    with open(os.path.join(config.workspace_path, '.devtool_md5'), 'w') as f:
+        for entry in md5entries:
+            splitentry = entry.rstrip().split('|')
+            if len(splitentry) > 2:
+                if splitentry[0] == args.recipename:
+                    splitentry[0] = newname
+                    if splitentry[1] == os.path.relpath(append, config.workspace_path):
+                        splitentry[1] = os.path.relpath(newappend, config.workspace_path)
+                        if appendmd5 and splitentry[2] == appendmd5:
+                            splitentry[2] = newappendmd5
+                    elif splitentry[1] == os.path.relpath(recipefile, config.workspace_path):
+                        splitentry[1] = os.path.relpath(newfile, config.workspace_path)
+                        if recipefilemd5 and splitentry[2] == recipefilemd5:
+                            splitentry[2] = newrecipefilemd5
+                    elif relbpndir and splitentry[1].startswith(relbpndir):
+                        splitentry[1] = os.path.relpath(os.path.join(newbpndir, splitentry[1][len(relbpndir):]), config.workspace_path)
+                    elif relbpdir and splitentry[1].startswith(relbpdir):
+                        splitentry[1] = os.path.relpath(os.path.join(newbpdir, splitentry[1][len(relbpdir):]), config.workspace_path)
+                    entry = '|'.join(splitentry) + '\n'
+            f.write(entry)
+    return 0
+
+
 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.
@@ -1630,6 +1823,15 @@ def register_commands(subparsers, context):
     parser_sync.add_argument('--keep-temp', action="store_true", help='Keep temporary directory (for debugging)')
     parser_sync.set_defaults(func=sync)
 
+    parser_rename = subparsers.add_parser('rename', help='Rename a recipe file in the workspace',
+                                       description='Renames the recipe file for a recipe in the workspace, changing the name or version part or both, ensuring that all references within the workspace are updated at the same time. Only works when the recipe file itself is in the workspace, e.g. after devtool add. Particularly useful when devtool add did not automatically determine the correct name.',
+                                       group='working', order=10)
+    parser_rename.add_argument('recipename', help='Current name of recipe to rename')
+    parser_rename.add_argument('newname', nargs='?', help='New name for recipe (optional, not needed if you only want to change the version)')
+    parser_rename.add_argument('--version', '-V', help='Change the version (NOTE: this does not change the version fetched by the recipe, just the version in the recipe file name)')
+    parser_rename.add_argument('--no-srctree', '-s', action='store_true', help='Do not rename the source tree directory (if the default source tree path has been used) - keeping the old name may be desirable if there are internal/other external references to this path')
+    parser_rename.set_defaults(func=rename)
+
     parser_update_recipe = subparsers.add_parser('update-recipe', help='Apply changes from external source tree to recipe',
                                        description='Applies changes from external source tree to a recipe (updating/adding/removing patches as necessary, or by updating SRCREV). Note that these changes need to have been committed to the git repository in order to be recognised.',
                                        group='working', order=-90)
-- 
2.5.5