Fix empty .start file logic bug

* The presence of a readable but empty .start file should suppress import lines in .pth files.
* Slightly cleaner tearDown()
* Slightly cleaner filename calculation
* Add a test to check that two .start files containing the same entry point are not deduplicated.

Author: warsaw

Lib/site.py

@@ -155,31 +155,32 @@ def _init_pathinfo():
 def _read_pthstart_file(sitedir, name, suffix):
     """Parse a .start or .pth file and return (lines, filename).
 
-    Always returns a 2-tuple.  On failure (hidden, unreadable, etc.),
-    returns ([], filename) so callers can proceed without checking.
+    On success, ``lines`` is a (possibly empty) list of the file's lines.
+    On failure (file missing, hidden, unreadable, or .start with bad
+    encoding), ``lines`` is ``None`` so callers can distinguish a
+    successfully-read empty file from one that could not be read.
     """
-    content = ""
     filename = os.path.join(sitedir, name)
     _trace(f"Reading startup configuration file: {filename}")
 
     try:
         st = os.lstat(filename)
     except OSError as exc:
         _print_error(f"Cannot stat {filename!r}", exc)
-        return [], filename
+        return None, filename
 
     if ((getattr(st, 'st_flags', 0) & stat.UF_HIDDEN) or
         (getattr(st, 'st_file_attributes', 0) & stat.FILE_ATTRIBUTE_HIDDEN)):
         _trace(f"Skipping hidden {suffix} file: {filename!r}")
-        return [], filename
+        return None, filename
 
     _trace(f"Processing {suffix} file: {filename!r}")
     try:
         with io.open_code(filename) as f:
             raw_content = f.read()
     except OSError as exc:
         _print_error(f"Cannot read {filename!r}", exc)
-        return [], filename
+        return None, filename
 
     try:
         # Accept BOM markers in .start and .pth files as we do in source files (Windows PowerShell
@@ -193,7 +194,7 @@ def _read_pthstart_file(sitedir, name, suffix):
             content = raw_content.decode(locale.getencoding())
             _trace(f"Using fallback encoding {locale.getencoding()!r}")
         else:
-            return [], filename
+            return None, filename
 
     return content.splitlines(), filename
 
