utils.py: Change _ensure_real_directory() to not resolve symlinks

Resolving symlinks during staging causes various issues:

* Split rules may not work properly as the resolved paths will differ
  depending on whether another artifact with a directory symlink has
  been staged in the same root directory or not, e.g., as part of
  compose.

* The order of symlinks in file lists is difficult to get right to
  guarantee consistent and predictable behavior as paths in a file list
  might rely on symlinks in the same file list. See #647 and #817.

* Staging order differences can lead to surprising results. See #390.

* Difficult to properly support absolute symlinks. Absolute symlinks are
  currently converted to relative symlinks, however, this doesn't always
  work. See #606 and #830.

This will require changes in projects that rely on the current behavior.
However, the changes are expected to be small and are often a sign of
buggy element files. E.g., elements that don't fully obey `bindir` or
`sbindir` variables.

3 files changed, 37 insertions, 52 deletions

diff --git a/buildstream/utils.py b/buildstream/utils.py
index 12407ba30..adc593d7c 100644
--- a/buildstream/utils.py
+++ b/buildstream/utils.py
@@ -772,32 +772,31 @@ def _resolve_symlinks(path):
     return os.path.realpath(path)
 
 
-def _ensure_real_directory(root, destpath):
-    # The realpath in the sandbox may refer to a file outside of the
-    # sandbox when any of the direcory branches are a symlink to an
-    # absolute path.
-    #
-    # This should not happen as we rely on relative_symlink_target() below
-    # when staging the actual symlinks which may lead up to this path.
-    #
-    destpath_resolved = _resolve_symlinks(destpath)
-    if not destpath_resolved.startswith(_resolve_symlinks(root)):
-        raise UtilError('Destination path resolves to a path outside ' +
-                        'of the staging area\n\n' +
-                        '  Destination path: {}\n'.format(destpath) +
-                        '  Real path: {}'.format(destpath_resolved))
-
-    # Ensure the real destination path exists before trying to get the mode
-    # of the real destination path.
-    #
-    # It is acceptable that chunks create symlinks inside artifacts which
-    # refer to non-existing directories, they will be created on demand here
-    # at staging time.
-    #
-    if not os.path.exists(destpath_resolved):
-        os.makedirs(destpath_resolved)
-
-    return destpath_resolved
+# _ensure_real_directory()
+#
+# Ensure `path` is a real directory and there are no symlink components.
+#
+# Symlink components are allowed in `root`.
+#
+def _ensure_real_directory(root, path):
+    destpath = root
+    for name in os.path.split(path):
+        destpath = os.path.join(destpath, name)
+        try:
+            deststat = os.lstat(destpath)
+            if not stat.S_ISDIR(deststat.st_mode):
+                relpath = destpath[len(root):]
+
+                if stat.S_ISLNK(deststat.st_mode):
+                    filetype = 'symlink'
+                elif stat.S_ISREG(deststat.st_mode):
+                    filetype = 'regular file'
+                else:
+                    filetype = 'special file'
+
+                raise UtilError('Destination is a {}, not a directory: {}'.format(filetype, relpath))
+        except FileNotFoundError:
+            os.makedirs(destpath)
 
 
 # _process_list()
@@ -836,6 +835,10 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result,
         srcpath = os.path.join(srcdir, path)
         destpath = os.path.join(destdir, path)
 
+        # Ensure that the parent of the destination path exists without symlink
+        # components.
+        _ensure_real_directory(destdir, os.path.dirname(path))
+
         # Add to the results the list of files written
         if report_written:
             result.files_written.append(path)
@@ -847,11 +850,6 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result,
         # The destination directory may not have been created separately
         permissions.extend(_copy_directories(srcdir, destdir, path))
 
-        # Ensure that broken symlinks to directories have their targets
-        # created before attempting to stage files across broken
-        # symlink boundaries
-        _ensure_real_directory(destdir, os.path.dirname(destpath))
-
         try:
             file_stat = os.lstat(srcpath)
             mode = file_stat.st_mode
