Skip to content

Commit

Permalink
Add the os_cmd_shell kernel config parameter to point to system shell…
Browse files Browse the repository at this point in the history
… for os:cmd/2

Initialize the parameter at startup

Fix the test - restore the old shell after the test
Add initialization to other tests that use os:cmd/2
Update os:cmd/2 description
  • Loading branch information
yarisx committed Oct 24, 2024
1 parent bcafd64 commit 7af29a1
Show file tree
Hide file tree
Showing 24 changed files with 100 additions and 29 deletions.
4 changes: 4 additions & 0 deletions lib/kernel/doc/kernel_app.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,10 @@ 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.

- **`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

In Erlang/OTP 21.0, a new API for logging was added. The old `error_logger`
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/src/kernel.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
69 changes: 44 additions & 25 deletions lib/kernel/src/os.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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]).
Expand Down Expand Up @@ -517,6 +519,9 @@ cmd(Cmd) ->
-doc """
Executes `Command` in a command shell of the target OS, captures the standard
output and standard error of the command, and returns this result as a string.
The command shell can be set using the
[kernel configuration parameter](kernel_app.md#os_cmd_shell), by default the
shell is detected upon system startup.
_Examples:_
Expand Down Expand Up @@ -581,35 +586,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 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, 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.
Expand All @@ -628,6 +612,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)
Expand Down
2 changes: 2 additions & 0 deletions lib/kernel/test/application_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -2060,6 +2060,7 @@ do_configfd_test_bash() ->
Command =
lists:flatten(io_lib:format("bash -c \"~s\"",
[quote_sub_strings(String)])),
os:internal_init_cmd_shell(),
Res = os:cmd(Command),
io:format("Command:~n"),
io:format("~s~n", [Command]),
Expand Down Expand Up @@ -2255,6 +2256,7 @@ total_memory() ->
configfd_bash(Conf) when is_list(Conf) ->
case os:type() of
{unix,_} ->
os:internal_init_cmd_shell(),
case os:cmd("bash -c \"echo -n yes_bash_shell_exists\"") of
"yes_bash_shell_exists" ->
do_configfd_test_bash();
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/erl_distribution_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ groups() ->
xdg_cookie]}].

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
start_gen_tcp_dist_test_type_server(),
Config.

Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/file_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ end_per_group(_GroupName, Config) ->


init_per_suite(Config) when is_list(Config) ->
os:internal_init_cmd_shell(),
SaslConfig = case application:start(sasl) of
{error, {already_started, sasl}} ->
[];
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/file_name_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ groups() ->
[].

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
Config.

end_per_suite(_Config) ->
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/gen_tcp_misc_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ socket_monitor_cases() ->
].

init_per_suite(Config0) ->
os:internal_init_cmd_shell(),

?P("init_per_suite -> entry with"
"~n Config: ~p"
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/global_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ end_per_group(_GroupName, Config) ->
Config.

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
Config.

end_per_suite(_Config) ->
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/inet_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ required(hosts) ->


init_per_suite(Config0) ->
os:internal_init_cmd_shell(),

?P("init_per_suite -> entry with"
"~n Config: ~p"
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/init_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ groups() ->
[{boot, [], [boot1, boot2]}].

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
Config.

end_per_suite(_Config) ->
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/interactive_shell_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ groups() ->
].

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
Term = os:getenv("TERM", "dumb"),
os:putenv("TERM", "vt100"),
[{term,Term}|Config].
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/kernel_config_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ end_per_group(_GroupName, Config) ->


init_per_suite(Config) when is_list(Config) ->
os:internal_init_cmd_shell(),
Config.

end_per_suite(Config) when is_list(Config) ->
Expand Down
3 changes: 3 additions & 0 deletions lib/kernel/test/kernel_test_lib.erl
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,14 @@
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

init_per_suite([{allow_skip, Allow}|Config]) ->
os:internal_init_cmd_shell(),
init_per_suite(Allow, Config);
init_per_suite(Config) ->
os:internal_init_cmd_shell(),
init_per_suite(true, Config).

init_per_suite(AllowSkip, Config) when is_boolean(AllowSkip) ->
os:internal_init_cmd_shell(),

print("kernel environment: "
"~n (kernel) app: ~p"
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/logger_simple_h_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ suite() ->
[{timetrap,{seconds,30}}].

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
Hs0 = logger:get_handler_config(),
Hs = lists:keydelete(cth_log_redirect,1,Hs0),
[ok = logger:remove_handler(Id) || {Id,_,_} <- Hs],
Expand Down
20 changes: 17 additions & 3 deletions lib/kernel/test/os_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
find_executable/1, unix_comment_in_command/1, deep_list_command/1,
large_output_command/1, background_command/0, background_command/1,
message_leak/1, close_stdin/0, close_stdin/1, max_size_command/1,
perf_counter_api/1, error_info/1]).
perf_counter_api/1, error_info/1, os_cmd_shell/1]).

-include_lib("common_test/include/ct.hrl").

Expand All @@ -43,12 +43,13 @@ all() ->
find_executable, unix_comment_in_command, deep_list_command,
large_output_command, background_command, message_leak,
close_stdin, max_size_command, perf_counter_api,
error_info].
error_info, os_cmd_shell].

groups() ->
[].

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
Config.

end_per_suite(_Config) ->
Expand All @@ -61,7 +62,7 @@ end_per_group(_GroupName, Config) ->
Config.

init_per_testcase(TC, Config)
when TC =:= background_command; TC =:= close_stdin ->
when TC =:= background_command; TC =:= close_stdin; TC =:= os_cmd_shell ->
case os:type() of
{win32, _} ->
{skip,"Should not work on windows"};
Expand Down Expand Up @@ -469,6 +470,19 @@ error_info(Config) ->
],
error_info_lib:test_error_info(os, L).

os_cmd_shell(Config) ->
DataDir = proplists:get_value(data_dir, Config),
SysShell = filename:join(DataDir, "sys_shell"),

{ok, OldShell} = application:get_env(kernel, os_cmd_shell),
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:os_cmd_shell parameter
Ls = os:cmd("ls"),
application:set_env(kernel, os_cmd_shell, OldShell),
comp("sys_shell", Ls).

no_limit_for_opened_files() ->
case os:type() of
{unix, freebsd} ->
Expand Down
8 changes: 7 additions & 1 deletion lib/kernel/test/os_SUITE_data/Makefile.src
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ LD = @LD@
CFLAGS = @CFLAGS@ -I@erl_include@ @DEFS@
CROSSLDFLAGS = @CROSSLDFLAGS@

PROGS = my_echo@exe@ my_fds@exe@
PROGS = my_echo@exe@ my_fds@exe@ sys_shell@exe@

all: $(PROGS)

Expand All @@ -18,3 +18,9 @@ my_fds@exe@: my_fds@obj@

my_fds@obj@: my_fds.c
$(CC) -c -o my_fds@obj@ $(CFLAGS) my_fds.c

sys_shell@exe@: sys_shell@obj@
$(LD) $(CROSSLDFLAGS) -o sys_shell sys_shell@obj@ @LIBS@

sys_shell@obj@: sys_shell.c
$(CC) -c -o sys_shell@obj@ $(CFLAGS) sys_shell.c
6 changes: 6 additions & 0 deletions lib/kernel/test/os_SUITE_data/sys_shell.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <stdio.h>
int main(void)
{
printf("sys_shell");
return 0;
}
1 change: 1 addition & 0 deletions lib/kernel/test/prim_file_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ end_per_group(_GroupName, Config) ->


init_per_suite(Config) when is_list(Config) ->
os:internal_init_cmd_shell(),
case os:type() of
{win32, _} ->
Priv = proplists:get_value(priv_dir, Config),
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/sendfile_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ all() ->
t_sendfile_arguments].

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
case {os:type(),os:version()} of
{{unix,sunos}, {5,8,_}} ->
{skip, "Solaris 8 not supported for now"};
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/seq_trace_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ groups() ->
[].

init_per_suite(Config) ->
os:internal_init_cmd_shell(),
Config.

end_per_suite(_Config) ->
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/socket_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ otp18240_cases() ->
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

init_per_suite(Config0) ->
os:internal_init_cmd_shell(),
?P("init_per_suite -> entry with"
"~n Config: ~p"
"~n Nodes: ~p", [Config0, erlang:nodes()]),
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/socket_api_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,7 @@ api_op_with_timeout_cases() ->
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

init_per_suite(Config0) ->
os:internal_init_cmd_shell(),
?P("init_per_suite -> entry with"
"~n Config: ~p"
"~n Nodes: ~p", [Config0, erlang:nodes()]),
Expand Down
1 change: 1 addition & 0 deletions lib/kernel/test/socket_traffic_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ traffic_pp_sendmsg_recvmsg_cases() ->
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

init_per_suite(Config0) ->
os:internal_init_cmd_shell(),
?P("init_per_suite -> entry with"
"~n Config: ~p"
"~n Nodes: ~p", [Config0, erlang:nodes()]),
Expand Down

0 comments on commit 7af29a1

Please sign in to comment.