[PATCH 0/3] combo-layer + file_exclude

[PATCH 0/3] combo-layer + file_exclude

[Patrick Ohly]

Applying the new feature to additional repos highlighted some issue.
Proposing for master, but would also be suitable for Fido.

The following changes since commit 2d923d6dfe9431dbc005f8ba39838eb4519c471c:

  python-pygobject: Disable parallel make install in native case (2015-03-25 12:38:44 +0000)

are available in the git repository at:

  git://github.com/pohly/openembedded-core combo-layer
  https://github.com/pohly/openembedded-core/tree/combo-layer

Patrick Ohly (3):
  combo-layer: clean up dest_dir checking
  combo-layer: fix file_exclude for dest_dir = .
  combo-layer: fix file_exclude for empty commits

 scripts/combo-layer | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

-- 
2.1.4

[PATCH 1/3] combo-layer: clean up dest_dir checking

[Patrick Ohly]

Empty dest_dir is basically undocumented behavior. The sample conf
only mentions using just a dot for the current directory. In practice,
the empty string does not work because of code like this:

def action_splitpatch(conf, args):
    ...
        if dest_dir != ".":
            filerange_root = '%s -x "%s/*"' % (filerange_root, dest_dir)

However, the empty string was not explicitly checked for, leading to
strange errors when trying to apply patches:

[12:50:23] Applying: foobar: xyz
fatal: unable to stat newly created file '/foobar': No such file or directory

This patch turns the empty string into an alias for the dot. This seems
more user-friendly than throwing an error. This alias is intentionally
not document in the sample conf, because the dot is clearer and works also
with older copies of combo-layer.

Instead of checking for both all the time and normalizing the path when
needed (as done in some places), rewrite the value in sanity_check()
and then only check for '.'.

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

diff --git a/scripts/combo-layer b/scripts/combo-layer
index 83cfc8e..4aed072 100755
--- a/scripts/combo-layer
+++ b/scripts/combo-layer
@@ -144,6 +144,10 @@ class Configuration(object):
                 if option not in self.repos[name]:
                     msg = "%s\nOption %s is not defined for component %s" %(msg, option, name)
                     missing_options.append(option)
+            # Sanitize dest_dir so that we do not have to deal with edge cases
+            # (empty string, double slashes) in the rest of the code.
+            dest_dir = os.path.normpath(self.repos[name]["dest_dir"])
+            self.repos[name]["dest_dir"] = "." if not dest_dir else dest_dir
         if msg != "":
             logger.error("configuration file %s has the following error: %s" % (self.conffile,msg))
             if self.localconffile and 'last_revision' in missing_options:
@@ -231,7 +235,7 @@ def action_init(conf, args):
                     pass
                 initialrev = rev
             dest_dir = repo['dest_dir']
-            if dest_dir and dest_dir != ".":
+            if dest_dir != ".":
                 extract_dir = os.path.join(os.getcwd(), dest_dir)
                 if not os.path.exists(extract_dir):
                     os.makedirs(extract_dir)
@@ -325,7 +329,7 @@ EOF
                             runcmd('git replace --edit %s' % rev)
 
                 # Optional: rewrite history to change commit messages or to move files.
-                if 'hook' in repo or dest_dir and dest_dir != ".":
+                if 'hook' in repo or dest_dir != ".":
                     filter_branch = ['git', 'filter-branch', '--force']
                     with tempfile.NamedTemporaryFile() as hookwrapper:
                         if 'hook' in repo:
