pythonnet · lostmsu · Aug 19, 2020 · Mar 4, 2020
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,9 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
 
 ### Changed
 -   Drop support for Python 2
+-   `clr.AddReference` may now throw errors besides `FileNotFoundException`, that provide more
+details about the cause of the failure
+-   `clr.AddReference` no longer adds ".dll" implicitly
 
 ### Fixed
 

diff --git a/src/embed_tests/pyimport.cs b/src/embed_tests/pyimport.cs
@@ -38,8 +38,8 @@ public void SetUp()
             string testPath = Path.Combine(TestContext.CurrentContext.TestDirectory, s);
 
             IntPtr str = Runtime.Runtime.PyString_FromString(testPath);
-            IntPtr path = Runtime.Runtime.PySys_GetObject("path");
-            Runtime.Runtime.PyList_Append(new BorrowedReference(path), str);
+            BorrowedReference path = Runtime.Runtime.PySys_GetObject("path");
+            Runtime.Runtime.PyList_Append(path, str);
         }
 
         [TearDown]
@@ -83,5 +83,28 @@ public void TestCastGlobalVar()
             Assert.AreEqual("2", foo.FOO.ToString());
             Assert.AreEqual("2", foo.test_foo().ToString());
         }
+
+        [Test]
+        public void BadAssembly()
+        {
+            string path;
+            if (Python.Runtime.Runtime.IsWindows)
+            {
+                path = @"C:\Windows\System32\kernel32.dll";
+            }
+            else
+            {
+                Assert.Pass("TODO: add bad assembly location for other platforms");
+                return;
+            }
+
+            string code = $@"
+import clr
+clr.AddReference('{path}')
+";
+
+            var error = Assert.Throws<PythonException>(() => PythonEngine.Exec(code));
+            Assert.AreEqual(nameof(FileLoadException), error.PythonTypeName);
+        }
     }
 }
diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs
@@ -137,7 +137,7 @@ private static Assembly ResolveHandler(object ob, ResolveEventArgs args)
         /// </summary>
         internal static void UpdatePath()
         {
-            IntPtr list = Runtime.PySys_GetObject("path");
+            BorrowedReference list = Runtime.PySys_GetObject("path");
             var count = Runtime.PyList_Size(list);
             if (count != pypath.Count)
             {
@@ -199,19 +199,14 @@ public static string FindAssembly(string name)
         /// </summary>
         public static Assembly LoadAssembly(string name)
         {
-            Assembly assembly = null;
             try
             {
-                assembly = Assembly.Load(name);
+                return Assembly.Load(name);
             }
-            catch (Exception)
+            catch (FileNotFoundException)
             {
-                //if (!(e is System.IO.FileNotFoundException))
-                //{
-                //    throw;
-                //}
+                return null;
             }
-            return assembly;
         }
 
 
@@ -221,18 +216,8 @@ public static Assembly LoadAssembly(string name)
         public static Assembly LoadAssemblyPath(string name)
         {
             string path = FindAssembly(name);
-            Assembly assembly = null;
-            if (path != null)
-            {
-                try
-                {
-                    assembly = Assembly.LoadFrom(path);
-                }
-                catch (Exception)
-                {
-                }
-            }
-            return assembly;
+            if (path == null) return null;
+            return Assembly.LoadFrom(path);
         }
 
         /// <summary>
@@ -242,25 +227,14 @@ public static Assembly LoadAssemblyPath(string name)
         /// <returns></returns>
         public static Assembly LoadAssemblyFullPath(string name)
         {
-            Assembly assembly = null;
             if (Path.IsPathRooted(name))
             {
-                if (!Path.HasExtension(name))
-                {
-                    name = name + ".dll";
-                }
                 if (File.Exists(name))
                 {
-                    try
-                    {
-                        assembly = Assembly.LoadFrom(name);
-                    }
-                    catch (Exception)
-                    {
-                    }
+                    return Assembly.LoadFrom(name);
                 }
             }
-            return assembly;
+            return null;
         }
 
         /// <summary>
@@ -291,7 +265,7 @@ public static Assembly FindLoadedAssembly(string name)
         /// actually loads an assembly.
         /// Call ONLY for namespaces that HAVE NOT been cached yet.
         /// </remarks>
-        public static bool LoadImplicit(string name, bool warn = true)
+        public static bool LoadImplicit(string name, Action<Exception> assemblyLoadErrorHandler, bool warn = true)
         {
             string[] names = name.Split('.');
             var loaded = false;
@@ -308,14 +282,23 @@ public static bool LoadImplicit(string name, bool warn = true)
                         assembliesSet = new HashSet<Assembly>(AppDomain.CurrentDomain.GetAssemblies());
                     }
                     Assembly a = FindLoadedAssembly(s);
-                    if (a == null)
-                    {
-                        a = LoadAssemblyPath(s);
-                    }
-                    if (a == null)
+                    try
                     {
-                        a = LoadAssembly(s);
+                        if (a == null)
+                        {
+                            a = LoadAssemblyPath(s);
+                        }
+
+                        if (a == null)
+                        {
+                            a = LoadAssembly(s);
+                        }
                     }
+                    catch (FileLoadException e) { assemblyLoadErrorHandler(e); }
+                    catch (BadImageFormatException e) { assemblyLoadErrorHandler(e); }
+                    catch (System.Security.SecurityException e) { assemblyLoadErrorHandler(e); }
+                    catch (PathTooLongException e) { assemblyLoadErrorHandler(e); }
+
                     if (a != null && !assembliesSet.Contains(a))
                     {
                         loaded = true;

diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs
@@ -254,9 +254,9 @@ public static void SetError(IntPtr ob, string value)
         /// Sets the current Python exception given a Python object.
         /// This is a wrapper for the Python PyErr_SetObject call.
         /// </remarks>
-        public static void SetError(IntPtr ob, IntPtr value)
+        public static void SetError(IntPtr type, IntPtr exceptionObject)
         {
-            Runtime.PyErr_SetObject(ob, value);
+            Runtime.PyErr_SetObject(new BorrowedReference(type), new BorrowedReference(exceptionObject));
         }
 
         /// <summary>
@@ -286,7 +286,7 @@ public static void SetError(Exception e)
 
             IntPtr op = CLRObject.GetInstHandle(e);
             IntPtr etype = Runtime.PyObject_GetAttrString(op, "__class__");
-            Runtime.PyErr_SetObject(etype, op);
+            Runtime.PyErr_SetObject(new BorrowedReference(etype), new BorrowedReference(op));
             Runtime.XDecref(etype);
             Runtime.XDecref(op);
         }

diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Runtime.InteropServices;
 
 namespace Python.Runtime
@@ -222,22 +223,12 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
             // Check these BEFORE the built-in import runs; may as well
             // do the Incref()ed return here, since we've already found
             // the module.
-            if (mod_name == "clr")
+            if (mod_name == "clr" || mod_name == "CLR")
             {
-                IntPtr clr_module = GetCLRModule(fromList);
-                if (clr_module != IntPtr.Zero)
+                if (mod_name == "CLR")
                 {
-                    IntPtr sys_modules = Runtime.PyImport_GetModuleDict();
-                    if (sys_modules != IntPtr.Zero)
-                    {
-                        Runtime.PyDict_SetItemString(sys_modules, "clr", clr_module);
-                    }
+                    Exceptions.deprecation("The CLR module is deprecated. Please use 'clr'.");
                 }
-                return clr_module;
-            }
-            if (mod_name == "CLR")
-            {
-                Exceptions.deprecation("The CLR module is deprecated. Please use 'clr'.");
                 IntPtr clr_module = GetCLRModule(fromList);
                 if (clr_module != IntPtr.Zero)
                 {
@@ -249,8 +240,10 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
                 }
                 return clr_module;
             }
+
             string realname = mod_name;
             string clr_prefix = null;
+
             if (mod_name.StartsWith("CLR."))
             {
                 clr_prefix = "CLR."; // prepend when adding the module to sys.modules
@@ -312,11 +305,22 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)
             AssemblyManager.UpdatePath();
             if (!AssemblyManager.IsValidNamespace(realname))
             {
-                if (!AssemblyManager.LoadImplicit(realname))
+                var loadExceptions = new List<Exception>();
+                if (!AssemblyManager.LoadImplicit(realname, assemblyLoadErrorHandler: loadExceptions.Add))
                 {
                     // May be called when a module being imported imports a module.
                     // In particular, I've seen decimal import copy import org.python.core
-                    return Runtime.PyObject_Call(py_import, args, kw);
+                    IntPtr importResult = Runtime.PyObject_Call(py_import, args, kw);
+                    // TODO: use ModuleNotFoundError in Python 3.6+
+                    if (importResult == IntPtr.Zero && loadExceptions.Count > 0
+                        && Exceptions.ExceptionMatches(Exceptions.ImportError))
+                    {
+                        loadExceptions.Add(new PythonException());
+                        var importError = new PyObject(new BorrowedReference(Exceptions.ImportError));
+                        importError.SetAttr("__cause__", new AggregateException(loadExceptions).ToPython());
+                        Runtime.PyErr_SetObject(new BorrowedReference(Exceptions.ImportError), importError.Reference);
+                    }
+                    return importResult;
                 }
             }
 

diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs
@@ -291,8 +291,8 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth
                 IntPtr valueList = Runtime.PyDict_Values(kw);
                 for (int i = 0; i < pynkwargs; ++i)
                 {
-                    var keyStr = Runtime.GetManagedString(Runtime.PyList_GetItem(keylist, i));
-                    kwargDict[keyStr] = Runtime.PyList_GetItem(valueList, i).DangerousGetAddress();
+                    var keyStr = Runtime.GetManagedString(Runtime.PyList_GetItem(new BorrowedReference(keylist), i));
+                    kwargDict[keyStr] = Runtime.PyList_GetItem(new BorrowedReference(valueList), i).DangerousGetAddress();
                 }
                 Runtime.XDecref(keylist);
                 Runtime.XDecref(valueList);

diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs
@@ -119,7 +119,7 @@ public ManagedType GetAttribute(string name, bool guess)
             // cost. Ask the AssemblyManager to do implicit loading for each
             // of the steps in the qualified name, then try it again.
             bool ignore = name.StartsWith("__");
-            if (AssemblyManager.LoadImplicit(qname, !ignore))
+            if (AssemblyManager.LoadImplicit(qname, assemblyLoadErrorHandler: ImportWarning, !ignore))
             {
                 if (AssemblyManager.IsValidNamespace(qname))
                 {
@@ -161,6 +161,11 @@ public ManagedType GetAttribute(string name, bool guess)
             return null;
         }
 
+        static void ImportWarning(Exception exception)
+        {
+            Exceptions.warn(exception.ToString(), Exceptions.ImportWarning);
+        }
+
 
         /// <summary>
         /// Stores an attribute in the instance dict for future lookups.
@@ -365,7 +370,7 @@ internal void InitializePreload()
             if (interactive_preload)
             {
                 interactive_preload = false;
-                if (Runtime.PySys_GetObject("ps1") != IntPtr.Zero)
+                if (!Runtime.PySys_GetObject("ps1").IsNull)
                 {
                     preload = true;
                 }

diff --git a/src/runtime/pylist.cs b/src/runtime/pylist.cs
@@ -22,6 +22,11 @@ public PyList(IntPtr ptr) : base(ptr)
         {
         }
 
+        /// <summary>
+        /// Creates new <see cref="PyList"/> pointing to the same object, as the given reference.
+        /// </summary>
+        internal PyList(BorrowedReference reference) : base(reference) { }
+
 
         /// <summary>
         /// PyList Constructor

diff --git a/src/runtime/pysequence.cs b/src/runtime/pysequence.cs
@@ -16,6 +16,8 @@ protected PySequence(IntPtr ptr) : base(ptr)
         {
         }
 
+        internal PySequence(BorrowedReference reference) : base(reference) { }
+
         protected PySequence()
         {
         }

diff --git a/src/runtime/pytuple.cs b/src/runtime/pytuple.cs
@@ -22,6 +22,15 @@ public PyTuple(IntPtr ptr) : base(ptr)
         {
         }
 
+        /// <summary>
+        /// PyTuple Constructor
+        /// </summary>
+        /// <remarks>
+        /// Creates a new PyTuple from an existing object reference.
+        /// The object reference is not checked for type-correctness.
+        /// </remarks>
+        internal PyTuple(BorrowedReference reference) : base(reference) { }
+
 
         /// <summary>
         /// PyTuple Constructor
-Original file line number
+Diff line change
@@ Expand Up / @@ -16,6 +16,8 @@ protected PySequence(IntPtr ptr) : base(ptr) @@
             {
             }
+            internal PySequence(BorrowedReference reference) : base(reference) { }
             protected PySequence()
             {
             }
@@ Expand Down @@