Skip to content

Remove deprecated implicit assembly loading #1277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 14, 2020
Merged

Remove deprecated implicit assembly loading #1277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8a1b15
minor improvement to debugging experience
lostmsu Nov 11, 2020
1747b42
Remove deprecated implicit assembly loading
lostmsu Nov 11, 2020
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ details about the cause of the failure
- Fixed a bug where indexers could not be used if they were inherited
- Made it possible to use `__len__` also on `ICollection<>` interface objects

### Removed

- implicit assembly loading (you have to explicitly `clr.AddReference` before doing import)

## [2.5.0][] - 2020-06-14

This version improves performance on benchmarks significantly compared to 2.3.
Expand Down
69 changes: 0 additions & 69 deletions src/runtime/assemblymanager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,75 +252,6 @@ public static Assembly FindLoadedAssembly(string name)
return null;
}

/// <summary>
/// Given a qualified name of the form A.B.C.D, attempt to load
/// an assembly named after each of A.B.C.D, A.B.C, A.B, A. This
/// will only actually probe for the assembly once for each unique
/// namespace. Returns true if any assemblies were loaded.
/// </summary>
/// <remarks>
/// TODO item 3 "* Deprecate implicit loading of assemblies":
/// Set the fromFile flag if the name of the loaded assembly matches
/// the fully qualified name that was requested if the framework
/// actually loads an assembly.
/// Call ONLY for namespaces that HAVE NOT been cached yet.
/// </remarks>
public static bool LoadImplicit(string name, Action<Exception> assemblyLoadErrorHandler, bool warn = true)
{
string[] names = name.Split('.');
var loaded = false;
var s = "";
Assembly lastAssembly = null;
HashSet<Assembly> assembliesSet = null;
for (var i = 0; i < names.Length; i++)
{
s = i == 0 ? names[0] : s + "." + names[i];
if (!probed.ContainsKey(s))
{
if (assembliesSet == null)
{
assembliesSet = new HashSet<Assembly>(AppDomain.CurrentDomain.GetAssemblies());
}
Assembly a = FindLoadedAssembly(s);
try
{
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;
lastAssembly = a;
}
probed[s] = 1;
}
}

// Deprecation warning
if (warn && loaded)
{
string location = Path.GetFileNameWithoutExtension(lastAssembly.Location);
string deprWarning = "The module was found, but not in a referenced namespace.\n" +
$"Implicit loading is deprecated. Please use clr.AddReference('{location}').";
Exceptions.deprecation(deprWarning);
}

return loaded;
}


/// <summary>
/// Scans an assembly for exported namespaces, adding them to the
/// mapping of valid namespaces. Note that for a given namespace
Expand Down
32 changes: 0 additions & 32 deletions src/runtime/importhook.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,38 +310,6 @@ public static IntPtr __import__(IntPtr self, IntPtr args, IntPtr kw)

string[] names = realname.Split('.');

// Now we need to decide if the name refers to a CLR module,
// and may have to do an implicit load (for b/w compatibility)
// using the AssemblyManager. The assembly manager tries
// really hard not to use Python objects or APIs, because
// parts of it can run recursively and on strange threads.
//
// It does need an opportunity from time to time to check to
// see if sys.path has changed, in a context that is safe. Here
// we know we have the GIL, so we'll let it update if needed.

AssemblyManager.UpdatePath();
if (!AssemblyManager.IsValidNamespace(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
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;
}
}

// See if sys.modules for this interpreter already has the
// requested module. If so, just return the existing module.
IntPtr modules = Runtime.PyImport_GetModuleDict();
Expand Down
24 changes: 0 additions & 24 deletions src/runtime/moduleobject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,30 +118,6 @@ public ManagedType GetAttribute(string name, bool guess)
return c;
}

