Repository: pytest-dev/pyfakefs (python)

Issue:
adding a realdir with target to '/' creates a self-referential '' entry that makes the fs unusable
**Describe the bug**
My first baby steps with `pyfakefs` aren't going well... I am trying to populate a fake fs with custom entries under root (to test a system that expects certain files at absolute locations).

Instead of seeing the desired content, doing a simple `os.walk()` results in an infinite loop, seemingly because an extra, self-referential entry (the empty string `''`) is created under `/`.  If I look at the beginning of the iterator of `os.walk()`, I see a repeating alternation of `'/'` and `'/tmp'`.

**How To Reproduce**

```shell
cd $(mktemp -d)
mkdir -p data/root/foo/bar
touch data/root/foo/bar/file-{0,1,2,3}.txt
touch __init__.py

cat > conftest.py <<"EOF"
import pytest

@pytest.fixture
def myfs(fs):
    fs.add_real_directory('data/root', target_path='/')
    yield fs
EOF

cat > test_fake.py <<"EOF"
import os

def test_fake(myfs):
    print('\n\nwith fake fs')
    for _, e in zip(range(10), os.walk('/')):
        print(e)

def test_real():
    print('\n\nwith real fs')
    os.chdir('data/root')
    for _, e in zip(range(10), os.walk('.')):
        print(e)
EOF

pytest -s test_fake.py
```

The output of the above is:

```none
================================================== test session starts ==================================================
platform linux -- Python 3.10.9, pytest-7.1.2, pluggy-1.0.0
rootdir: /tmp/tmp.xpzkMjworA
plugins: xdist-2.5.0, anyio-3.5.0, cov-3.0.0, pyfakefs-5.3.0, hypothesis-6.29.3, forked-1.3.0
collected 2 items                                                                                                       

test_fake.py 

with fake fs
('/', ['tmp', ''], [])
('/tmp', [], [])
('/', ['tmp', ''], [])
('/tmp', [], [])
('/', ['tmp', ''], [])
('/tmp', [], [])
('/', ['tmp', ''], [])
('/tmp', [], [])
('/', ['tmp', ''], [])
('/tmp', [], [])
.

with real fs
('.', ['foo'], [])
('./foo', ['bar'], [])
('./foo/bar', [], ['file-2.txt', 'file-3.txt', 'file-0.txt', 'file-1.txt'])
.

=================================================== 2 passed in 0.19s ===================================================
```

What I was expecting was to see the same output for `test_fake` as for `test_real`, but relocated under `'/'`.

**Your environment**

```none
Linux-6.2.0-32-generic-x86_64-with-glibc2.35
Python 3.10.9 (main, Jan 11 2023, 15:21:40) [GCC 11.2.0]
pyfakefs 5.3.0
pytest 7.1.2
```

**Probable cause**

When I run the test in PyCharm's debugger, I noticed a strange entry under the root dir of `myfs`: the empty string (`''`, third line of the output below):

```none
>>> str(myfs)
'/'(40777):
  'tmp'(40777):
  ''(40775):
  'foo'(40775):
    'bar'(40775):
      'file-2.txt'(100444)
      'file-3.txt'(100444)
      'file-0.txt'(100444)
      'file-1.txt'(100444)
```

Incidentally, if I try to print that in the unit test itself (rather than copying the view from the debugger above), I get:

```none
self = <pyfakefs.fake_file.FakeDirectory object at 0x7fe0490633d0>

    def __str__(self) -> str:
        description = super(FakeDirectory, self).__str__() + ":\n"
>       for item in self.entries:
E       RuntimeError: dictionary changed size during iteration
```

In any case, it seems that this erroneous entry has been created during `fs.add_real_directory`. Inspecting it in the debugger, that entry refers to the filesystem itself. Specifically:

```python
>>> myfs.root_dir.entries[''].filesystem is myfs
True

# or, in other words:
>>> id(myfs.root_dir.entries[''].filesystem) == id(myfs)
True
```


Pull request:
Allow merging real with fake directories
- changed behavior of add_real_directory() to be able to map a real directory to an existing directory in the fake filesystem
- fixes #901

#### Tasks
- [x] Unit tests added that reproduce the issue or prove feature is working
- [x] Fix or feature added
- [x] Entry to release notes added
- [x] Pre-commit CI shows no errors
- [x] Unit tests passing
- [x] For documentation changes: The Read the Docs preview builds and looks as expected


Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 029d571..f1e7746 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -8,6 +8,8 @@ Adds official support for Python 3.12.
 
 ### Changes
 * add official support for Python 3.12
+* changed behavior of `add_real_directory` to be able to map a real directory
+  to an existing directory in the fake filesystem (see #901)
 
 ### Fixes
 * removed a leftover debug print statement (see [#869](../../issues/869))
diff --git a/docs/usage.rst b/docs/usage.rst
index 7a38a53..95376b0 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -726,7 +726,9 @@ files are never changed.
 
 ``add_real_file()``, ``add_real_directory()`` and ``add_real_symlink()`` also
 allow you to map a file or a directory tree into another location in the
-fake filesystem via the argument ``target_path``.
+fake filesystem via the argument ``target_path``. If the target directory already exists
+in the fake filesystem, the directory contents are merged. If a file in the fake filesystem
+would be overwritten by a file from the real filesystem, an exception is raised.
 
 .. code:: python
 
diff --git a/extra_requirements.txt b/extra_requirements.txt
index ea3bf32..dd9bb59 100644
--- a/extra_requirements.txt
+++ b/extra_requirements.txt
@@ -15,6 +15,6 @@ scandir>=1.8
 # we use the latest version to see any problems with new versions
 pandas==1.3.5; python_version == '3.7' # pyup: ignore
 pandas==2.0.3; python_version == '3.8' # pyup: ignore
-pandas==2.1.2; python_version > '3.8'
+pandas==2.1.3; python_version > '3.8'
 xlrd==2.0.1
 openpyxl==3.1.2
diff --git a/pyfakefs/fake_filesystem.py b/pyfakefs/fake_filesystem.py
index 2777576..28bcd69 100644
--- a/pyfakefs/fake_filesystem.py
+++ b/pyfakefs/fake_filesystem.py
@@ -2232,6 +2232,8 @@ class FakeFilesystem:
         :py:class:`FakeDirectory<pyfakefs.fake_file.FakeDirectory>` object.
         Add entries in the fake directory corresponding to
         the entries in the real directory.  Symlinks are supported.
+        If the target directory already exists in the fake filesystem, the directory
+        contents are merged. Overwriting existing files is not allowed.
 
         Args:
             source_path: The path to the existing directory.
@@ -2254,49 +2256,78 @@ class FakeFilesystem:
             :py:class:`FakeDirectory<pyfakefs.fake_file.FakeDirectory>` object.
 
         Raises:
-            OSError: if the directory does not exist in the real file system.
-            OSError: if the directory already exists in the fake file system.
+            OSError: if the directory does not exist in the real filesystem.
+            OSError: if a file or link exists in the fake filesystem where a real
+                file or directory shall be mapped.
         """
-        source_path_str = make_string_path(source_path)  # TODO: add test
+        source_path_str = make_string_path(source_path)
         source_path_str = self._path_without_trailing_separators(source_path_str)
         if not os.path.exists(source_path_str):
             self.raise_os_error(errno.ENOENT, source_path_str)
         target_path_str = make_string_path(target_path or source_path_str)
+
+        # get rid of inconsistencies between real and fake path separators
+        if os.altsep is not None:
+            target_path_str = os.path.normpath(target_path_str)
+        if os.sep != self.path_separator:
+            target_path_str = target_path_str.replace(os.sep, self.path_separator)
+
         self._auto_mount_drive_if_needed(target_path_str)
-        new_dir: FakeDirectory
         if lazy_read:
-            parent_path = os.path.split(target_path_str)[0]
-            if self.exists(parent_path):
-                parent_dir = self.get_object(parent_path)
-            else:
-                parent_dir = self.create_dir(parent_path)
-            new_dir = FakeDirectoryFromRealDirectory(
-                source_path_str, self, read_only, target_path_str
+            self._create_fake_from_real_dir_lazily(
+                source_path_str, target_path_str, read_only
             )
-            parent_dir.add_entry(new_dir)
         else:
-            new_dir = self.create_dir(target_path_str)
-            for base, _, files in os.walk(source_path_str):
-                new_base = os.path.join(
-                    new_dir.path,  # type: ignore[arg-type]
-                    os.path.relpath(base, source_path_str),
-                )
-                for fileEntry in os.listdir(base):
-                    abs_fileEntry = os.path.join(base, fileEntry)
-
-                    if not os.path.islink(abs_fileEntry):
-                        continue
-
-                    self.add_real_symlink(
-                        abs_fileEntry, os.path.join(new_base, fileEntry)
-                    )
-                for fileEntry in files:
-                    path = os.path.join(base, fileEntry)
-                    if os.path.islink(path):
-                        continue
+            self._create_fake_from_real_dir(source_path_str, target_path_str, read_only)
+        return cast(FakeDirectory, self.get_object(target_path_str))
+
+    def _create_fake_from_real_dir(self, source_path_str, target_path_str, read_only):
+        if not self.exists(target_path_str):
+            self.create_dir(target_path_str)
+        for base, _, files in os.walk(source_path_str):
+            new_base = os.path.join(
+                target_path_str,
+                os.path.relpath(base, source_path_str),
+            )
+            for file_entry in os.listdir(base):
+                file_path = os.path.join(base, file_entry)
+                if os.path.islink(file_path):
+                    self.add_real_symlink(file_path, os.path.join(new_base, file_entry))
+            for file_entry in files:
+                path = os.path.join(base, file_entry)
+                if not os.path.islink(path):
                     self.add_real_file(
-                        path, read_only, os.path.join(new_base, fileEntry)
+                        path, read_only, os.path.join(new_base, file_entry)
                     )
+
+    def _create_fake_from_real_dir_lazily(
+        self, source_path_str, target_path_str, read_only
+    ):
+        if self.exists(target_path_str):
+            if not self.isdir(target_path_str):
+                raise OSError(errno.ENOTDIR, "Mapping target is not a directory")
+            for entry in os.listdir(source_path_str):
+                src_entry_path = os.path.join(source_path_str, entry)
+                target_entry_path = os.path.join(target_path_str, entry)
+                if os.path.isdir(src_entry_path):
+                    self.add_real_directory(
+                        src_entry_path, read_only, True, target_entry_path
+                    )
+                elif os.path.islink(src_entry_path):
+                    self.add_real_symlink(src_entry_path, target_entry_path)
+                elif os.path.isfile(src_entry_path):
+                    self.add_real_file(src_entry_path, read_only, target_entry_path)
+            return self.get_object(target_path_str)
+
+        parent_path = os.path.split(target_path_str)[0]
+        if self.exists(parent_path):
+            parent_dir = self.get_object(parent_path)
+        else:
+            parent_dir = self.create_dir(parent_path)
+        new_dir = FakeDirectoryFromRealDirectory(
+            source_path_str, self, read_only, target_path_str
+        )
+        parent_dir.add_entry(new_dir)
         return new_dir
 
     def add_real_paths(
@@ -2322,8 +2353,8 @@ class FakeFilesystem:
         Raises:
             OSError: if any of the files and directories in the list
                 does not exist in the real file system.
-            OSError: if any of the files and directories in the list
-                already exists in the fake file system.
+            OSError: if a file or link exists in the fake filesystem where a real
+                file or directory shall be mapped.
         """
         for path in path_list:
             if os.path.isdir(path):


Test patch:
diff --git a/pyfakefs/tests/fake_filesystem_test.py b/pyfakefs/tests/fake_filesystem_test.py
index 812e298..917725f 100644
--- a/pyfakefs/tests/fake_filesystem_test.py
+++ b/pyfakefs/tests/fake_filesystem_test.py
@@ -17,8 +17,10 @@
 import contextlib
 import errno
 import os
+import shutil
 import stat
 import sys
+import tempfile
 import unittest
 from unittest.mock import patch
 
@@ -2046,10 +2048,88 @@ class RealFileSystemAccessTest(RealFsTestCase):
         with self.raises_os_error(errno.EEXIST):
             self.filesystem.add_real_file(real_file_path)
 
-    def test_existing_fake_directory_raises(self):
-        self.filesystem.create_dir(self.root_path)
-        with self.raises_os_error(errno.EEXIST):
-            self.filesystem.add_real_directory(self.root_path)
+    @contextlib.contextmanager
+    def create_real_paths(self):
+        real_dir_root = os.path.join(tempfile.gettempdir(), "root")
+        try:
+            for dir_name in ("foo", "bar"):
+                real_dir = os.path.join(real_dir_root, dir_name)
+                os.makedirs(real_dir, exist_ok=True)
+                with open(os.path.join(real_dir, "test.txt"), "w") as f:
+                    f.write("test")
+                sub_dir = os.path.join(real_dir, "sub")
+                os.makedirs(sub_dir, exist_ok=True)
+                with open(os.path.join(sub_dir, "sub.txt"), "w") as f:
+                    f.write("sub")
+            yield real_dir_root
+        finally:
+            shutil.rmtree(real_dir_root, ignore_errors=True)
+
+    def test_existing_fake_directory_is_merged_lazily(self):
+        self.filesystem.create_file(os.path.join("/", "root", "foo", "test1.txt"))
+        self.filesystem.create_dir(os.path.join("root", "baz"))
+        with self.create_real_paths() as root_dir:
+            self.filesystem.add_real_directory(root_dir, target_path="/root")
+            self.assertTrue(
+                self.filesystem.exists(os.path.join("root", "foo", "test.txt"))
+            )
+            self.assertTrue(
+                self.filesystem.exists(os.path.join("root", "foo", "test1.txt"))
+            )
+            self.assertTrue(
+                self.filesystem.exists(os.path.join("root", "bar", "sub", "sub.txt"))
+            )
+            self.assertTrue(self.filesystem.exists(os.path.join("root", "baz")))
+
+    def test_existing_fake_directory_is_merged(self):
+        self.filesystem.create_file(os.path.join("/", "root", "foo", "test1.txt"))
+        self.filesystem.create_dir(os.path.join("root", "baz"))
+        with self.create_real_paths() as root_dir:
+            self.filesystem.add_real_directory(
+                root_dir, target_path="/root", lazy_read=False
+            )
+            self.assertTrue(
+                self.filesystem.exists(os.path.join("root", "foo", "test.txt"))
+            )
+            self.assertTrue(
+                self.filesystem.exists(os.path.join("root", "foo", "test1.txt"))
+            )
+            self.assertTrue(
+                self.filesystem.exists(os.path.join("root", "bar", "sub", "sub.txt"))
+            )
+            self.assertTrue(self.filesystem.exists(os.path.join("root", "baz")))
+
+    def test_fake_files_cannot_be_overwritten(self):
+        self.filesystem.create_file(os.path.join("/", "root", "foo", "test.txt"))
+        with self.create_real_paths() as root_dir:
+            with self.raises_os_error(errno.EEXIST):
+                self.filesystem.add_real_directory(root_dir, target_path="/root")
+
+    def test_cannot_overwrite_file_with_dir(self):
+        self.filesystem.create_file(os.path.join("/", "root", "foo"))
+        with self.create_real_paths() as root_dir:
+            with self.raises_os_error(errno.ENOTDIR):
+                self.filesystem.add_real_directory(root_dir, target_path="/root/")
+
+    def test_cannot_overwrite_symlink_with_dir(self):
+        self.filesystem.create_symlink(
+            os.path.join("/", "root", "foo"), os.path.join("/", "root", "link")
+        )
+        with self.create_real_paths() as root_dir:
+            with self.raises_os_error(errno.EEXIST):
+                self.filesystem.add_real_directory(root_dir, target_path="/root/")
+
+    def test_symlink_is_merged(self):
+        self.skip_if_symlink_not_supported(force_real_fs=True)
+        self.filesystem.create_dir(os.path.join("/", "root", "foo"))
+        with self.create_real_paths() as root_dir:
+            link_path = os.path.join(root_dir, "link.txt")
+            target_path = os.path.join("foo", "sub", "sub.txt")
+            os.symlink(target_path, link_path)
+            self.filesystem.add_real_directory(root_dir, target_path="/root")
+            fake_link_path = os.path.join("/", "root", "link.txt")
+            self.assertTrue(self.filesystem.exists(fake_link_path))
+            self.assertTrue(self.filesystem.islink(fake_link_path))
 
     def check_fake_file_stat(self, fake_file, real_file_path, target_path=None):
         if target_path is None or target_path == real_file_path:
@@ -2366,11 +2446,6 @@ class RealFileSystemAccessTest(RealFsTestCase):
                 self.filesystem.exists("/path/fixtures/symlink_file_relative")
             )
 
-    def test_add_existing_real_directory_tree_to_existing_path(self):
-        self.filesystem.create_dir("/foo/bar")
-        with self.raises_os_error(errno.EEXIST):
-            self.filesystem.add_real_directory(self.root_path, target_path="/foo/bar")
-
     def test_add_existing_real_directory_tree_to_other_path(self):
         self.filesystem.add_real_directory(self.root_path, target_path="/foo/bar")
         self.assertFalse(
@@ -2420,7 +2495,7 @@ class RealFileSystemAccessTest(RealFsTestCase):
         self.filesystem.set_disk_usage(disk_size, real_dir_path)
         self.filesystem.add_real_directory(real_dir_path)
 
-        # the directory contents have not been read, the the disk usage
+        # the directory contents have not been read, the disk usage
         # has not changed
         self.assertEqual(disk_size, self.filesystem.get_disk_usage(real_dir_path).free)
         # checking for existence shall read the directory contents