fix: detect available Linux terminal emulator instead of hardcoding gnome-terminal - #622
Open
tusharbana-ansys wants to merge 1 commit into
Open
fix: detect available Linux terminal emulator instead of hardcoding gnome-terminal#622tusharbana-ansys wants to merge 1 commit into
tusharbana-ansys wants to merge 1 commit into
Conversation
…nome-terminal (#591) - Add find_linux_terminal() to detect gnome-terminal, konsole, xfce4-terminal, mate-terminal, tilix, xterm, or x-terminal-emulator via shutil.which - Raise a clear NoLinuxTerminalError with actionable guidance when none are found, instead of silently doing nothing (the root cause of #591 on WSL, which ships no terminal emulator by default) - Rewrite execute_linux_command to use subprocess argv lists instead of os.system, fixing a latent shell-quoting bug - Surface terminal-launch failures to the user via show_error() in installed_table.py (Console/launch actions), uninstall.py (uninstall flow), and create_virtual_environment.py (venv creation), instead of failing silently or showing a misleading success dialog after a failure - Fix find_miniforge_linux() silently failing to detect system conda (invalid shell pipe passed to subprocess without shell=True) - Fix Ubuntu prerequisites (docs + installer.sh) requiring the full gnome desktop metapackage instead of just gnome-terminal - Add regression tests for terminal detection and the no-terminal error path
Contributor
Author
|
Although automated tests have passed, I am still testing this manually on each supported distro. Please do not merge yet. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #591 — Ansys Python Manager silently did nothing when clicking any action (Console, Install, Create venv, Uninstall, etc.) on Linux/WSL systems that don't have
gnome-terminalinstalled, which is the common case on WSL.Root cause
execute_linux_command()inlinux_functions.pyhardcodedgnome-terminalfor every Linux command execution, via a bareos.system(...)call whose exit status was never checked.gnome-terminalby default, so the command silently failed with no feedback to the user.installer.sh) told users to install the entiregnomedesktop metapackage instead of justgnome-terminal.Changes
linux_functions.py):find_linux_terminal(), which detects the first available terminal emulator viashutil.which()from a prioritized list:gnome-terminal,konsole,xfce4-terminal,mate-terminal,tilix,xterm,x-terminal-emulator.execute_linux_command()now uses whichever terminal is found, building the command withsubprocessargv lists instead ofos.system()(also fixes a latent shell-quoting bug from the old string-interpolated approach).NoLinuxTerminalErrorwith a clear, actionable message (e.g.sudo apt-get install xterm) when no terminal emulator is available, instead of failing silently.installed_table.py: Console/launch/package-management actions now show an error dialog viashow_error()on failure.uninstall.py: the uninstall confirmation flow previously had no error handling at all around the terminal launch — now wrapped and surfaced viashow_error().create_virtual_environment.py: venv creation previously showed a failure dialog and then unconditionally also showed a "successfully created" dialog right after — fixed to stop after the failure dialog.find_miniforge_linux()passed a shell pipe (printenv | grep ...) tosubprocess.check_output()withoutshell=True, so it always silently failed to detect system conda. Replaced with a directos.environ["CONDA_PYTHON_EXE"]lookup.gnome-terminal(not the fullgnomedesktop), matching what CentOS9/RHEL9/Fedora docs already said, plus a note recommending a lightweightxterminstall for WSL.tests/test_linux_functions.py).Testing
pytest tests/test_linux_functions.py tests/test_auto_updater.py tests/test_metadata.py— all passing.xterm) is installed — for the Console action, virtual environment creation, and the uninstall flow.Notes / follow-ups (out of scope for this PR)
execute_linux_command()only verifies that the terminal launched, not that the wrapped script succeeded — pre-existing design gap (also applies to the Windows code path), not something we changed here.