[PATCH 0/2] combo-layer: exclude files

[PATCH 0/2] combo-layer: exclude files

[Patrick Ohly]

I ran into the merge conflicts described in the second patch recently
due to changes in OE-core's conf sample files. Richard told me that he
is aware of the issue and just deals with it manually because he never
had time to automate it; I hope these patches do that sufficiently
well to be included.

They worked for me when I re-imported all patches in OE-core and
bitbake since Dizzy was branched off. I also tested "combo-layer init"
with files excluded.

Note that these patches may apply directly to master, but I haven't
tested that because in my tree I have them on top of the other two,
independent patches to combo-layer that I posted yesterday. I can
resend as a single patch series if that's preferred.

Patrick Ohly (2):
  combo-layer: runcmd() with separate output
  combo-layer: exclude files

 scripts/combo-layer              | 59 ++++++++++++++++++++++++++++++++++------
 scripts/combo-layer.conf.example | 14 ++++++++++
 2 files changed, 65 insertions(+), 8 deletions(-)

-- 
2.1.4

[PATCH 1/2] combo-layer: runcmd() with separate output

[Patrick Ohly]

Allow the caller to specify a separate output stream. stderr is always
a temporary file opened by runcmd(), so read from that to capture
output for error reporting *and* the return value.

The reasoning for the latter is a) that this preserves the traditional
behavior when out=None and b) if the caller wants the content of
stdout, it can read from the stream itself, which is not possible for
the temporary stderr.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 scripts/combo-layer | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/scripts/combo-layer b/scripts/combo-layer
index 5f10e83..fb3fb50 100755
--- a/scripts/combo-layer
+++ b/scripts/combo-layer
@@ -149,23 +149,27 @@ class Configuration(object):
             logger.error("ERROR: patchutils package is missing, please install it (e.g. # apt-get install patchutils)")
             sys.exit(1)
 
-def runcmd(cmd,destdir=None,printerr=True):
+def runcmd(cmd,destdir=None,printerr=True,out=None):
     """
         execute command, raise CalledProcessError if fail
         return output if succeed
     """
     logger.debug("run cmd '%s' in %s" % (cmd, os.getcwd() if destdir is None else destdir))
-    out = os.tmpfile()
+    if not out:
+        out = os.tmpfile()
+        err = out
+    else:
+        err = os.tmpfile()
     try:
-        subprocess.check_call(cmd, stdout=out, stderr=out, cwd=destdir, shell=True)
+        subprocess.check_call(cmd, stdout=out, stderr=err, cwd=destdir, shell=isinstance(cmd, str))
     except subprocess.CalledProcessError,e:
-        out.seek(0)
+        err.seek(0)
         if printerr:
-            logger.error("%s" % out.read())
+            logger.error("%s" % err.read())
         raise e
 
-    out.seek(0)
-    output = out.read()
+    err.seek(0)
+    output = err.read()
     logger.debug("output: %s" % output )
     return output
 
-- 
2.1.4

Re: [PATCH 1/2] combo-layer: runcmd() with separate output

[Paul Eggleton]

