Clean up subshell precmd and PS1 handling

[mc-butler]

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/4634
Reporter	zaytsev (@zyv)
Mentions	ossi (@ossilator), d3m3t3r (d3m3t3r@….com)

Follow-up for #3748:

We want to remove our PS1 / prompt fallbacks for all shells and cleanup fallback precmd handling:

        "_mc_precmd() { "
        "  pwd>&%d; "
        "  kill -STOP $$; "
        "}; "
        "_MC_PRECMD=_mc_precmd; "
        "PS1='$($_MC_PRECMD)'\"${PS1:-'$PWD $ '}\"\n";

to actually answer the question whether PS1 can be unset in the first place: probably not. i mean, it can if you manually unset it, but dash actually shows no prompt at all in this case. so arguably, providing even a minimal PS1 fallback is unnecessary.

It's definitely quite unlikely an interactive shell not setting PS1. The user would need to mess up something. IMO, the default PS1 could be removed from every 'precmd' code if we check all shells cope with empty PS1 reasonably. For bash, you can test it by adding "unset PS1" into .bashrc. It seems to work fine.

---

add /bin/lksh as well, it’s present on many systems (all Debian derivatives, plus others) and is an mksh binary built with POSIX semantics where they cannot be made a run-time option (such as width of the arithmetic data type)

Originally posted by @mirabilos in 9f5d80d

[mc-butler]

Changed by zaytsev (@zyv) on Jan 25, 2025 at 13:52 UTC (comment 1)

PR: #212

[mc-butler]

Changed by d3m3t3r (d3m3t3r@….com) on Jan 25, 2025 at 23:25 UTC (comment 2)

Note the PS1 fallback is still present in the proposed code, only now it is '$PWD $ '.
IMHO, this ticket would make the code look a bit simpler and cleaner but many users, e.g. with busybox/ash/dash, will just loose a feature - $HOME prefix in $PWD no longer replaced by '~'. May not be worth it.

#212

This PR is about *fixing* the existing PS1 fallback, not removing it.

[mc-butler]

Changed by ossi (@ossilator) on Jan 26, 2025 at 11:48 UTC (comment 3)

the point i tried to make previously is that this fallback feature is kinda useless - the user will have set up PS1 to their liking already, because they will want it to be nice even if mc is not used. therefore, we only need to preserve what is already there.

[mc-butler]

Changed by zaytsev (@zyv) on Jan 30, 2025 at 18:12 UTC (comment 4)

TODO: when adding support for ksh/mksh the tests were not updated! Check that other subshells are listed in tests/lib/mcconfig/user_configs_path.c.

• Good: https://source.midnight-commander.org/R/MC_ZSHRC_FILE.html

• Bad: https://source.midnight-commander.org/R/MC_KSHRC_FILE.html
• Bad: https://source.midnight-commander.org/R/MC_MKSHRC_FILE.html

[zyv]

Another set of TODOs:

#define MC_BASHRC_FILE "bashrc"
#define MC_ZSHRC_FILE ".zshrc"
#define MC_ASHRC_FILE "ashrc"

Why the dot? Apparently zsh cannot be given a custom filename (see #4203)? If so, add a comment.

Move hardcoded filenames (.profile) etc.

[zyv]

This is bogus, it causes warnings about >&10 being a syntax error (also in the current refactored form), you must either ensure to use subshell_pipe[WRITE] is smaller than 10 (for ash as well, probably) or use >/dev/fd/%d if extant.

Originally posted by @mirabilos in 9f5d80d

[mirabilos]

Sorry, @mirabilos, but I have no idea what you are talking about. This was user-contributed code that I only tried to test on mksh, since I don't use it myself and I didn't notice any warnings.

It’s visible in strace output only, but since I had to trace it I saw it. The net effect is that mc doesn’t see the output of the pwd command. (Perhaps this is what causes that weird } $ prompt inside mc?)

mksh did support fds > 9 for a while, but I had to remove it as it caused trouble and wasn’t future-proof.

It would be really great if you could please provide a patch.

Eurghs… bit hard, I don’t have the tuits to test a patch well at the moment.

[zyv]

It would be really great if you could please provide a patch.

Eurghs… bit hard, I don’t have the tuits to test a patch well at the moment.

It's a shame. In that case, I'm afraid it will remain broken until someone else does something about it... it's difficult to find a more knowledgeable expert on mksh than you.

[mirabilos]

Something like this might work. Not tested.

--- common.c~	2025-06-16 19:12:50.687374256 +0200
+++ common.c	2025-06-16 19:20:14.521953529 +0200
@@ -445,6 +445,25 @@
     // at the master side of the pty and so it will disappear.
     close (mc_global.tty.subshell_pty);
 
+    // We’re in the child, so we have many free file descriptors available.
+    // subshell_pipe[WRITE] must be ≤9 as it’s used in “>&%d” constructs in
+    // PS1, so if it’s not dup it to lower; prefer to let the OS allocate a
+    // free fd, now that all those above are closed. This isn’t fail-proof,
+    // but better than not doing it at all and probably better than forcing
+    // a hardcoded fd (like 4).
+    if (subshell_pipe[WRITE] > 9) {
+        int moved_fd;
+
+        // should always be true but if not, give up
+        if ((moved_fd = dup(subshell_pipe[WRITE])) != -1) {
+            if (moved_fd < 9) {
+                close(subshell_pipe[WRITE]);
+                subshell_pipe[WRITE] = moved_fd;
+            } else
+                close(moved_fd);
+        }
+    }
+
     // Execute the subshell at last
 
     switch (mc_global.shell->type)

The idea here is to do it in the child, which closes many fds already before exec’ing the shell, so the chance that a lower one is now available even if it wasn’t at the time of the pipe() call is high, and if it isn’t to just do what the current code does (no regression), that is, nothing and hope the shell supports high fds in redirections.

[mirabilos]

On Mon, 16 Jun 2025, Yury V. Zaytsev wrote: It's a shame. In that case, I'm afraid it will remain broken until someone else does something about it... it's difficult to find a more knowledgeable expert on `mksh` than you.

Yeah, this isn’t just about mksh though. If the patch I posted doesn’t help I’ll leave this open until I can get to it more. I’ve just had the first day in a new job, and the next weeks are going to be hectic. bye, //mirabilos -- 11:56⎜«liwakura:#!/bin/mksh» also, i wanted to add mksh to my own distro │ i was disappointed that there is no makefile │ but somehow the Build.sh is the least painful built system i've ever seen │ honours CC, {CPP,C,LD}FLAGS properly │ looks cleary like done by someone who knows what they are doing

[mirabilos]

Is it possible that this problem (pwd output never sent to mc) is also responsible for the pretty much exactly ten-second delay I’m seeing on starting mc -d? From looking at strace -t output, it seems so.

(The strace also shows it’s in parts horridly inefficient, doing one-byte reads and writes, before that… it’s the “MC_PS1_SAVE=…” string that gets written byte by byte…)

I’m not sure how close the Debian trixie release is to upstream’s top of development branch, but it does look like I’ll spend some time in there, perhaps even send a PR (I’ll try to build upstream git then, too). mc is rather close to my heart, so… let’s make it better.

[mirabilos]

It gets even better:

• this does not happen always, e.g. it just didn’t happen from an ssh session (this is probably because pipe() returned small enough file descriptors)
• if it happens, the “Pause after run” option is ignored (!!!), so if I type “date”+Enter, I see flicker and the panel and have to ^O to see the date; if it doesn’t happens, this works…

Curious and curiouser… so, yes, I’ll definitely get that fixed. Debian trixie has 4.8.33, so should be close enough?