@@ -205,6 +206,8 @@ def _read_pth_file(sitedir, name, known_paths):
     file (PEP 829).
     """
     lines, filename = _read_pthstart_file(sitedir, name, ".pth")
+    if lines is None:
+        return
 
     for n, line in enumerate(lines, 1):
         line = line.strip()
@@ -236,6 +239,14 @@ def _read_pth_file(sitedir, name, known_paths):
 def _read_start_file(sitedir, name):
     """Parse a .start file and return a list of entry point strings."""
     lines, filename = _read_pthstart_file(sitedir, name, ".start")
+    if lines is None:
+        return
+
+    # PEP 829: the *presence* of a matching .start file disables `import`
+    # line processing in the matched .pth file, regardless of whether the
+    # .start file produced any entry points.  Register the filename as a
+    # key now so an empty (or comment-only) .start file still suppresses.
+    entrypoints = _pending_entrypoints.setdefault(filename, [])
 
     for n, line in enumerate(lines, 1):
         line = line.strip()
@@ -248,7 +259,7 @@ def _read_start_file(sitedir, name):
                    f"skipping invalid entry point: {line}")
             continue
 
-        _pending_entrypoints.setdefault(filename, []).append(line)
+        entrypoints.append(line)
 
 
 def _extend_syspath():

Lib/test/test_site.py

@@ -935,24 +935,21 @@ def setUp(self):
         site._pending_importexecs.clear()
 
     def tearDown(self):
-        site._pending_entrypoints.clear()
-        site._pending_entrypoints.update(self.saved_entrypoints)
-        site._pending_syspaths.clear()
-        site._pending_syspaths.update(self.saved_syspaths)
-        site._pending_importexecs.clear()
-        site._pending_importexecs.update(self.saved_importexecs)
+        site._pending_entrypoints = self.saved_entrypoints.copy()
+        site._pending_syspaths = self.saved_syspaths.copy()
+        site._pending_importexecs = self.saved_importexecs.copy()
 
     def _make_start(self, content, name='testpkg'):
         """Write a <name>.start file and return its basename."""
-        basename = name + '.start'
+        basename = f"{name}.start"
         filepath = os.path.join(self.tmpdir, basename)
         with open(filepath, 'w', encoding='utf-8') as f:
             f.write(content)
         return basename
 
     def _make_pth(self, content, name='testpkg'):
         """Write a <name>.pth file and return its basename."""
-        basename = name + '.pth'
+        basename = f"{name}.pth"
         filepath = os.path.join(self.tmpdir, basename)
         with open(filepath, 'w', encoding='utf-8') as f:
             f.write(content)
@@ -966,6 +963,9 @@ def _all_entrypoints(self):
                 result.append((filename, entry))
         return result
 
+    def _just_entrypoints(self):
+        return [entry for filename, entry in self._all_entrypoints()]
+
     # --- _read_start_file tests ---
 
     def test_read_start_file_basic(self):
@@ -995,14 +995,21 @@ def test_read_start_file_missing_colon_skipped(self):
         self.assertEqual(site._pending_entrypoints[fullname], ['os.path:join'])
 
     def test_read_start_file_empty(self):
+        # PEP 829: an empty .start file is still registered as present
+        # (with an empty entry-point list) so that it suppresses `import`
+        # lines in any matching .pth file.
         self._make_start("", name='foo')
         site._read_start_file(self.sitedir, 'foo.start')
-        self.assertEqual(site._pending_entrypoints, {})
+        fullname = os.path.join(self.sitedir, 'foo.start')
+        self.assertEqual(site._pending_entrypoints, {fullname: []})
 
     def test_read_start_file_comments_only(self):
+        # As with an empty file, a comments-only .start file is registered
+        # as present so it can suppress matching .pth `import` lines.
         self._make_start("# just a comment\n# another\n", name='foo')
         site._read_start_file(self.sitedir, 'foo.start')
-        self.assertEqual(site._pending_entrypoints, {})
+        fullname = os.path.join(self.sitedir, 'foo.start')
+        self.assertEqual(site._pending_entrypoints, {fullname: []})
 
     def test_read_start_file_nonexistent(self):
         with captured_stderr():
@@ -1026,6 +1033,14 @@ def test_read_start_file_duplicates_not_deduplicated(self):
         self.assertEqual(site._pending_entrypoints[fullname],
                          ['os.path:join', 'os.path:join'])
 
+    def test_two_start_files_with_duplicates_not_deduplicated(self):
+        self._make_start("os.path:join", name="foo")
+        self._make_start("os.path:join", name="bar")
+        site._read_start_file(self.sitedir, 'foo.start')
+        site._read_start_file(self.sitedir, 'bar.start')
+        self.assertEqual(self._just_entrypoints(),
+                         ['os.path:join', 'os.path:join'])
+
     # --- _read_pth_file tests ---
 
     def test_read_pth_file_paths(self):
@@ -1060,7 +1075,7 @@ def test_read_pth_file_deduplication(self):
         all_dirs = []
         for dirs in site._pending_syspaths.values():
             all_dirs.extend(dirs)
-        self.assertEqual(all_dirs.count(subdir), 1)
+        self.assertEqual(all_dirs, [subdir])
 
     def test_read_pth_file_bad_line_continues(self):
         # PEP 829: errors on individual lines don't abort the file.
@@ -1164,6 +1179,27 @@ def test_exec_imports_not_suppressed_by_different_start(self):
         # Should execute the import line without error.
         site._exec_imports()
 
+    def test_exec_imports_suppressed_by_empty_matching_start(self):
+        self._make_start("", name='foo')
+        self._make_pth("import epmod; epmod.startup()", name='foo')
+        mod_dir = os.path.join(self.sitedir, 'epmod')
+        os.mkdir(mod_dir)
+        init_file = os.path.join(mod_dir, '__init__.py')
+        with open(init_file, 'w') as f:
+            f.write("""\
+called = False
+def startup():
+    global called
+    called = True
+""")
+        sys.path.insert(0, self.sitedir)
+        self.addCleanup(sys.modules.pop, 'epmod', None)
+        site._read_pth_file(self.sitedir, 'foo.pth', set())
+        site._read_start_file(self.sitedir, 'foo.start')
+        site._exec_imports()
+        import epmod
+        self.assertFalse(epmod.called)
+
     # --- _extend_syspath tests ---
 
     def test_extend_syspath_existing_dir(self):