On Tuesday 10 March 2015 11:10:48 Patrick Ohly wrote:
> Allow the caller to specify a separate output stream. stderr is always
> a temporary file opened by runcmd(), so read from that to capture
> output for error reporting *and* the return value.
> 
> The reasoning for the latter is a) that this preserves the traditional
> behavior when out=None and b) if the caller wants the content of
> stdout, it can read from the stream itself, which is not possible for
> the temporary stderr.
> 
> Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> ---
>  scripts/combo-layer | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/combo-layer b/scripts/combo-layer
> index 5f10e83..fb3fb50 100755
> --- a/scripts/combo-layer
> +++ b/scripts/combo-layer
> @@ -149,23 +149,27 @@ class Configuration(object):
>              logger.error("ERROR: patchutils package is missing, please
> install it (e.g. # apt-get install patchutils)") sys.exit(1)
> 
> -def runcmd(cmd,destdir=None,printerr=True):
> +def runcmd(cmd,destdir=None,printerr=True,out=None):
>      """
>          execute command, raise CalledProcessError if fail
>          return output if succeed
>      """
>      logger.debug("run cmd '%s' in %s" % (cmd, os.getcwd() if destdir is
> None else destdir)) -    out = os.tmpfile()
> +    if not out:
> +        out = os.tmpfile()
> +        err = out
> +    else:
> +        err = os.tmpfile()
>      try:
> -        subprocess.check_call(cmd, stdout=out, stderr=out, cwd=destdir,
> shell=True) +        subprocess.check_call(cmd, stdout=out, stderr=err,
> cwd=destdir, shell=isinstance(cmd, str)) except
> subprocess.CalledProcessError,e:
> -        out.seek(0)
> +        err.seek(0)
>          if printerr:
> -            logger.error("%s" % out.read())
> +            logger.error("%s" % err.read())
>          raise e
> 
> -    out.seek(0)
> -    output = out.read()
> +    err.seek(0)
> +    output = err.read()
>      logger.debug("output: %s" % output )
>      return output

I'm fine with this change, but FYI unfortunately we have a number of copies of 
this function floating around. We'll probably end up having to do a refactoring 
of these at some point soon.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

[PATCH 2/2] combo-layer: exclude files

[Patrick Ohly]

Some combined repos intentionally do not include certain files.
For example, Poky does not include bitbake's setup files and
OE-core's sample files under meta/conf.

When these files get modified in the upstream repository, applying the
patches fails and requires manual intervention. That is merely a
nuisance for someone familiar with the problem, but a real show
stopper when having the import run automatically or by someone less
experienced.

Therefore this change introduces "file_exclude", a new per-repo list
of file patterns which removes all matching files when initializing or
updating a combined repository. Because fnmatch is used under the hood
to match full path strings, removing entire directories must be done
with a pattern ending in a '/*' (in contrast to file_filter).

For Poky, the additional configuration looks like this:

[bitbake]
...
file_exclude = classes/base.bbclass
	conf/bitbake.conf
	.gitignore
	MANIFEST.in
	setup.py
	TODO

[openembedded-core]
...
file_exclude = meta/conf/bblayers.conf.sample
	meta/conf/local.conf.sample
	meta/conf/local.conf.sample.extended
	meta/conf/site.conf.sample
---
 scripts/combo-layer              | 41 +++++++++++++++++++++++++++++++++++++++-
 scripts/combo-layer.conf.example | 14 ++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/scripts/combo-layer b/scripts/combo-layer
index fb3fb50..b121c99 100755
--- a/scripts/combo-layer
+++ b/scripts/combo-layer
@@ -20,6 +20,7 @@
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
+import fnmatch
 import os, sys
 import optparse
 import logging
@@ -208,7 +209,18 @@ def action_init(conf, args):
             else:
                 extract_dir = os.getcwd()
             file_filter = repo.get('file_filter', "")
-            runcmd("git archive %s | tar -x -C %s %s" % (initialrev, extract_dir, file_filter), ldir)
+            files = runcmd("git archive %s | tar -x -v -C %s %s" % (initialrev, extract_dir, file_filter), ldir)
+            exclude_patterns = repo.get('file_exclude', '').split()
+            if exclude_patterns:
+                # Implement file removal by letting tar create the
+                # file and then deleting it in the file system
+                # again. Uses the list of files created by tar (easier
+                # than walking the tree).
+                for file in files.split('\n'):
+                    for pattern in exclude_patterns:
+                        if fnmatch.fnmatch(file, pattern):
+                            os.unlink(os.path.join(extract_dir, file))
+                            break
             if not lastrev:
                 lastrev = runcmd("git rev-parse %s" % initialrev, ldir).strip()
                 conf.update(name, "last_revision", lastrev, initmode=True)
@@ -423,6 +435,33 @@ def action_update(conf, args):
                 runcmd("%s %s %s %s" % (repo['hook'], patch, revlist[count], name))
                 count=count-1
 
+        # Step 3a: Filter out unwanted files and patches.
+        exclude = repo.get('file_exclude', '')
+        if exclude:
+            filter = ['filterdiff', '-p1']
+            for path in exclude.split():
+                filter.append('-x')
+                filter.append(path)
+            for patch in patchlist[:]:
+                filtered = patch + '.tmp'
+                with open(filtered, 'w') as f:
+                    runcmd(filter + [patch], out=f)
+                # Now check for empty patches.
+                if runcmd(['filterdiff', '--list', filtered]):
+                    # Possibly modified.
+                    os.unlink(patch)
+                    os.rename(filtered, patch)
+                else:
+                    # Empty, ignore it. Must also remove from revlist.
+                    fromline = open(patch, 'r').readline()
+                    m = re.match(r'''^From ([0-9a-fA-F]+) .*\n''', fromline)
+                    rev = m.group(1)
+                    logger.debug('skipping empty patch %s = %s' % (patch, rev))
+                    os.unlink(patch)
+                    os.unlink(filtered)
+                    patchlist.remove(patch)
+                    revlist.remove(rev)
+
         # Step 4: write patch list and revision list to file, for user to edit later
         patchlist_file = os.path.join(os.getcwd(), patch_dir, "patchlist-%s" % name)
         repo['patchlist'] = patchlist_file
diff --git a/scripts/combo-layer.conf.example b/scripts/combo-layer.conf.example
index 8ad8615..0ef80cb 100644
--- a/scripts/combo-layer.conf.example
+++ b/scripts/combo-layer.conf.example
@@ -38,6 +38,20 @@ last_revision =
 #   file_filter = src/*.c : only include the src *.c file
 #   file_filter = src/main.c src/Makefile.am : only include these two files
 
+# file_exclude: filter out these file(s)
+# file_exclude = [path] [path] ...
+#
+# Each entry must match a file name. In contrast do file_filter, matching
+# a directory has no effect. To achieve that, use append a * wildcard
+# at the end.
+#
+# Wildcards are applied to the complete path and also match slashes.
+#
+# example:
+#   file_exclude = src/foobar/*  : exclude everything under src/foobar
+#   file_exclude = src/main.c : filter out main.c after including it with file_filter = src/*.c
+#   file_exclude = *~ : exclude backup files
+
 # hook: if provided, the tool will call the hook to process the generated
 #     patch from upstream, and then apply the modified patch to the combo
 #     repo.
-- 
2.1.4

Re: [PATCH 2/2] combo-layer: exclude files

[Paul Eggleton]

On Tuesday 10 March 2015 11:10:49 Patrick Ohly wrote:
> Some combined repos intentionally do not include certain files.
> For example, Poky does not include bitbake's setup files and
> OE-core's sample files under meta/conf.
> 
> When these files get modified in the upstream repository, applying the
> patches fails and requires manual intervention. That is merely a
> nuisance for someone familiar with the problem, but a real show
> stopper when having the import run automatically or by someone less
> experienced.
> 
> Therefore this change introduces "file_exclude", a new per-repo list
> of file patterns which removes all matching files when initializing or
> updating a combined repository. Because fnmatch is used under the hood
> to match full path strings, removing entire directories must be done
> with a pattern ending in a '/*' (in contrast to file_filter).
> 
> For Poky, the additional configuration looks like this:
> 
> [bitbake]
> ...
> file_exclude = classes/base.bbclass
> 	conf/bitbake.conf
> 	.gitignore
> 	MANIFEST.in
> 	setup.py
> 	TODO
> 
> [openembedded-core]
> ...
> file_exclude = meta/conf/bblayers.conf.sample
> 	meta/conf/local.conf.sample
> 	meta/conf/local.conf.sample.extended
> 	meta/conf/site.conf.sample
> ---
>  scripts/combo-layer              | 41
> +++++++++++++++++++++++++++++++++++++++- scripts/combo-layer.conf.example |
> 14 ++++++++++++++
>  2 files changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/combo-layer b/scripts/combo-layer
> index fb3fb50..b121c99 100755
> --- a/scripts/combo-layer
> +++ b/scripts/combo-layer
> @@ -20,6 +20,7 @@
>  # with this program; if not, write to the Free Software Foundation, Inc.,
>  # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> 
> +import fnmatch
>  import os, sys
>  import optparse
>  import logging
> @@ -208,7 +209,18 @@ def action_init(conf, args):
>              else:
>                  extract_dir = os.getcwd()
>              file_filter = repo.get('file_filter', "")
> -            runcmd("git archive %s | tar -x -C %s %s" % (initialrev,
> extract_dir, file_filter), ldir) +            files = runcmd("git archive
> %s | tar -x -v -C %s %s" % (initialrev, extract_dir, file_filter), ldir) + 
>           exclude_patterns = repo.get('file_exclude', '').split() +        
>    if exclude_patterns:
> +                # Implement file removal by letting tar create the
> +                # file and then deleting it in the file system
> +                # again. Uses the list of files created by tar (easier
> +                # than walking the tree).
> +                for file in files.split('\n'):
> +                    for pattern in exclude_patterns:
> +                        if fnmatch.fnmatch(file, pattern):
> +                            os.unlink(os.path.join(extract_dir, file))
> +                            break
>              if not lastrev:
>                  lastrev = runcmd("git rev-parse %s" % initialrev,
> ldir).strip() conf.update(name, "last_revision", lastrev, initmode=True) @@
> -423,6 +435,33 @@ def action_update(conf, args):
>                  runcmd("%s %s %s %s" % (repo['hook'], patch,
> revlist[count], name)) count=count-1
> 
> +        # Step 3a: Filter out unwanted files and patches.
> +        exclude = repo.get('file_exclude', '')
> +        if exclude:
> +            filter = ['filterdiff', '-p1']
> +            for path in exclude.split():
> +                filter.append('-x')
> +                filter.append(path)
> +            for patch in patchlist[:]:
> +                filtered = patch + '.tmp'
> +                with open(filtered, 'w') as f:
> +                    runcmd(filter + [patch], out=f)
> +                # Now check for empty patches.
> +                if runcmd(['filterdiff', '--list', filtered]):
> +                    # Possibly modified.
> +                    os.unlink(patch)
> +                    os.rename(filtered, patch)
> +                else:
> +                    # Empty, ignore it. Must also remove from revlist.
> +                    fromline = open(patch, 'r').readline()

Can you use "with open(..." here? Otherwise looks good.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre