From 5303190810f94ca600e12e77d53ebed4f34c89f8 Mon Sep 17 00:00:00 2001
From: Tooru Fujisawa <arai_a@mac.com>
Date: Tue, 27 Feb 2024 07:31:35 +0000
Subject: [PATCH] Bug 1880746 - Part 4: Throw error when same directory is
 added to DIRS twice. r=firefox-build-system-reviewers,glandium

Differential Revision: https://phabricator.services.mozilla.com/D202099
---
 python/mozbuild/mozbuild/frontend/reader.py   | 37 ++++++++++++-------
 .../mozbuild/test/frontend/test_reader.py     | 16 ++++++--
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/python/mozbuild/mozbuild/frontend/reader.py b/python/mozbuild/mozbuild/frontend/reader.py
index d0066e107141..8c7dfcdcac97 100644
--- a/python/mozbuild/mozbuild/frontend/reader.py
+++ b/python/mozbuild/mozbuild/frontend/reader.py
@@ -831,7 +831,7 @@ class BuildReader(object):
         self.config = config
 
         self._log = logging.getLogger(__name__)
-        self._read_files = set()
+        self._read_files = {}
         self._execution_stack = []
         self.finder = finder
 
@@ -1113,18 +1113,6 @@ class BuildReader(object):
             "Reading file: {path}".format(path=path),
         )
 
-        if path in self._read_files:
-            log(
-                self._log,
-                logging.WARNING,
-                "read_already",
-                {"path": path},
-                "File already read. Skipping: {path}".format(path=path),
-            )
-            return
-
-        self._read_files.add(path)
-
         time_start = time.monotonic()
 
         topobjdir = config.topobjdir
@@ -1132,6 +1120,20 @@ class BuildReader(object):
         relpath = mozpath.relpath(path, config.topsrcdir)
         reldir = mozpath.dirname(relpath)
 
+        # NOTE: descend case is handled in the loop below.
+        if not descend:
+            if relpath in self._read_files:
+                log(
+                    self._log,
+                    logging.WARNING,
+                    "read_already",
+                    {"path": path},
+                    "File already read. Skipping: {path}".format(path=path),
+                )
+                return
+
+            self._read_files[relpath] = (relpath, "")
+
         if mozpath.dirname(relpath) == "js/src" and not config.substs.get(
             "JS_STANDALONE"
         ):
@@ -1234,6 +1236,15 @@ class BuildReader(object):
             if not descend:
                 continue
 
+            child_relpath = mozpath.relpath(child_path, self.config.topsrcdir)
+
+            if child_relpath in self._read_files:
+                (prev_parent, prev_path) = self._read_files[child_relpath]
+                raise Exception(
+                    f"File already read. A directory should not be added to DIRS twice: {child_relpath} is referred from {prev_parent} as '{prev_path}', and {relpath} as '{path}'"
+                )
+            self._read_files[child_relpath] = (relpath, path)
+
             for res in self.read_mozbuild(
                 child_path, context.config, metadata=child_metadata
             ):
diff --git a/python/mozbuild/mozbuild/test/frontend/test_reader.py b/python/mozbuild/mozbuild/test/frontend/test_reader.py
index a15bb15d7ec5..91d453a21fc6 100644
--- a/python/mozbuild/mozbuild/test/frontend/test_reader.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_reader.py
@@ -83,12 +83,20 @@ class TestBuildReader(unittest.TestCase):
         contexts = list(reader.read_topsrcdir())
         self.assertEqual(len(contexts), 3)
 
-    def test_repeated_dirs_ignored(self):
-        # Ensure repeated directories are ignored.
+    def test_repeated_dirs_error(self):
         reader = self.reader("traversal-repeated-dirs")
 
-        contexts = list(reader.read_topsrcdir())
-        self.assertEqual(len(contexts), 3)
+        with self.assertRaises(BuildReaderError) as bre:
+            list(reader.read_topsrcdir())
+
+        e = bre.exception
+        self.assertEqual(
+            e.actual_file, self.file_path("traversal-repeated-dirs", "bar", "moz.build")
+        )
+        self.assertIn(
+            "File already read. A directory should not be added to DIRS twice: foo/moz.build is referred from moz.build as 'foo', and bar/moz.build as '../foo'",
+            str(e),
+        )
 
     def test_outside_topsrcdir(self):
         # References to directories outside the topsrcdir should fail.