@@ -353,7 +357,7 @@ tail -c +18 $tmpname | head -c -4
 ''' % (hook, name))
                             hookwrapper.flush()
                             filter_branch.extend(['--msg-filter', 'bash %s' % hookwrapper.name])
-                        if dest_dir and dest_dir != ".":
+                        if dest_dir != ".":
                             parent = os.path.dirname(dest_dir)
                             if not parent:
                                 parent = '.'
@@ -371,7 +375,7 @@ tail -c +18 $tmpname | head -c -4
                     if not os.path.exists(extract_dir):
                         os.makedirs(extract_dir)
                     copy_selected_files('HEAD', extract_dir, file_filter, exclude_patterns, '.',
-                                        subdir=dest_dir if dest_dir and dest_dir != '.' else '')
+                                        subdir=dest_dir if dest_dir != '.' else '')
                     runcmd('git add --all --force .')
                     if runcmd('git status --porcelain'):
                         # Something to commit.
-- 
2.1.4

[PATCH 2/3] combo-layer: fix file_exclude for dest_dir = .

[Patrick Ohly]

"filterdiff -x ./some/file" does not remove changes for some/file.
We must be more careful about constructing the path name and
only add the prefix when it really means a directory.

While at it, also better normalize the path in copy_selected_files()
early on, to handle double slashes. Useful should the function ever
gets used for something other that dest_dir (which gets normalized in
sanity_check()).

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

diff --git a/scripts/combo-layer b/scripts/combo-layer
index 4aed072..a1fc6ac 100755
--- a/scripts/combo-layer
+++ b/scripts/combo-layer
@@ -249,13 +249,16 @@ def action_init(conf, args):
                 # files already moved, we need to prepend the
                 # subdirectory to all filters, otherwise they would
                 # not match.
-                if subdir:
+                if subdir == '.':
+                    subdir = ''
+                elif subdir:
+                    subdir = os.path.normpath(subdir)
                     file_filter = ' '.join([subdir + '/' + x for x in file_filter.split()])
                     exclude_patterns = [subdir + '/' + x for x in exclude_patterns]
                 # To handle both cases, we cd into the target
                 # directory and optionally tell tar to strip the path
                 # prefix when the files were already moved.
-                subdir_components = len(os.path.normpath(subdir).split(os.path.sep)) if subdir else 0
+                subdir_components = len(subdir.split(os.path.sep)) if subdir else 0
                 strip=('--strip-components=%d' % subdir_components) if subdir else ''
                 # TODO: file_filter wild cards do not work (and haven't worked before either), because
                 # a) GNU tar requires a --wildcards parameter before turning on wild card matching.
@@ -375,7 +378,7 @@ tail -c +18 $tmpname | head -c -4
                     if not os.path.exists(extract_dir):
                         os.makedirs(extract_dir)
                     copy_selected_files('HEAD', extract_dir, file_filter, exclude_patterns, '.',
-                                        subdir=dest_dir if dest_dir != '.' else '')
+                                        subdir=dest_dir)
                     runcmd('git add --all --force .')
                     if runcmd('git status --porcelain'):
                         # Something to commit.
@@ -648,7 +651,7 @@ def action_update(conf, args):
             filter = ['filterdiff', '-p1']
             for path in exclude.split():
                 filter.append('-x')
-                filter.append('%s/%s' % (dest_dir, path) if dest_dir else path)
+                filter.append('%s/%s' % (dest_dir, path) if dest_dir != '.' else path)
             for patch in patchlist[:]:
                 filtered = patch + '.tmp'
                 with open(filtered, 'w') as f:
-- 
2.1.4

[PATCH 3/3] combo-layer: fix file_exclude for empty commits

[Patrick Ohly]

The code detecting empty patches after removing files with
file_exclude failed for commits which were already empty before (like
the initial commit in some repos): such patches are completely empty
files, without a From line.

Detect that case and just let the normal empty patch detection deal
with it.

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

diff --git a/scripts/combo-layer b/scripts/combo-layer
index a1fc6ac..1dce4a6 100755
--- a/scripts/combo-layer
+++ b/scripts/combo-layer
@@ -665,6 +665,10 @@ def action_update(conf, args):
                     # Empty, ignore it. Must also remove from revlist.
                     with open(patch, 'r') as f:
                         fromline = f.readline()
+                    if not fromline:
+                        # Patch must have been empty to start with. No need
+                        # to remove it.
+                        continue
                     m = re.match(r'''^From ([0-9a-fA-F]+) .*\n''', fromline)
                     rev = m.group(1)
                     logger.debug('skipping empty patch %s = %s' % (patch, rev))
-- 
2.1.4