plugins/elements/compose.py: Avoid losing files inside directory symlinks

The logic for determining which files were removed by integration
commands was broken when dealing with files staged within symlink
directories.

This rather weird scenario is only possible because of the way
BuildStream layers artifacts. If artifact 1 contains a symlink from
`/sbin` to `/usr/sbin`, and artifact 2 is staged on top and contains
a file `/sbin/init`, then the resulting filesystem contains a file at
`/usr/sbin/init`.

The manifest used by the compose element is generated from the contents
of the individual artifacts, so it lists the original paths such as
`/sbin/init`, but would would not contain `/usr/sbin/init` as nothing
has processed the symlinks.

The path `/sbin/init` is valid inside the composed tree, but filesystem
traversals that don't follow symlinks will not report that path in their
results. The compose plugin would look for `/sbin/init` in the results
of `utils.list_relative_paths()`, find it missing, and would act as if
some integration command had removed the file. This meant it would not
end up in the results.

To fix this, I have inverted the logic that processes the results of the
integration commands. We now work through every path in the manifest
and check it against the results of the integration commands, rather
than the other way around, and if any path from the manifest doesn't
appear in the snapshot we assume that it has staged in a different
location due to symlinks.

See: https://gitlab.com/BuildStream/buildstream/issues/270

1 files changed, 20 insertions, 13 deletions

diff --git a/buildstream/plugins/elements/compose.py b/buildstream/plugins/elements/compose.py
index 88da3799b..0e666c6e5 100644
--- a/buildstream/plugins/elements/compose.py
+++ b/buildstream/plugins/elements/compose.py
@@ -126,20 +126,27 @@ class ComposeElement(Element):
 
                 if require_split:
 
-                    seen = set()
-                    # Calculate added modified files
-                    for path in utils.list_relative_paths(basedir):
-                        seen.add(path)
-                        if snapshot.get(path) is None:
+                    # Calculate added, modified and removed files
+                    basedir_contents = set(utils.list_relative_paths(basedir))
+                    for path in manifest:
+                        if path in basedir_contents:
+                            if path in snapshot:
+                                preintegration_mtime = snapshot[path]
+                                if preintegration_mtime != getmtime(os.path.join(basedir, path)):
+                                    modified_files.add(path)
+                            else:
+                                # If the path appears in the manifest but not the initial snapshot,
+                                # it may be a file staged inside a directory symlink. In this case
+                                # the path we got from the manifest won't show up in the snapshot
+                                # because utils.list_relative_paths() doesn't recurse into symlink
+                                # directories.
+                                pass
+                        elif path in snapshot:
+                            removed_files.add(path)
+
+                    for path in basedir_contents:
+                        if path not in snapshot:
                             added_files.add(path)
-                        elif snapshot[path] != getmtime(os.path.join(basedir, path)):
-                            modified_files.add(path)
-
-                    # Calculate removed files
-                    removed_files = set([
-                        path for path in manifest
-                        if path not in seen
-                    ])
 
                     self.info("Integration modified {}, added {} and removed {} files"
                               .format(len(modified_files), len(added_files), len(removed_files)))