// This is a little repetitive, but it ensures that the right
// thing happens with implicit assembly loading at a reasonable
// 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, assemblyLoadErrorHandler: ImportWarning, !ignore))
{
if (AssemblyManager.IsValidNamespace(qname))
{
m = new ModuleObject(qname);
StoreAttribute(name, m);
m.DecrRefCount();
return m;
}

type = AssemblyManager.LookupTypes(qname).FirstOrDefault(t => t.IsPublic);
if (type != null)
{
c = ClassManager.GetClass(type);
StoreAttribute(name, c);
return c;
}
}

// We didn't find the name, so we may need to see if there is a
// generic type with this base name. If so, we'll go ahead and
// return it. Note that we store the mapping of the unmangled
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/pythonexception.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ public static bool Matches(IntPtr ob)
return Runtime.PyErr_ExceptionMatches(ob) != 0;
}

[System.Diagnostics.DebuggerHidden]
public static void ThrowIfIsNull(IntPtr ob)
{
if (ob == IntPtr.Zero)
Expand All @@ -258,6 +259,7 @@ public static void ThrowIfIsNull(IntPtr ob)
}
}

[System.Diagnostics.DebuggerHidden]
internal static void ThrowIfIsNull(BorrowedReference reference)
{
if (reference.IsNull)
Expand All @@ -266,6 +268,7 @@ internal static void ThrowIfIsNull(BorrowedReference reference)
}
}

[System.Diagnostics.DebuggerHidden]
public static void ThrowIfIsNotZero(int value)
{
if (value != 0)
Expand Down
3 changes: 3 additions & 0 deletions src/tests/test_array.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

"""Test support for managed arrays."""

import clr
import Python.Test as Test
import System
import pytest
Expand Down Expand Up @@ -1143,6 +1144,8 @@ def test_boxed_value_type_mutation_result():
# to accidentally write code like the following which is not really
# mutating value types in-place but changing boxed copies.

clr.AddReference('System.Drawing')

from System.Drawing import Point
from System import Array

Expand Down
4 changes: 4 additions & 0 deletions src/tests/test_class.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

"""Test CLR class support."""

import clr
import Python.Test as Test
import System
import pytest
Expand Down Expand Up @@ -124,6 +125,9 @@ def __init__(self, v):

def test_struct_construction():
"""Test construction of structs."""

clr.AddReference('System.Drawing')

from System.Drawing import Point

p = Point()
Expand Down
3 changes: 3 additions & 0 deletions src/tests/test_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

"""Backward-compatibility tests for deprecated features."""

import clr
import types

import pytest
Expand Down Expand Up @@ -137,6 +138,8 @@ def test_dotted_name_import_from_with_alias():

def test_from_module_import_star():
"""Test from module import * behavior."""
clr.AddReference('System.Management')

count = len(locals().keys())
m = __import__('CLR.System.Management', globals(), locals(), ['*'])
assert m.__name__ == 'System.Management'
Expand Down
14 changes: 7 additions & 7 deletions src/tests/test_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ def test_dotted_name_import_from_with_alias():

def test_from_module_import_star():
"""Test from module import * behavior."""
clr.AddReference('System.Xml')

count = len(locals().keys())
m = __import__('System.Xml', globals(), locals(), ['*'])
assert m.__name__ == 'System.Xml'
Expand All @@ -201,16 +203,14 @@ def test_from_module_import_star():

def test_implicit_assembly_load():
"""Test implicit assembly loading via import."""
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
with pytest.raises(ImportError):
# MS.Build should not have been added as a reference yet
# (and should exist for mono)

# should trigger a DeprecationWarning as Microsoft.Build hasn't
# been added as a reference yet (and should exist for mono)
# The implicit behavior has been disabled in 3.0
# therefore this should fail
import Microsoft.Build

assert len(w) == 1
assert isinstance(w[0].message, DeprecationWarning)

with warnings.catch_warnings(record=True) as w:
clr.AddReference("System.Windows.Forms")
import System.Windows.Forms as Forms
Expand Down