Fix PyPackageDir

Passing a module which the active Python (system or virtualenv)
cannot locate through the import machinery would cause SCons to fail
with an AttributError, because the result of the initial lookup was
used without checking for success. Now returns None for not found.
Manpage entry and docstring also updated.

Signed-off-by: Mats Wichmann <mats@linux.com>

5 files changed, 73 insertions, 25 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 30543e5..61ceb4a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -81,6 +81,10 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Fix bad typing in Action.py: process() and strfunction().
     - Add Pseudo() to global functions, had been omitted. Fixes #4474.
       The Pseudo manpage entry was updated to provide more clarity.
+    - The internal routine which implements the PyPackageDir function
+      would fail with an exception if called with a module which is
+      not found.  It will now return None.  Updated manpage entry and
+      docstring..
 
 
 RELEASE 4.6.0 -  Sun, 19 Nov 2023 17:22:20 -0700
diff --git a/RELEASE.txt b/RELEASE.txt
index 8afc3d9..e014974 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -54,6 +54,9 @@ FIXES
   make sure decoding of bytes doesn't fail.
 - Documentation indicated that both Pseudo() and env.Pseudo() were usable,
   but Pseudo() did not work; is now enabled.
+- PyPackageDir no longer fails if passed a module name which cannot be found,
+  now returns None.
+
 
 IMPROVEMENTS
 ------------
diff --git a/SCons/Environment.xml b/SCons/Environment.xml
index 94dab08..0014426 100644
--- a/SCons/Environment.xml
+++ b/SCons/Environment.xml
@@ -2880,20 +2880,41 @@ and &f-link-env-Prepend;.
 </arguments>
 <summary>
 <para>
-This returns a Directory Node similar to Dir.
-The python module / package is looked up and if located
-the directory is returned for the location.
-<parameter>modulename</parameter>
-Is a named python package / module to
-lookup the directory for it's location.
-</para>
-<para>
-If
-<parameter>modulename</parameter>
-is a list, SCons returns a list of Dir nodes.
+Finds the location of <parameter>modulename</parameter>,
+which can be a string or a sequence of strings,
+each representing the name of a &Python; module.
 Construction variables are expanded in
 <parameter>modulename</parameter>.
+Returns a Directory Node (see &f-link-Dir;),
+or a list of Directory Nodes if
+<parameter>modulename</parameter> is a sequence.
+<literal>None</literal> is returned for any module not found.
+</para>
+
+<para>
+When a Tool module which is installed as a
+&Python; module is used, you need
+to specify a <parameter>toolpath</parameter> argument to
+&f-link-Tool;,
+&f-link-Environment;
+or &f-link-Clone;,
+as tools outside the standard project locations
+(<filename>site_scons/site_tools</filename>)
+will not be found otherwise.
+Using &f-PyPackageDir; allows this path to be
+discovered at runtime instead of hardcoding the path.
+</para>
+
+<para>
+Example:
 </para>
+
+<example_commands>
+env = Environment(
+    tools=["default", "ExampleTool"],
+    toolpath=[PyPackageDir("example_tool")]
+)
+</example_commands>
 </summary>
 </scons_function>
 
diff --git a/SCons/Node/FS.py b/SCons/Node/FS.py
index a5282e6..d1ffddb 100644
--- a/SCons/Node/FS.py
+++ b/SCons/Node/FS.py
@@ -1294,7 +1294,7 @@ class FS(LocalFS):
                 self.Root[''] = root
             return root
 
-    def _lookup(self, p, directory, fsclass, create: int=1):
+    def _lookup(self, p, directory, fsclass, create: bool = True):
         """
         The generic entry point for Node lookup with user-supplied data.
 
@@ -1430,7 +1430,7 @@ class FS(LocalFS):
 
         return root._lookup_abs(p, fsclass, create)
 
-    def Entry(self, name, directory = None, create: int = 1):
+    def Entry(self, name, directory = None, create: bool = True):
         """Look up or create a generic Entry node with the specified name.
         If the name is a relative path (begins with ./, ../, or a file
         name), then it is looked up relative to the supplied directory
@@ -1439,7 +1439,7 @@ class FS(LocalFS):
         """
         return self._lookup(name, directory, Entry, create)
 
-    def File(self, name, directory = None, create: int = 1):
+    def File(self, name, directory = None, create: bool = True):
         """Look up or create a File node with the specified name.  If
         the name is a relative path (begins with ./, ../, or a file name),
         then it is looked up relative to the supplied directory node,
@@ -1486,21 +1486,24 @@ class FS(LocalFS):
                 d = self.Dir(d)
             self.Top.addRepository(d)
 
-    def PyPackageDir(self, modulename):
-        r"""Locate the directory of a given python module name
+    def PyPackageDir(self, modulename) -> Optional[Dir]:
+        r"""Locate the directory of Python module *modulename*.
 
-        For example scons might resolve to
-        Windows: C:\Python27\Lib\site-packages\scons-2.5.1
-        Linux: /usr/lib/scons
+        For example 'SCons' might resolve to
+        Windows: C:\Python311\Lib\site-packages\SCons
+        Linux: /usr/lib64/python3.11/site-packages/SCons
 
-        This can be useful when we want to determine a toolpath based on a python module name"""
+        Can be used to determine a toolpath based on a Python module name.
 
-        dirpath = ''
-
-        # Python3 Code
+        This is the backend called by the public API function
+        :meth:`~Environment.Base.PyPackageDir`.
+        """
         modspec = importlib.util.find_spec(modulename)
-        dirpath = os.path.dirname(modspec.origin)
-        return self._lookup(dirpath, None, Dir, True)
+        if modspec:
+            origin = os.path.dirname(modspec.origin)
+            return self._lookup(origin, directory=None, fsclass=Dir, create=True)
+        else:
+            return None
 
 
     def variant_dir_target_climb(self, orig, dir, tail):
diff --git a/SCons/Node/FSTests.py b/SCons/Node/FSTests.py
index e2eb0af..e50f721 100644
--- a/SCons/Node/FSTests.py
+++ b/SCons/Node/FSTests.py
@@ -4046,6 +4046,23 @@ class AbsolutePathTestCase(unittest.TestCase):
             os.chdir(save_cwd)
 
 
+class PyPackageDir(unittest.TestCase):
+    def runTest(self) -> None:
+        """Test calling the PyPackageDir() method.
+
+        We don't want to mock the positive case here - there's
+        testing for that in E2E test test/Dir/PyPackageDir.
+        We're only making sure we don't die in the negative case
+        (module not found) and instead return None.
+        """
+        fs = SCons.Node.FS.FS('/')
+        try:
+            pkdir = fs.PyPackageDir("garglemod")
+        except AttributeError:
+            self.fail("non-existent module raised AttributeError")
+        self.assertIsNone(pkdir)
+
+
 if __name__ == "__main__":
     unittest.main()