Skip to content

Commit

Permalink
fix(tools): Upgrade shell detection & simplify autocompletion
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fhrbata authored and Marek Fiala committed Sep 18, 2024
1 parent 2109f81 commit 6f3241a
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 32 deletions.
6 changes: 5 additions & 1 deletion export.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
14 changes: 10 additions & 4 deletions tools/export_utils/activate_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion tools/export_utils/console_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
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='')
Expand All @@ -34,6 +34,8 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:

if rv_on_ok:
eprint(f'[green]{rv}[/green]')
elif msg_result:
eprint(f'[green]{msg_result}[/green]')
else:
eprint('[green]OK[/green]')

Expand Down
54 changes: 29 additions & 25 deletions tools/export_utils/shell_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import re
import shutil
import sys
import textwrap
from pathlib import Path
from subprocess import run
from tempfile import gettempdir
Expand Down Expand Up @@ -92,26 +93,23 @@ def click_ver(self) -> int:


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')

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
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
""")

return autocom

def init_file(self) -> None:
with open(self.script_file_path, 'w') as fd:
Expand All @@ -130,13 +128,19 @@ def spawn(self) -> None:


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.
Expand Down
1 change: 0 additions & 1 deletion tools/export_utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def __init__(self) -> None:
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
Expand Down

0 comments on commit 6f3241a

Please sign in to comment.