search

ansys / python-installer-qt-gui

Python QT app for installing Python
https://installer.docs.pyansys.com/
MIT License
30 stars 5 forks source link

fix: Windows DLL load issues related to PyInstaller #377

Closed RobPasMue closed 2 weeks ago

RobPasMue commented 2 weeks ago

Closes #376

RobPasMue commented 2 weeks ago

@phx-mkoninckx could you give it a try to the executable that gets generated from the workflow? We are resetting the DLL loading as suggested in the PyInstaller documentation now. Also took the opportunity to revert the hack we implemented for the paths.

RobPasMue commented 2 weeks ago

(Linux failing build is expected and under investigation by @tusharbana-ansys)

phx-mkoninckx commented 2 weeks ago

@RobPasMue

This does seem to resolve the specific issue I was looking at when I reported #376. However, I'm concerned about the revert of #344. The fix in #344 did actually have some impact; without it, PATH includes the _internal and _internal\PySide6 subdirectories from the directory where the Ansys Python Manager exists on disk:

(bug_repro) C:\Users\Admin>echo %PATH%
C:\Users\Admin\.ansys_python_venvs\bug_repro\Scripts;C:\Users\Admin\python-installer-qt-gui\dist\ansys_python_manager\_internal\PySide6;C:\Users\Admin\python-installer-qt-gui\dist\ansys_python_manager\_internal;C:\Program Files\Microsoft\jdk-11.0.18.10-hotspot\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Program Files\Git\cmd;C:\Users\Admin\AppData\Local\Programs\Python\Python311\Scripts\;C:\Users\Admin\AppData\Local\Programs\Python\Python311\;C:\Users\Admin\AppData\Local\Microsoft\WindowsApps;

Overall, this is less concerning than the impact of calling SetDllDirectory, since:

  1. Generally, DLLs are located by searching the PATH as a last-ditch effort. This means that these alterations to the PATH are, practically, somewhat less likely to impact most applications, since most native Windows apps don't depend on finding their DLLs on the PATH and instead expect to find them in the same directory as the application. Or, if they do depend on the PATH to load certain DLLs, they are probably putting their expected locations on the PATH first before attempting such a load. (This is the case for ModelCenter -- the DOE plug-in is run out-of-process, and when ModelCenter Desktop launches the DOE plug-in, it adds a centralized installation directory for the oSL algorithms to the subprocess' PATH as the first item, so the presence of the _internal directory here winds up not mattering).
  2. When attempting to debug issues caused by alterations to the PATH, it's somewhat easier to tell what's going on and deal with it.

However, if #344 was really the fix for #343, then reverting it could cause #343 to regress?

RobPasMue commented 2 weeks ago

Good point @phx-mkoninckx - I just checked it with the dev version and saw it was no longer in the PATH, buuut... that is not true if you used the full solution (executable on workflow). Reapplying the change. Thanks a lot for all the explanations @phx-mkoninckx!!

RobPasMue commented 2 weeks ago

Let's do that on a follow up PR - merging!