Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions Lib/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,14 @@ def addsitedir(sitedir, known_paths=None):
if not sitedircase in known_paths:
sys.path.append(sitedir) # Add path component
known_paths.add(sitedircase)
try:
names = os.listdir(sitedir)
except OSError:
return
names = [name for name in names
if name.endswith(".pth") and not name.startswith(".")]
for name in sorted(names):
addpackage(sitedir, name, known_paths)
try:
names = os.listdir(sitedir)
except OSError:
return
names = [name for name in names
if name.endswith(".pth") and not name.startswith(".")]
for name in sorted(names):
addpackage(sitedir, name, known_paths)
if reset:
known_paths = None
return known_paths
Expand Down
24 changes: 24 additions & 0 deletions Lib/test/test_site.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ def pth_file_tests(self, pth_file):
"%s not in sys.modules" % pth_file.imported)
self.assertIn(site.makepath(pth_file.good_dir_path)[0], sys.path)
self.assertFalse(os.path.exists(pth_file.bad_dir_path))
self.assertFalse(os.path.exists(pth_file.idempotent_fail_path))

def test_addpackage(self):
# Make sure addpackage() imports if the line starts with 'import',
Expand Down Expand Up @@ -199,6 +200,19 @@ def test_addsitedir(self):
finally:
pth_file.cleanup()

def test_addsitedir_idempotent(self):
pth_file = PthFile()
pth_file.cleanup(prep=True)

try:
pth_file.create()
dirs = set()
dirs = site.addsitedir(pth_file.base_dir, dirs)
dirs = site.addsitedir(pth_file.base_dir, dirs)
self.pth_file_tests(pth_file)
finally:
pth_file.cleanup()

def test_addsitedir_dotfile(self):
pth_file = PthFile('.dotfile')
pth_file.cleanup(prep=True)
Expand Down Expand Up @@ -414,6 +428,7 @@ def __init__(self, filename_base=TESTFN, imported="time",
self.bad_dirname = bad_dirname
self.good_dir_path = os.path.join(self.base_dir, self.good_dirname)
self.bad_dir_path = os.path.join(self.base_dir, self.bad_dirname)
self.idempotent_fail_path = os.path.join(self.base_dir, 'idempotent')

def create(self):
"""Create a .pth file with a comment, blank lines, an ``import
Expand All @@ -430,6 +445,13 @@ def create(self):
try:
print("#import @bad module name", file=FILE)
print("\n", file=FILE)

PROG = f'''\
if {self.imported!r} in sys.modules:
open({self.idempotent_fail_path!r}, 'a+').close()
'''
print(f"import sys; exec({PROG!r})", file=FILE)
Comment on lines +449 to +453
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider this (zero effort because dedent is already imported)

Suggested change
PROG = f'''\
if {self.imported!r} in sys.modules:
open({self.idempotent_fail_path!r}, 'a+').close()
'''
print(f"import sys; exec({PROG!r})", file=FILE)
PROG = dedent(f'''\
if {self.imported!r} in sys.modules:
open({self.idempotent_fail_path!r}, 'a+').close()
''')
print(f"import sys; exec({PROG!r})", file=FILE)


print("import %s" % self.imported, file=FILE)
print(self.good_dirname, file=FILE)
print(self.bad_dirname, file=FILE)
Expand All @@ -454,6 +476,8 @@ def cleanup(self, prep=False):
os.rmdir(self.good_dir_path)
if os.path.exists(self.bad_dir_path):
os.rmdir(self.bad_dir_path)
if os.path.exists(self.idempotent_fail_path):
os.remove(self.idempotent_fail_path)

class ImportSideEffectTests(unittest.TestCase):
"""Test side-effects from importing 'site'."""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid re-executing ``.pth`` files when :meth:`site.addsitedir` is called for a known directory.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use the func role here, not the meth one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this from other parts in the docs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(while I do agree with you)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. Ok let's use func and we will make changes in a separate PR for elsewhere to match role semantics.

Loading