From 2109f81102ffaa6d77e2c897cabaf4e245dacb32 Mon Sep 17 00:00:00 2001 From: Frantisek Hrbata Date: Tue, 17 Sep 2024 09:34:32 +0200 Subject: [PATCH 1/2] fix(tools): do not include rc scripts in export script The rc scripts should be included only when a new shell is started. At present, for bash and zsh, we are also including them in the export scripts, which is incorrect and may cause recursion. Closes https://github.com/espressif/esp-idf/issues/14584 Signed-off-by: Frantisek Hrbata --- tools/export_utils/shell_types.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tools/export_utils/shell_types.py b/tools/export_utils/shell_types.py index c1f0c4e690..9daf91774e 100644 --- a/tools/export_utils/shell_types.py +++ b/tools/export_utils/shell_types.py @@ -72,10 +72,6 @@ class UnixShell(Shell): # Basic POSIX shells does not support autocompletion return None - def init_file(self) -> None: - with open(self.script_file_path, 'w') as fd: - self.export_file(fd) - def export_file(self, fd: TextIO) -> None: fd.write(f'{self.deactivate_cmd}\n') for var, value in self.new_esp_idf_env.items(): @@ -87,7 +83,8 @@ class UnixShell(Shell): 'Go to the project directory and run:\n\n idf.py build"\n')) def export(self) -> None: - self.init_file() + with open(self.script_file_path, 'w') as fd: + self.export_file(fd) print(f'. {self.script_file_path}') def click_ver(self) -> int: @@ -188,6 +185,10 @@ class FishShell(UnixShell): stdout: str = run_cmd([sys.executable, conf.IDF_PY], env=env) return stdout + def init_file(self) -> None: + with open(self.script_file_path, 'w') as fd: + self.export_file(fd) + def spawn(self) -> None: self.init_file() new_env = os.environ.copy() From 6f3241a34b7c10e4e9d9a15137b1cd0c78fa9bb7 Mon Sep 17 00:00:00 2001 From: Frantisek Hrbata Date: Mon, 16 Sep 2024 16:16:38 +0200 Subject: [PATCH 2/2] fix(tools): Upgrade shell detection & simplify autocompletion Explicitly set shell type in export.sh for bash and zsh -Most of the issues reported with the updated export scripts are related to shell detection. Since we already know the shell type for commonly used ones like bash or zsh, we can pass the shell type directly to the activate script. This should hopefully resolve the shell detection problems for most users. This update also modifies the shell type detection to rely solely on psutil.Process().cmdline() rather than psutil.Process().exe(). This change aims to resolve permission issues that may occur if, for example, the Python binary has certain capabilities assigned. Move shell completion to the init script - Currently we are expanding the autocompletion in the activate script and adding the expanded commands into the init script. To avoid concerns about shell versions, move the entire expansion to the init script. --- export.sh | 6 +++- tools/export_utils/activate_venv.py | 14 +++++--- tools/export_utils/console_output.py | 4 ++- tools/export_utils/shell_types.py | 52 +++++++++++++++------------- tools/export_utils/utils.py | 1 - 5 files changed, 46 insertions(+), 31 deletions(-) diff --git a/export.sh b/export.sh index 38a8cece0e..6bea2c4824 100644 --- a/export.sh +++ b/export.sh @@ -19,15 +19,19 @@ fi # Attempt to identify the ESP-IDF directory idf_path="." +shell_type="detect" + # shellcheck disable=SC2128,SC2169,SC2039,SC3054,SC3028 # ignore array expansion warning if test -n "${BASH_SOURCE-}" then # shellcheck disable=SC3028,SC3054 # unreachable with 'dash' idf_path=$(dirname "${BASH_SOURCE[0]}") + shell_type="bash" elif test -n "${ZSH_VERSION-}" then # shellcheck disable=SC2296 # ignore parameter starts with '{' because it's zsh idf_path=$(dirname "${(%):-%x}") + shell_type="zsh" elif test -n "${IDF_PATH-}" then idf_path=$IDF_PATH @@ -46,7 +50,7 @@ fi . "${idf_path}/tools/detect_python.sh" # Evaluate the ESP-IDF environment set up by the activate.py script. -idf_exports=$("$ESP_PYTHON" "${idf_path}/tools/activate.py" --export) +idf_exports=$("$ESP_PYTHON" "${idf_path}/tools/activate.py" --export --shell $shell_type) eval "${idf_exports}" unset idf_path return 0 diff --git a/tools/export_utils/activate_venv.py b/tools/export_utils/activate_venv.py index 8949943afc..fa3ed05145 100644 --- a/tools/export_utils/activate_venv.py +++ b/tools/export_utils/activate_venv.py @@ -25,7 +25,7 @@ def parse_arguments() -> argparse.Namespace: epilog='On Windows, run `python activate.py` to execute this script in the current terminal window.') parser.add_argument('-s', '--shell', metavar='SHELL', - default=os.environ.get('ESP_IDF_SHELL', None), + default=os.environ.get('ESP_IDF_SHELL', 'detect'), help='Explicitly specify shell to start. For example bash, zsh, powershell.exe, cmd.exe') parser.add_argument('-l', '--list', action='store_true', @@ -38,6 +38,7 @@ def parse_arguments() -> argparse.Namespace: help=('Disable ANSI color escape sequences.')) parser.add_argument('-d', '--debug', action='store_true', + default=bool(os.environ.get('ESP_IDF_EXPORT_DEBUG')), help=('Enable debug information.')) parser.add_argument('-q', '--quiet', action='store_true', @@ -100,17 +101,21 @@ def get_idf_env() -> Dict[str,str]: def detect_shell(args: Any) -> str: import psutil - if args.shell is not None: + if args.shell != 'detect': + debug(f'Shell explicitly stated: "{args.shell}"') return str(args.shell) current_pid = os.getpid() detected_shell_name = '' while True: parent_pid = psutil.Process(current_pid).ppid() - parent_name = os.path.basename(psutil.Process(parent_pid).exe()) + parent = psutil.Process(parent_pid) + parent_cmdline = parent.cmdline() + parent_exe = parent_cmdline[0].lstrip('-') + parent_name = os.path.basename(parent_exe) + debug(f'Parent: pid: {parent_pid}, cmdline: {parent_cmdline}, exe: {parent_exe}, name: {parent_name}') if not parent_name.lower().startswith('python'): detected_shell_name = parent_name - conf.DETECTED_SHELL_PATH = psutil.Process(parent_pid).exe() break current_pid = parent_pid @@ -143,6 +148,7 @@ def main() -> None: # Fill config global holder conf.ARGS = args + debug(f'command line: {sys.argv}') if conf.ARGS.list: oprint(SUPPORTED_SHELLS) sys.exit() diff --git a/tools/export_utils/console_output.py b/tools/export_utils/console_output.py index 03d8cb8453..af06abdc32 100644 --- a/tools/export_utils/console_output.py +++ b/tools/export_utils/console_output.py @@ -17,7 +17,7 @@ CONSOLE_STDERR = Console(stderr=True, width=255) CONSOLE_STDOUT = Console(width=255) -def status_message(msg: str, rv_on_ok: bool=False, die_on_err: bool=True) -> Callable: +def status_message(msg: str, msg_result: str='', rv_on_ok: bool=False, die_on_err: bool=True) -> Callable: def inner(func: Callable) -> Callable: def wrapper(*args: Any, **kwargs: Any) -> Any: eprint(f'[dark_orange]*[/dark_orange] {msg} ... ', end='') @@ -34,6 +34,8 @@ def status_message(msg: str, rv_on_ok: bool=False, die_on_err: bool=True) -> Cal if rv_on_ok: eprint(f'[green]{rv}[/green]') + elif msg_result: + eprint(f'[green]{msg_result}[/green]') else: eprint('[green]OK[/green]') diff --git a/tools/export_utils/shell_types.py b/tools/export_utils/shell_types.py index 9daf91774e..bb0d0c5ff5 100644 --- a/tools/export_utils/shell_types.py +++ b/tools/export_utils/shell_types.py @@ -4,6 +4,7 @@ import os import re import shutil import sys +import textwrap from pathlib import Path from subprocess import run from tempfile import gettempdir @@ -92,26 +93,23 @@ class UnixShell(Shell): class BashShell(UnixShell): - def get_bash_major_minor(self) -> float: - env = self.expanded_env() - bash_interpreter = conf.DETECTED_SHELL_PATH if conf.DETECTED_SHELL_PATH else 'bash' - stdout = run_cmd([bash_interpreter, '-c', 'echo ${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}'], env=env) - bash_maj_min = float(stdout) - return bash_maj_min - - @status_message('Shell completion', die_on_err=False) + @status_message('Shell completion', msg_result='Autocompletion code generated') def autocompletion(self) -> str: - bash_maj_min = self.get_bash_major_minor() - # Click supports bash version >= 4.4 - # https://click.palletsprojects.com/en/8.1.x/changes/#version-8-0-0 - if bash_maj_min < 4.4: - raise RuntimeError('Autocompletion not supported') + bash_source = 'bash_source' if self.click_ver() >= 8 else 'source_bash' + autocom = textwrap.dedent(f""" + WARNING_MSG="WARNING: Failed to load shell autocompletion for bash version: $BASH_VERSION!" + if test ${{BASH_VERSINFO[0]}} -lt 4 + then + echo "$WARNING_MSG" + else + if ! eval "$(env LANG=en _IDF.PY_COMPLETE={bash_source} idf.py)" + then + echo "$WARNING_MSG" + fi + fi + """) - env = self.expanded_env() - env['LANG'] = 'en' - env['_IDF.PY_COMPLETE'] = 'bash_source' if self.click_ver() >= 8 else 'source_bash' - stdout: str = run_cmd([sys.executable, conf.IDF_PY], env=env) - return stdout + return autocom def init_file(self) -> None: with open(self.script_file_path, 'w') as fd: @@ -130,13 +128,19 @@ class BashShell(UnixShell): class ZshShell(UnixShell): - @status_message('Shell completion', die_on_err=False) + @status_message('Shell completion', msg_result='Autocompletion code generated') def autocompletion(self) -> str: - env = self.expanded_env() - env['LANG'] = 'en' - env['_IDF.PY_COMPLETE'] = 'zsh_source' if self.click_ver() >= 8 else 'source_zsh' - stdout = run_cmd([sys.executable, conf.IDF_PY], env=env) - return f'autoload -Uz compinit && compinit -u\n{stdout}' + zsh_source = 'zsh_source' if self.click_ver() >= 8 else 'source_zsh' + autocom = textwrap.dedent(f""" + WARNING_MSG="WARNING: Failed to load shell autocompletion for zsh version: $ZSH_VERSION!" + autoload -Uz compinit && compinit -u + if ! eval "$(env _IDF.PY_COMPLETE={zsh_source} idf.py)" + then + echo "$WARNING_MSG" + fi + """) + + return autocom def init_file(self) -> None: # If ZDOTDIR is unset, HOME is used instead. diff --git a/tools/export_utils/utils.py b/tools/export_utils/utils.py index 53ba07a852..bd1d60ba45 100644 --- a/tools/export_utils/utils.py +++ b/tools/export_utils/utils.py @@ -22,7 +22,6 @@ class Config: self.IDF_TOOLS_PY = os.path.join(self.IDF_PATH, 'tools', 'idf_tools.py') self.IDF_PY = os.path.join(self.IDF_PATH, 'tools', 'idf.py') self.ARGS: Optional[argparse.Namespace] = None - self.DETECTED_SHELL_PATH: str = '' # Global variable instance