[YORROY123]

rpi-eeprom-config --edit quotes the whole of $EDITOR as a single word before passing it to os.system, so an editor given with arguments fails:

$ sudo env EDITOR="nano -w" rpi-eeprom-config -e
sh: 1: nano -w: not found
ERROR: Aborting update because ''nano -w' '/tmp/tmpzc3rn6bc/boot.conf'' exited with code 32512.

crontab -e and git accept the same value (EDITOR="code --wait" is the common case). This splits $EDITOR with shlex and runs it with subprocess.call, without a shell. The abort message also showed the raw os.system wait status (32512, 256); it now shows the editor's exit code, and says when the editor cannot be started.

Tested on a Pi 5 (Debian trixie, rpi-eeprom 28.30-1) with editors that make --edit abort, so nothing was staged (/boot/firmware/pieeprom.upd absent afterwards):

EDITOR	before	after
truncate -s 0	''truncate -s 0' '…'' exited with code 32512	runs, then '…/boot.conf' appears to be empty
false	exited with code 256	exited with code 1
no-such-editor	exited with code 32512	'no-such-editor' could not be started: No such file or directory
sh -c "exit 3" sh	exited with code 32512	exited with code 3

One behaviour change: an editor path containing spaces now needs quoting inside EDITOR (e.g. EDITOR='"/opt/My Editor/ed"'), the same as for git and crontab. shlex.join is avoided so this still runs on Python 3.7.

[timg236]

Thanks, the intent is right and the exit-code reporting is an improvement. Three behaviour changes relative to the current os.system path are noted inline; the first one needs fixing before this can be merged.

[timg236]

This regresses Ctrl-C handling inside the editor. system(3) ignores SIGINT and SIGQUIT in the parent for the duration of the call, so only the editor sees the keypress (vim uses Ctrl-C to return to normal mode, nano to show the cursor position). With subprocess.call the terminal delivers SIGINT to the whole foreground group, Python raises KeyboardInterrupt in wait(), subprocess.call kills the child, the handler in main prints Interrupted, and exit_handler removes the temp directory from under the editor. The SIGKILLed editor cannot restore termios, so the terminal is left in raw/no-echo mode.

Please ignore SIGINT and SIGQUIT around the call and restore them afterwards, as git's launch_editor does:

old_int = signal.signal(signal.SIGINT, signal.SIG_IGN)
old_quit = signal.signal(signal.SIGQUIT, signal.SIG_IGN)
try:
    result = subprocess.call(cmd)
finally:
    signal.signal(signal.SIGINT, old_int)
    signal.signal(signal.SIGQUIT, old_quit)

[timg236]

shlex.split raises ValueError on an unbalanced quote and nothing catches it, so EDITOR="vim -c 'set" rpi-eeprom-config -e now dies with a Python traceback (ValueError: No closing quotation). Before this change the shell reported a syntax error and the existing non-zero path aborted cleanly. Please catch ValueError here and route it through exit_error.

[timg236]

The paths-with-spaces change is acknowledged in the description and is fine, but this message hides the cause. A user who had EDITOR="/opt/My Editor/ed" working now sees '/opt/My Editor/ed' could not be started: No such file or directory although that path exists, because the argv is actually ['/opt/My', 'Editor/ed', ...]. Printing cmd[0], or the shlex-quoted cmd that the branch below already builds, would make it obvious the value was split. Worth a note in the --edit help text too, which currently only says EDITOR may be overridden.

[YORROY123]

Thanks for the careful review. All three are addressed in c6edcc8:

1. Ctrl-C: the editor is started with Popen, then SIGINT and SIGQUIT are ignored around wait() and restored in finally. I ignore them after Popen rather than before subprocess.call, because a disposition of SIG_IGN is inherited across exec, so ignoring first would leave an editor that does not install its own handler unable to be interrupted. That matches git, which calls sigchain_push(SIGINT, SIG_IGN) after start_command().
2. Unbalanced quotes: ValueError from shlex.split now goes through exit_error.
3. Paths with spaces: the start failure names cmd[0], and the --edit help text says EDITOR may carry arguments and that a path with spaces must be quoted inside it.

Tested on a Pi 5 under setsid so each run has its own process group, with editors that make --edit abort (no pieeprom.upd afterwards). The Ctrl-C case uses an editor script that traps SIGINT, sends kill -INT 0 to the whole group, then empties the file:

EDITOR	previous commit	now
trap SIGINT + kill -INT 0	editor survives, then Interrupted (rc 130)	editor survives, '…/boot.conf' appears to be empty
vim -c 'set	ValueError: No closing quotation traceback	EDITOR 'vim -c 'set' cannot be parsed: No closing quotation
/opt/My Editor/ed	'/opt/My Editor/ed' could not be started	'/opt/My' could not be started
"/opt/My Editor/ed"		'/opt/My Editor/ed' could not be started
false, truncate -s 0, no-such-editor	unchanged	unchanged

[timg236]

Please could you squash the commits before merging?

[YORROY123]

Squashed into a single commit (b05bd19). The code is unchanged. Thanks for the review!

[timg236]

Thanks!