@@ -865,13 +863,7 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result,
 
         if stat.S_ISDIR(mode):
             # Ensure directory exists in destination
-            if not os.path.exists(destpath):
-                _ensure_real_directory(destdir, destpath)
-
-            dest_stat = os.lstat(_resolve_symlinks(destpath))
-            if not stat.S_ISDIR(dest_stat.st_mode):
-                raise UtilError('Destination not a directory. source has {}'
-                                ' destination has {}'.format(srcpath, destpath))
+            _ensure_real_directory(destdir, path)
             permissions.append((destpath, os.stat(srcpath).st_mode))
 
         elif stat.S_ISLNK(mode):
diff --git a/tests/integration/compose-symlinks.py b/tests/integration/compose-symlinks.py
index c6027bf2b..027feea7c 100644
--- a/tests/integration/compose-symlinks.py
+++ b/tests/integration/compose-symlinks.py
@@ -18,7 +18,7 @@ DATA_DIR = os.path.join(
 )
 
 
-# Test that staging a file inside a directory symlink works as expected.
+# Test that staging a file inside a directory symlink fails.
 #
 # Regression test for https://gitlab.com/BuildStream/buildstream/issues/270
 @pytest.mark.datafiles(DATA_DIR)
@@ -34,11 +34,6 @@ def test_compose_symlinks(cli, tmpdir, datafiles):
     os.symlink(os.path.join('usr', 'sbin'), symlink_file, target_is_directory=True)
 
     result = cli.run(project=project, args=['build', 'compose-symlinks/compose.bst'])
-    result.assert_success()
 
-    result = cli.run(project=project, args=['artifact', 'checkout', 'compose-symlinks/compose.bst',
-                                            '--directory', checkout])
-    result.assert_success()
-
-    assert set(walk_dir(checkout)) == set(['/sbin', '/usr', '/usr/sbin',
-                                           '/usr/sbin/init', '/usr/sbin/dummy'])
+    assert result.exit_code == -1
+    assert 'Destination is a symlink, not a directory: /sbin' in result.stderr
diff --git a/tests/integration/symlinks.py b/tests/integration/symlinks.py
index 22ff527f8..5674b5778 100644
--- a/tests/integration/symlinks.py
+++ b/tests/integration/symlinks.py
@@ -44,7 +44,7 @@ def test_absolute_symlinks_made_relative(cli, tmpdir, datafiles):
 
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason='Only available with a functioning sandbox')
-def test_allow_overlaps_inside_symlink_with_dangling_target(cli, tmpdir, datafiles):
+def test_disallow_overlaps_inside_symlink_with_dangling_target(cli, tmpdir, datafiles):
     project = os.path.join(datafiles.dirname, datafiles.basename)
     checkout = os.path.join(cli.directory, 'checkout')
     element_name = 'symlinks/dangling-symlink-overlap.bst'
@@ -53,10 +53,8 @@ def test_allow_overlaps_inside_symlink_with_dangling_target(cli, tmpdir, datafil
     assert result.exit_code == 0
 
     result = cli.run(project=project, args=['artifact', 'checkout', element_name, '--directory', checkout])
-    assert result.exit_code == 0
-
-    # See the dangling-symlink*.bst elements for details on what we are testing.
-    assert_contains(checkout, ['/usr/orgs/orgname/etc/org.conf'])
+    assert result.exit_code == -1
+    assert 'Destination is a symlink, not a directory: /opt/orgname' in result.stderr
 
 
 @pytest.mark.datafiles(DATA_DIR)
@@ -75,4 +73,4 @@ def test_detect_symlink_overlaps_pointing_outside_sandbox(cli, tmpdir, datafiles
     # tries to actually write there.
     result = cli.run(project=project, args=['artifact', 'checkout', element_name, '--directory', checkout])
     assert result.exit_code == -1
-    assert "Destination path resolves to a path outside of the staging area" in result.stderr
+    assert 'Destination is a symlink, not a directory: /opt/escape-hatch' in result.stderr