pythonnet / clr-loader

Loader for different .NET runtimes
MIT License
32 stars 23 forks source link

macOS: get_coreclr fails if run in an already open Terminal window after a fresh dotnet install. Works on Windows. #24

Closed DareDevilDenis closed 2 years ago

DareDevilDenis commented 2 years ago

On macOS with pythonnet 3.0.0a2 installed (clr-loader 0.1.7) but without .NET core installed:

This results in the following exception:

  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/MyPackage/my_script.py", line 28, in <module>
    runtime_config_path = get_coreclr(r'C:\MyApp.runtimeconfig.json')
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/clr_loader/__init__.py", line 42, in get_coreclr
    dotnet_root = find_dotnet_root()
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/clr_loader/util/find.py", line 22, in find_dotnet_root
    raise RuntimeError("Can not determine dotnet root")
RuntimeError: Can not determine dotnet root

If a NEW Terminal window is opened then this error does not occur. However we should not have to open a new Terminal window for it to find the dotnet executable. Performing the same steps on Windows works fine - we don't need to open a new command window.

My application has the prerequisites that pythonnet and .NET runtime are first installed. I am getting reports from people that they have installed the prerequisites but it is still failing. They don't realise that they need to open a new Terminal window to get it working.

The reason for the exception seems to be in clr-loader/util/find.py there is a difference how it finds the "dotnet" executable between Windows and macOS:

def find_dotnet_root() -> str:
    dotnet_root = os.environ.get("DOTNET_ROOT", None)
    if dotnet_root is not None:
        return dotnet_root

    if sys.platform == "win32":
        # On Windows, the host library is stored separately from dotnet.exe for x86
        prog_files = os.environ.get("ProgramFiles")
        dotnet_root = os.path.join(prog_files, "dotnet")
        if os.path.isdir(dotnet_root):
            return dotnet_root

    # Try to discover dotnet from PATH otherwise
    dotnet_path = shutil.which("dotnet")
    if not dotnet_path:
        raise RuntimeError("Can not determine dotnet root")

To fix this, could macOS just look in /usr/local/share/dotnet ? (I'm don't have Linux so I'm not sure where dotnet gets installed to on Linux)

filmor commented 2 years ago

I'd be fine with special-casing this for macOS.

PRs are welcome, I have no mac to test this. The CI doesn't have this problem as it (properly, IMO) sets PATH and DOTNET_ROOT.

filmor commented 2 years ago

And by the way, the reason this is special-cased on Windows is that on this system one expects it to always install into Program Files and to not update the global PATH. On Unix systems, I expect things to be in PATH after installation. The error here happens because the profile (i.e. set of environment variables) is not automagically updated, you'd need to add that to your installation routine.

DareDevilDenis commented 2 years ago

Thanks @filmor

Based on your advice, we could add the following which would make it work on macOS:

def find_dotnet_root() -> str:
    dotnet_root = os.environ.get("DOTNET_ROOT", None)
    if dotnet_root is not None:
        return dotnet_root

    if sys.platform == "win32":
        # On Windows, the host library is stored separately from dotnet.exe for x86
        prog_files = os.environ.get("ProgramFiles")
        dotnet_root = os.path.join(prog_files, "dotnet")
        ## if os.path.isdir(dotnet_root):            MOVED BELOW
        ##    return dotnet_root                     MOVED BELOW

    ### START NEW 

    if sys.platform == "darwin":
        dotnet_root = r'/usr/local/share/dotnet'

    if os.path.isdir(dotnet_root):
        return dotnet_root

    ### END NEW

    # Try to discover dotnet from PATH otherwise
    dotnet_path = shutil.which("dotnet")
    if not dotnet_path:
        raise RuntimeError("Can not determine dotnet root")

I have a few questions:

filmor commented 2 years ago

DOTNET_ROOT is something that you can set yourself. That change seems fine to me, please create a PR. I don't want to set a default on Linux, on all reasonable Linux systems, you will have dotnet on the PATH. Your other questions should be directed at Microsoft :)

DareDevilDenis commented 2 years ago

Thanks @filmor. I've created the PR: https://github.com/pythonnet/clr-loader/pull/25

filmor commented 2 years ago

Thank you :)