[mlir][python] stop initialization on ImportError

[ashay-github]

Summary

An _mlirRegisterEverything.*.so file from an old build that referenced
MLIRPythonExtension.RegisterEverything, but which no longer references
that extension in a new build, causes runtime errors in the new build
like:

ImportError: _mlirRegisterEverything.cpython-38-x86_64-linux-gnu.so: undefined symbol: mlirRegisterAllPasses

The error occurs because the MLIR Python binding tries to dynamically
import the _mlirRegisterEverything module but the dynamic importer
fails since the new build no longer references
MLIRPythonExtension.RegisterEverything.

One possible solution is for the user to manually remove the
_mlirRegisterEverything.*.so file. This patch instead resolves the
problem in code by printing a waning if the module cannot be
imported.

[stellaraccident]

mlir/python/mlir/_mlir_libs/__init__.py

63

The downside listed is a big one, in my opinion. While I am sympathetic to the upgrade plight, I've found the silently swallowing errors love this is a bigger foot gun.

Can you split the excpetion handler? Keep ModuleNotFound first, then add another one for ImportError. In that one, log an error with exc_info=True with a very draconian message like "error importing mlir initializer {module_name}. This may happen in unclean incremental builds but is likely a real bug if encountered elsewhere and the MLIR Python API may not function."

[ashay-github]

Now printing the exception to the console along with a message about possibly unclean incremental build upon encountering an ImportError exception.

[ashay-github]

mlir/python/mlir/_mlir_libs/__init__.py

63

I've found the silently swallowing errors love this is a bigger foot gun

I agree, and thanks for catching that. I updated the exception handler to print the message and abort execution.

[stellaraccident]

mlir/python/mlir/_mlir_libs/__init__.py

71

Let's not do the sys.exit. that is a really mean thing to do in a library. I think it is fine to return false here and let things go with the error.

I would also downgrade the message to warning.

[ashay-github]

Downgraded to a warning, also removed sys.exit(1).

[stellaraccident]

Also note https://reviews.llvm.org/D134812 in flight. I think that one should go in first

[ashay-github]

Thanks for the heads up! I'll wait until it lands and rebase this patch (and update it accordingly).

[stellaraccident]

Do you need help committing? The other one does - so if you both do, I can do final fix-ups as I'm landing both.

[ashay-github]

I have commit privilege, but feel free to commit both patches if that makes things easier! :)

[stellaraccident]

Go ahead and land at your convenience. I'll land the other one once you do (busy and not going to get to it today)

mlir/python/mlir/_mlir_libs/__init__.py

def _site_initialize():
   import logging
   from ._mlir import ir
   registry = ir.DialectRegistry()
   post_init_hooks = []
 
   def process_initializer_module(module_name):
     try:
       m = importlib.import_module(f".{module_name}", __name__)
     except ModuleNotFoundError:
       return False
+    except ImportError:
+      message = (f"Error importing mlir initializer {module_name}. This may "
+      "happen in unclean incremental builds but is likely a real bug if "
+      "encountered otherwise and the MLIR Python API may not function.")
+      logging.warning(message, exc_info=True)
 
     logging.debug("Initializing MLIR with module: %s", module_name)
     if hasattr(m, "register_dialects"):
       logging.debug("Registering dialects from initializer %r", m)
       m.register_dialects(registry)
     if hasattr(m, "context_init_hook"):
       logging.debug("Adding context init hook from %r", m)
       post_init_hooks.append(m.context_init_hook)
     return True