From f71fc7af25044b350511917fadb79df5fd41e615 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lukas=20Backstr=C3=B6m?= <lukas@erlang.org>
Date: Wed, 23 Oct 2024 13:03:40 +0200
Subject: [PATCH] Initialize at startup and change name to os_cmd_shell

---
 lib/kernel/doc/kernel_app.md |  4 +--
 lib/kernel/src/kernel.erl    |  1 +
 lib/kernel/src/os.erl        | 70 +++++++++++++++++++++---------------
 lib/kernel/test/os_SUITE.erl |  2 +-
 4 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/lib/kernel/doc/kernel_app.md b/lib/kernel/doc/kernel_app.md
index 0aff1f35d071..0428b911de30 100644
--- a/lib/kernel/doc/kernel_app.md
+++ b/lib/kernel/doc/kernel_app.md
@@ -512,8 +512,8 @@ For more information about configuration parameters, see file
   [Escripts and non-interactive I/O in Unicode Usage in Erlang](`e:stdlib:unicode_usage.md#escripts-and-non-interactive-i-o`)
   for more details.
 
-- **`erl_sys_shell = string()`{: #erl_sys_shell }** - Specifies which shell to
-  use when invoking system commands via os:cmd(). By default the shell is detected
+- **`os_cmd_shell = string()`{: #os_cmd_shell }** - Specifies which shell to
+  use when invoking system commands via `os:cmd/2`. By default the shell is detected
   automatically.
 
 ## Deprecated Configuration Parameters
diff --git a/lib/kernel/src/kernel.erl b/lib/kernel/src/kernel.erl
index 021676c33edc..f53f3b10713a 100644
--- a/lib/kernel/src/kernel.erl
+++ b/lib/kernel/src/kernel.erl
@@ -33,6 +33,7 @@
 start(_, []) ->
     %% Setup the logger and configure the kernel logger environment
     ok = logger:internal_init_logger(),
+    ok = os:internal_init_cmd_shell(),
     case supervisor:start_link({local, kernel_sup}, kernel, []) of
 	{ok, Pid} ->
             ok = erl_signal_handler:start(),
diff --git a/lib/kernel/src/os.erl b/lib/kernel/src/os.erl
index 87ab07a866d4..d93a186d0ce8 100644
--- a/lib/kernel/src/os.erl
+++ b/lib/kernel/src/os.erl
@@ -37,6 +37,8 @@ a program to run on most platforms.
 
 -export([type/0, version/0, cmd/1, cmd/2, find_executable/1, find_executable/2]).
 
+-export([internal_init_cmd_shell/0]).
+
 -include("file.hrl").
 
 -export_type([env_var_name/0, env_var_value/0, env_var_name_value/0]).
@@ -581,39 +583,14 @@ get_option(Opt, Options, Default) ->
         _ -> throw(badopt)
     end.
 
-mk_cmd({win32,Wtype}, Cmd) ->
-    Command = case {os:getenv("COMSPEC"),Wtype} of
-                  {false,windows} -> lists:concat(["command.com /c", Cmd]);
-                  {false,_} -> lists:concat(["cmd /c", Cmd]);
-                  {Cspec,_} -> lists:concat([Cspec," /c",Cmd])
-              end,
+mk_cmd({win32,_}, Cmd) ->
+    {ok, Shell} = application:get_env(kernel, os_cmd_shell),
+    Command = lists:concat([Shell, " /c", Cmd]),
     {Command, [], [], <<>>};
 mk_cmd(_,Cmd) ->
     %% Have to send command in like this in order to make sh commands like
     %% cd and ulimit available.
-    %%
-    %% We use an absolute path here because we do not want the path to be
-    %% searched in case a stale NFS handle is somewhere in the path before
-    %% the sh command.
-    %%
-    %% Check if the default shell is located in /bin/sh as expected usually
-    %% or in /system/bin/sh as implemented on Android. The raw option is
-    %% used to bypass the file server and speed up the file access.
-    Shell = case application:get_env(kernel, erl_sys_shell) of
-        undefined ->
-            case file:read_file_info("/bin/sh",[raw]) of
-                {ok,#file_info{type=regular}} ->
-                    "/bin/sh";
-                _ ->
-                    case file:read_file_info("/system/bin/sh",[raw]) of
-                        {ok,#file_info{type=regular}} ->
-                            "/system/bin/sh";
-                        _ ->
-                            "/bin/sh"
-                    end
-            end;
-        {ok, ShellName} -> ShellName
-    end,
+    {ok, Shell} = application:get_env(kernel, os_cmd_shell),
     {Shell ++ " -s unix:cmd", [out],
      %% We insert a new line after the command, in case the command
      %% contains a comment character.
@@ -632,6 +609,41 @@ mk_cmd(_,Cmd) ->
      ["(", unicode:characters_to_binary(Cmd), "\n) </dev/null; echo \"\^D\"\n"],
      <<$\^D>>}.
 
+-doc false.
+internal_init_cmd_shell() ->
+    case application:get_env(kernel, os_cmd_shell) of
+        undefined ->
+            application:set_env(kernel, os_cmd_shell,
+            internal_init_cmd_shell(os:type()));
+        _ ->
+            ok
+    end.
+internal_init_cmd_shell({win32,Wtype}) ->
+    case {os:getenv("COMSPEC"),Wtype} of
+        {false,windows} -> "command.com";
+        {false,_} -> "cmd";
+        {Cspec,_} -> Cspec
+    end;
+internal_init_cmd_shell(_) ->
+    %% We use an absolute path here because we do not want the path to be
+    %% searched in case a stale NFS handle is somewhere in the path before
+    %% the sh command.
+    %%
+    %% Check if the default shell is located in /bin/sh as expected usually
+    %% or in /system/bin/sh as implemented on Android. The raw option is
+    %% used to bypass the file server.
+    case file:read_file_info("/bin/sh",[raw]) of
+        {ok,#file_info{type=regular}} ->
+            "/bin/sh";
+        _ ->
+            case file:read_file_info("/system/bin/sh",[raw]) of
+                {ok,#file_info{type=regular}} ->
+                    "/system/bin/sh";
+                _ ->
+                    "/bin/sh"
+            end
+    end.
+
 validate(Term) ->
     try validate1(Term)
     catch error:_ -> throw(badarg)
diff --git a/lib/kernel/test/os_SUITE.erl b/lib/kernel/test/os_SUITE.erl
index 3cc66dabd30f..34ed65c5fca1 100644
--- a/lib/kernel/test/os_SUITE.erl
+++ b/lib/kernel/test/os_SUITE.erl
@@ -473,7 +473,7 @@ sys_shell(Config) ->
     DataDir = proplists:get_value(data_dir, Config),
     SysShell = filename:join(DataDir, "sys_shell"),
 
-    application:set_env(kernel, erl_sys_shell, SysShell),
+    application:set_env(kernel, os_cmd_shell, SysShell),
 
     %% os:cmd should not try to detect the shell location rather than use
     %% the value from kernel:sys_shell parameter