[PATCH 0/5] devtool: create kernel config fragment

[PATCH 0/5] devtool: create kernel config fragment

[Markus Lehtonen]

This patchset implements an initial support for creating kernel config
fragments on 'devtool update-recipe'. The feature relies on having an updated
.config file and an initial base config (.config.orig) in the source tree.
Devtool does a diff operation between the two and creates/upates a config
fragment which is added to SRC_URI of the recipe.

The first patch in the series is a bugfix and should be straightforward to
merge.

[YOCTO #6658]


The following changes since commit 6f822a9fd185f479ef86c584b6d91a51b3a24e44:

  meta: more removals of redunant FILES_${PN}-dbg (2015-12-16 12:11:26 +0000)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib marquiz/devtool/kernel

for you to fetch changes up to 2adc35e3129be74afe28dc3087ea44665a80221c:

  devtool: update-recipe: create kernel config fragment (2015-12-17 14:34:15 +0200)


Markus Lehtonen (5):
  devtool: extract: use the correct datastore for builddir
  kernel.bbclass: copy .config instead of moving
  devtool: extract: cleanup srctree
  cml1.bbclass: copy .config to S if externalsr is in use
  devtool: update-recipe: create kernel config fragment

 meta/classes/cml1.bbclass       |  5 ++++
 meta/classes/kernel.bbclass     |  2 +-
 scripts/lib/devtool/standard.py | 65 +++++++++++++++++++++++++++++++++++++++--
 3 files changed, 68 insertions(+), 4 deletions(-)

-- 
2.1.4

[PATCH 1/5] devtool: extract: use the correct datastore for builddir

[Markus Lehtonen]

Signed-off-by: Markus Lehtonen <markus.lehtonen@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 a5e81f3..43fce11 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -461,7 +461,7 @@ def _extract_source(srctree, keep_temp, devbranch, sync, d):
             # Store generate and store kernel config
             logger.info('Generating kernel config')
             task_executor.exec_func('do_configure', False)
-            kconfig = os.path.join(d.getVar('B', True), '.config')
+            kconfig = os.path.join(crd.getVar('B', True), '.config')
             shutil.copy2(kconfig, srcsubdir)
 
 
-- 
2.1.4

[PATCH 2/5] kernel.bbclass: copy .config instead of moving

[Markus Lehtonen]

Copy kernel .config from ${S} to ${B}, instead of moving it. This
prevents mangling the source tree, which is undesirable e.g. when
externalsrc is used.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 4ce1611..7de9f20 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -338,7 +338,7 @@ kernel_do_configure() {
 	touch ${B}/.scmversion ${S}/.scmversion
 
 	if [ "${S}" != "${B}" ] && [ -f "${S}/.config" ] && [ ! -f "${B}/.config" ]; then
-		mv "${S}/.config" "${B}/.config"
+		cp "${S}/.config" "${B}/.config"
 	fi
 
 	# Copy defconfig to .config if .config does not exist. This allows
-- 
2.1.4

Re: [PATCH 2/5] kernel.bbclass: copy .config instead of moving

[Richard Purdie]

On Fri, 2015-12-18 at 10:39 +0200, Markus Lehtonen wrote:
> Copy kernel .config from ${S} to ${B}, instead of moving it. This
> prevents mangling the source tree, which is undesirable e.g. when
> externalsrc is used.
> 
> Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
> ---
>  meta/classes/kernel.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/kernel.bbclass
> b/meta/classes/kernel.bbclass
> index 4ce1611..7de9f20 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -338,7 +338,7 @@ kernel_do_configure() {
>  	touch ${B}/.scmversion ${S}/.scmversion
>  
>  	if [ "${S}" != "${B}" ] && [ -f "${S}/.config" ] && [ ! -f
> "${B}/.config" ]; then
> -		mv "${S}/.config" "${B}/.config"
> +		cp "${S}/.config" "${B}/.config"
>  	fi
>  
>  	# Copy defconfig to .config if .config does not exist. This
> allows

I'm not sure about this, doesn't this trigger the kernel to see ${S} as
being 'dirty' and cause other issues when you try and do out of tree
builds with it? 

It also means we have two copies of "config" around which can end up
being different and confuse users no end :(.

Cheers,

Richard

Re: [PATCH 2/5] kernel.bbclass: copy .config instead of moving

[Markus Lehtonen]

Hi Richard,



On 18/12/15 14:22, "Richard Purdie" <richard.purdie@linuxfoundation.org> wrote:

>On Fri, 2015-12-18 at 10:39 +0200, Markus Lehtonen wrote:
>> Copy kernel .config from ${S} to ${B}, instead of moving it. This
>> prevents mangling the source tree, which is undesirable e.g. when
>> externalsrc is used.
>> 
>> Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
>> ---
>>  meta/classes/kernel.bbclass | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/meta/classes/kernel.bbclass
>> b/meta/classes/kernel.bbclass
>> index 4ce1611..7de9f20 100644
>> --- a/meta/classes/kernel.bbclass
>> +++ b/meta/classes/kernel.bbclass
>> @@ -338,7 +338,7 @@ kernel_do_configure() {
>>  	touch ${B}/.scmversion ${S}/.scmversion
>>  
>>  	if [ "${S}" != "${B}" ] && [ -f "${S}/.config" ] && [ ! -f
>> "${B}/.config" ]; then
>> -		mv "${S}/.config" "${B}/.config"
>> +		cp "${S}/.config" "${B}/.config"
>>  	fi
>>  
>>  	# Copy defconfig to .config if .config does not exist. This
>> allows
>
>I'm not sure about this, doesn't this trigger the kernel to see ${S} as
>being 'dirty' and cause other issues when you try and do out of tree
>builds with it?

That shouldn't be a problem as the kernel .gitignore ignores .config (or '.*' to be more specific). There are other tasks that make changes to the kernel source tree, as well, like do_kernel_metadata.


>It also means we have two copies of "config" around which can end up
>being different and confuse users no end :(.

Yes, I must agree. What do you think if ${B}/.config would be a symlink to ${S}/.config? I.e.
+               ln -s "${S}/.config" "${B}/.config"




Thanks,
  Markus

Re: [PATCH 2/5] kernel.bbclass: copy .config instead of moving

[Richard Purdie]

On Fri, 2015-12-18 at 14:39 +0200, Markus Lehtonen wrote:
> On 18/12/15 14:22, "Richard Purdie" <
> richard.purdie@linuxfoundation.org> wrote:
> > On Fri, 2015-12-18 at 10:39 +0200, Markus Lehtonen wrote: 
> > >  	if [ "${S}" != "${B}" ] && [ -f "${S}/.config" ] && [ ! 
> > > -f
> > > "${B}/.config" ]; then
> > > -		mv "${S}/.config" "${B}/.config"
> > > +		cp "${S}/.config" "${B}/.config"
> > >  	fi
> > >  
> > >  	# Copy defconfig to .config if .config does not exist.
> > > This
> > > allows
> > 
> > I'm not sure about this, doesn't this trigger the kernel to see
> > ${S} as
> > being 'dirty' and cause other issues when you try and do out of
> > tree
> > builds with it?
> 
> That shouldn't be a problem as the kernel .gitignore ignores .config
> (or '.*' to be more specific). There are other tasks that make
> changes to the kernel source tree, as well, like do_kernel_metadata.
> 
> 
> > It also means we have two copies of "config" around which can end
> > up
> > being different and confuse users no end :(.
> 
> Yes, I must agree. What do you think if ${B}/.config would be a
> symlink to ${S}/.config? I.e.
> +               ln -s "${S}/.config" "${B}/.config"
> 

I think I'd prefer we move the file over the ${B} and then symlink from
${S} since that way if its modified, its mostly likely to be done from
${B} at least by the automated code?

Cheers,

Richard

Re: [PATCH 2/5] kernel.bbclass: copy .config instead of moving

[Markus Lehtonen]

On 18/12/15 16:18, "Richard Purdie" <richard.purdie@linuxfoundation.org> wrote:

>On Fri, 2015-12-18 at 14:39 +0200, Markus Lehtonen wrote:
>> On 18/12/15 14:22, "Richard Purdie" <
>> richard.purdie@linuxfoundation.org> wrote:
>> > On Fri, 2015-12-18 at 10:39 +0200, Markus Lehtonen wrote: 
>> > >  	if [ "${S}" != "${B}" ] && [ -f "${S}/.config" ] && [ ! 
>> > > -f
>> > > "${B}/.config" ]; then
>> > > -		mv "${S}/.config" "${B}/.config"
>> > > +		cp "${S}/.config" "${B}/.config"
>> > >  	fi
>> > >  
>> > >  	# Copy defconfig to .config if .config does not exist.
>> > > This
>> > > allows
>> > 
>> > I'm not sure about this, doesn't this trigger the kernel to see
>> > ${S} as
>> > being 'dirty' and cause other issues when you try and do out of
>> > tree
>> > builds with it?
>> 
>> That shouldn't be a problem as the kernel .gitignore ignores .config
>> (or '.*' to be more specific). There are other tasks that make
>> changes to the kernel source tree, as well, like do_kernel_metadata.
>> 
>> 
>> > It also means we have two copies of "config" around which can end
>> > up
>> > being different and confuse users no end :(.
>> 
>> Yes, I must agree. What do you think if ${B}/.config would be a
>> symlink to ${S}/.config? I.e.
>> +               ln -s "${S}/.config" "${B}/.config"
>> 
>
>I think I'd prefer we move the file over the ${B} and then symlink from
>${S} since that way if its modified, its mostly likely to be done from
>${B} at least by the automated code?

Hmm, what I suggested is not good (at least on its own) because menuconfig moves .config to .config.old.

Symlinking from ${S} to ${B} is also problematic for the intended devtool use case because removing/cleaning ${B} also removes our .config. Back to the drawing board, I guess...


Thanks,
  Markus

[PATCH 3/5] devtool: extract: cleanup srctree

[Markus Lehtonen]

Some bitbake tasks, notably do_kernel_metadata et al. dirty the
sourcetree. Run git clean in order to get rid of the confusing and
possibly outdated extra files.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 43fce11..f817671 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -458,10 +458,21 @@ def _extract_source(srctree, keep_temp, devbranch, sync, d):
                 bb.process.run('git checkout patches', cwd=srcsubdir)
 
         if bb.data.inherits_class('kernel-yocto', d):
-            # Store generate and store kernel config
             logger.info('Generating kernel config')
             task_executor.exec_func('do_configure', False)
             kconfig = os.path.join(crd.getVar('B', True), '.config')
+        else:
+            kconfig = None
+
+        # Clean source tree in case it has been dirtied by some bitbake task
+        stdout, _ = bb.process.run('git status --porcelain --ignored',
+                                   cwd=srcsubdir)
+        if stdout:
+            logger.info('Source tree is dirty, cleaning up')
+            bb.process.run('git clean -fdx', cwd=srcsubdir)
+
+        if kconfig:
+            # Store kernel config in srctree
             shutil.copy2(kconfig, srcsubdir)
 
 
-- 
2.1.4

[PATCH 4/5] cml1.bbclass: copy .config to S if externalsr is in use

[Markus Lehtonen]

This makes it easier to track changes made to config.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 meta/classes/cml1.bbclass | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
index 95cf584..a95a2bf 100644
--- a/meta/classes/cml1.bbclass
+++ b/meta/classes/cml1.bbclass
@@ -28,6 +28,11 @@ python do_menuconfig() {
 
     oe_terminal("${SHELL} -c \"make ${KCONFIG_CONFIG_COMMAND}; if [ \$? -ne 0 ]; then echo 'Command failed.'; printf 'Press any key to continue... '; read r; fi\"", '${PN} Configuration', d)
 
+    # Copy .config back to source tree if externalsrc is in use
+    if (d.getVar('EXTERNALSRC', True) and
+            os.path.exists(os.path.join(d.getVar('S', True), '.config'))):
+        shutil.copy2('.config', os.path.join(d.getVar('S', True), '.config'))
+
     # FIXME this check can be removed when the minimum bitbake version has been bumped
     if hasattr(bb.build, 'write_taint'):
         try:
-- 
2.1.4

[PATCH 5/5] devtool: update-recipe: create kernel config fragment

[Markus Lehtonen]

Create kernel config fragment if the user makes modifications to
.config. User may change .config e.g. by directly editing it or by
running the 'do_menuconfig' bitbake task which will copy the modified
.config back to the source tree. Devtool generates one monolithic
fragment by simply doing a diff between .config and .config.orig files
in the source directory. If either of these files is missing, the config
fragment is not gerenrated or updated. The output is a file,
'devtool-fragment.cfg' that gets added to SRC_URI in the recipe (as well
as copied into the 'oe-local-files' directory if that is present in the
source tree).

This patch also changes the devtool 'extract' command to create the
.config.orig file at the source tree creation time.

[YOCTO #6658]

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 50 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index f817671..aa9414b 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -20,6 +20,7 @@ import os
 import sys
 import re
 import shutil
+import subprocess
 import tempfile
 import logging
 import argparse
@@ -474,6 +475,7 @@ def _extract_source(srctree, keep_temp, devbranch, sync, d):
         if kconfig:
             # Store kernel config in srctree
             shutil.copy2(kconfig, srcsubdir)
+            shutil.copy2(kconfig, os.path.join(srcsubdir, '.config.orig'))
 
 
         tempdir_localdir = os.path.join(tempdir, 'oe-local-files')
@@ -804,6 +806,30 @@ def _export_patches(srctree, rd, start_rev, destdir):
     return (updated, added, existing_patches)
 
 
+def _create_kconfig_diff(srctree, rd, outfile):
+    """Create a kernel config fragment"""
+    # Only update config fragment if both config files exist
+    orig_config = os.path.join(srctree, '.config.orig')
+    new_config = os.path.join(srctree, '.config')
+    if os.path.exists(orig_config) and os.path.exists(new_config):
+        cmd = ['diff', '--new-line-format=%L', '--old-line-format=',
+               '--unchanged-line-format=', orig_config, new_config]
+        pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+                                stderr=subprocess.PIPE)
+        stdout, stderr = pipe.communicate()
+        if pipe.returncode == 1:
+            with open(outfile, 'w') as fobj:
+                fobj.write(stdout)
+        elif pipe.returncode == 0:
+            if os.path.exists(outfile):
+                # Remove fragment file in case of empty diff
+                os.unlink(outfile)
+        else:
+            raise bb.process.ExecutionError(cmd, pipe.returncode, stdout, stderr)
+        return True
+    return False
+
+
 def _export_local_files(srctree, rd, destdir):
     """Copy local files from srctree to given location.
        Returns three-tuple of dicts:
@@ -824,6 +850,7 @@ def _export_local_files(srctree, rd, destdir):
     updated = OrderedDict()
     added = OrderedDict()
     removed = OrderedDict()
+    local_files_dir = os.path.join(srctree, 'oe-local-files')
     git_files = _git_ls_tree(srctree)
     if 'oe-local-files' in git_files:
         # If tracked by Git, take the files from srctree HEAD. First get
@@ -834,11 +861,32 @@ def _export_local_files(srctree, rd, destdir):
                         env=dict(os.environ, GIT_WORK_TREE=destdir,
                                  GIT_INDEX_FILE=tmp_index))
         new_set = _git_ls_tree(srctree, tree, True).keys()
-    elif os.path.isdir(os.path.join(srctree, 'oe-local-files')):
+    elif os.path.isdir(local_files_dir):
         # If not tracked by Git, just copy from working copy
         new_set = _ls_tree(os.path.join(srctree, 'oe-local-files'))
         bb.process.run(['cp', '-ax',
                         os.path.join(srctree, 'oe-local-files', '.'), destdir])
+    else:
+        new_set = []
+
+    # Special handling for kernel config
+    if bb.data.inherits_class('kernel-yocto', rd):
+        fragment_fn = 'devtool-fragment.cfg'
+        fragment_path = os.path.join(destdir, fragment_fn)
+        if _create_kconfig_diff(srctree, rd, fragment_path):
+            if os.path.exists(fragment_path):
+                if fragment_fn not in new_set:
+                    new_set.append(fragment_fn)
+                # Copy fragment to local-files
+                if os.path.isdir(local_files_dir):
+                    shutil.copy2(fragment_path, local_files_dir)
+            else:
+                if fragment_fn in new_set:
+                    new_set.remove(fragment_fn)
+                # Remove fragment from local-files
+                if os.path.exists(os.path.join(local_files_dir, fragment_fn)):
+                    os.unlink(os.path.join(local_files_dir, fragment_fn))
+
     if new_set is not None:
         for fname in new_set:
             if fname in existing_files:
-- 
2.1.4