[m-rev.] for review: move code for invoking commands via the shell

Julien Fischer jfischer at opturion.com
Fri Jan 5 15:02:25 AEDT 2024


For review by anyone.

The immediate motivation for this is an upcoming change that centralises
the code used for file copying within the compiler.

---------------------------------

Move code for invoking commands via the shell.

Move the code used for invoking commands via the shell out of the module_cmds
module and into its own module. In doing so, this moves it from the parse_tree
package into the libs package.

compiler/module_cmds.m:
     Shift the code for invoking commands via the shell from here ...

compiler/system_command.m:
     ... to here.

compiler/libs.m:
     Include the system_command module.

compiler/compile_target_code.m:
compiler/fact_table.m:
compiler/make.module_target.m:
compiler/make.program_target.m:
     Conform to the above change.

compiler/notes/compiler_design.m:
     Document the new module.

Julien.

diff --git a/compiler/compile_target_code.m b/compiler/compile_target_code.m
index 4cd9b1e..45b17e5 100644
--- a/compiler/compile_target_code.m
+++ b/compiler/compile_target_code.m
@@ -244,6 +244,7 @@
  :- import_module libs.optimization_options.
  :- import_module libs.options.
  :- import_module libs.shell_util.
+:- import_module libs.system_command.
  :- import_module libs.trace_params.
  :- import_module parse_tree.error_spec.
  :- import_module parse_tree.find_module.
diff --git a/compiler/fact_table.m b/compiler/fact_table.m
index 51e0eeb..0957567 100644
--- a/compiler/fact_table.m
+++ b/compiler/fact_table.m
@@ -140,12 +140,12 @@
  :- import_module libs.file_util.
  :- import_module libs.globals.
  :- import_module libs.options.
+:- import_module libs.system_command.
  :- import_module ll_backend.llds_out.
  :- import_module ll_backend.llds_out.llds_out_data.
  :- import_module mdbcomp.prim_data.
  :- import_module parse_tree.builtin_lib_types.
  :- import_module parse_tree.file_names.
-:- import_module parse_tree.module_cmds.
  :- import_module parse_tree.parse_tree_out_term.
  :- import_module parse_tree.parse_tree_out_type.
  :- import_module parse_tree.prog_foreign.
diff --git a/compiler/libs.m b/compiler/libs.m
index 4ccd76a..bbf5032 100644
--- a/compiler/libs.m
+++ b/compiler/libs.m
@@ -45,6 +45,7 @@
  % OS interfaces not provided by the standard library.
  :- include_module process_util.
  :- include_module shell_util.
+:- include_module system_command.
  :- include_module timestamp.

  % Constraint machinery for the termination analysers.
diff --git a/compiler/make.module_target.m b/compiler/make.module_target.m
index 3600daf..a3fe6cb 100644
--- a/compiler/make.module_target.m
+++ b/compiler/make.module_target.m
@@ -86,6 +86,7 @@
  :- import_module libs.options.
  :- import_module libs.process_util.
  :- import_module libs.shell_util.
+:- import_module libs.system_command.
  :- import_module libs.timestamp.
  :- import_module make.build.
  :- import_module make.check_up_to_date.
diff --git a/compiler/make.program_target.m b/compiler/make.program_target.m
index 3a31442..9d710f9 100644
--- a/compiler/make.program_target.m
+++ b/compiler/make.program_target.m
@@ -75,6 +75,7 @@
  :- import_module libs.options.
  :- import_module libs.process_util.
  :- import_module libs.shell_util.
+:- import_module libs.system_command.
  :- import_module libs.timestamp.
  :- import_module make.build.
  :- import_module make.check_up_to_date.
diff --git a/compiler/module_cmds.m b/compiler/module_cmds.m
index 3e3d3b6..4c9f84a 100644
--- a/compiler/module_cmds.m
+++ b/compiler/module_cmds.m
@@ -2,7 +2,7 @@
  % vim: ft=mercury ts=4 sw=4 et
  %-----------------------------------------------------------------------------%
  % Copyright (C) 2008-2012 The University of Melbourne.
-% Copyright (C) 2013-2022 The Mercury team.
+% Copyright (C) 2013-2024 The Mercury team.
  % This file may only be copied under the terms of the GNU General
  % Public License - see the file COPYING in the Mercury distribution.
  %-----------------------------------------------------------------------------%
@@ -27,7 +27,6 @@

  :- import_module io.
  :- import_module list.
-:- import_module maybe.

  %-----------------------------------------------------------------------------%

@@ -118,50 +117,6 @@
  :- pred maybe_set_exit_status(maybe_succeeded::in, io::di, io::uo) is det.

  %-----------------------------------------------------------------------------%
-
-:- type quote_char
-    --->    forward     % '
-    ;       double.     % "
-
-:- type command_verbosity
-    --->    cmd_verbose
-            % Output the command line only with `--verbose'.
-
-    ;       cmd_verbose_commands.
-            % Output the command line with `--verbose-commands'. This should be
-            % used for commands that may be of interest to the user.
-
-    % invoke_system_command(Globals, ProgressStream, CmdOutputStream,
-    %   Verbosity, Command, Succeeded):
-    %
-    % Invoke an executable. Progress messages, including error messages
-    % that say why we cannot make progress, will go to ProgressStream.
-    % Output from the invoked command will go to CmdOutputStream.
-    %
-:- pred invoke_system_command(globals::in, io.text_output_stream::in,
-    io.text_output_stream::in,
-    command_verbosity::in, string::in, maybe_succeeded::out,
-    io::di, io::uo) is det.
-
-    % invoke_system_command_maybe_filter_output(Globals,
-    %   ProgressStream, CmdOutputStream, Verbosity, Command,
-    %   MaybeProcessOutput, Succeeded)
-    %
-    % Invoke an executable. Progress messages, including error messages
-    % that say why we cannot make progress, will go to ProgressStream.
-    % Output from the invoked command, filtered if MaybeProcessOutput
-    % is set to yes(...), will go to CmdOutputStream.
-    %
-:- pred invoke_system_command_maybe_filter_output(globals::in,
-    io.text_output_stream::in, io.text_output_stream::in,
-    command_verbosity::in, string::in, maybe(string)::in, maybe_succeeded::out,
-    io::di, io::uo) is det.
-
-    % Make a command string, which needs to be invoked in a shell environment.
-    %
-:- pred make_command_string(string::in, quote_char::in, string::out) is det.
-
-%-----------------------------------------------------------------------------%
  %
  % Java command-line tools utilities.
  %
@@ -220,7 +175,7 @@

  :- import_module libs.compute_grade.    % for grade_directory_component
  :- import_module libs.options.
-:- import_module libs.process_util.
+:- import_module libs.system_command.
  :- import_module parse_tree.java_names.

  :- import_module bool.
@@ -229,6 +184,7 @@
  :- import_module io.call_system.
  :- import_module io.environment.
  :- import_module io.file.
+:- import_module maybe.
  :- import_module require.
  :- import_module set.
  :- import_module string.
@@ -607,241 +563,6 @@ maybe_set_exit_status(did_not_succeed, !IO) :-
      io.set_exit_status(1, !IO).

  %-----------------------------------------------------------------------------%
-
-invoke_system_command(Globals, ProgressStream,
-        CmdOutputStream, Verbosity, Command, Succeeded, !IO) :-
-    invoke_system_command_maybe_filter_output(Globals, ProgressStream,
-        CmdOutputStream, Verbosity, Command, no, Succeeded, !IO).
-
-invoke_system_command_maybe_filter_output(Globals, ProgressStream,
-        CmdOutputStream, Verbosity, Command, MaybeProcessOutput,
-        Succeeded, !IO) :-
-    % This predicate shouldn't alter the exit status of mercury_compile.
-    io.get_exit_status(OldStatus, !IO),
-    globals.lookup_bool_option(Globals, verbose, Verbose),
-    (
-        Verbosity = cmd_verbose,
-        PrintCommand = Verbose
-    ;
-        Verbosity = cmd_verbose_commands,
-        globals.lookup_bool_option(Globals, verbose_commands, PrintCommand)
-    ),
-    (
-        PrintCommand = yes,
-        io.format(ProgressStream,
-            "%% Invoking system command `%s'...\n", [s(Command)], !IO),
-        io.flush_output(ProgressStream, !IO)
-    ;
-        PrintCommand = no
-    ),
-
-    % The output from the command is written to a temporary file,
-    % which is then written to the output stream. Without this,
-    % the output from the command would go to the current C output
-    % and error streams.
-
-    io.file.make_temp_file(TmpFileResult, !IO),
-    (
-        TmpFileResult = ok(TmpFile),
-        ( if use_dotnet then
-            % XXX can't use Bourne shell syntax to redirect on .NET
-            % XXX the output will go to the wrong place!
-            CommandRedirected = Command
-        else if use_win32 then
-            % On windows, we can't in general redirect standard error
-            % in the shell.
-            CommandRedirected = string.format("%s > %s",
-                [s(Command), s(TmpFile)])
-        else
-            CommandRedirected = string.format("%s > %s 2>&1",
-                [s(Command), s(TmpFile)])
-        ),
-        io.call_system.call_system_return_signal(CommandRedirected,
-            CmdResult, !IO),
-        (
-            CmdResult = ok(exited(Status)),
-            maybe_write_string(ProgressStream, PrintCommand, "% done.\n", !IO),
-            ( if Status = 0 then
-                CommandSucceeded = succeeded
-            else
-                % The command should have produced output describing the error.
-                CommandSucceeded = did_not_succeed
-            )
-        ;
-            CmdResult = ok(signalled(Signal)),
-            string.format("system command received signal %d.", [i(Signal)],
-                ErrorMsg),
-            report_error(ProgressStream, ErrorMsg, !IO),
-            % Also report the error to standard output, because if we raise the
-            % signal, this error may not ever been seen, the process stops, and
-            % the user is confused.
-            io.stdout_stream(StdOut, !IO),
-            report_error(StdOut, ErrorMsg, !IO),
-
-            % Make sure the current process gets the signal. Some systems (e.g.
-            % Linux) ignore SIGINT during a call to system().
-            raise_signal(Signal, !IO),
-            CommandSucceeded = did_not_succeed
-        ;
-            CmdResult = error(Error),
-            report_error(ProgressStream, io.error_message(Error), !IO),
-            CommandSucceeded = did_not_succeed
-        )
-    ;
-        TmpFileResult = error(Error),
-        report_error(ProgressStream,
-            "Could not create temporary file: " ++ error_message(Error), !IO),
-        TmpFile = "",
-        CommandSucceeded = did_not_succeed
-    ),
-
-    ( if
-        MaybeProcessOutput = yes(ProcessOutput),
-        % We can't do bash style redirection on .NET.
-        not use_dotnet
-    then
-        io.file.make_temp_file(ProcessedTmpFileResult, !IO),
-        (
-            ProcessedTmpFileResult = ok(ProcessedTmpFile),
-
-            % XXX we should get rid of use_win32
-            ( if use_win32 then
-                get_system_env_type(Globals, SystemEnvType),
-                ( if SystemEnvType = env_type_powershell then
-                    ProcessOutputRedirected = string.format(
-                        "Get-context %s | %s > %s 2>&1",
-                        [s(TmpFile), s(ProcessOutput), s(ProcessedTmpFile)])
-                else
-                    % On windows, we can't in general redirect standard
-                    % error in the shell.
-                    ProcessOutputRedirected = string.format("%s < %s > %s",
-                        [s(ProcessOutput), s(TmpFile), s(ProcessedTmpFile)])
-                )
-            else
-                ProcessOutputRedirected = string.format("%s < %s > %s 2>&1",
-                    [s(ProcessOutput), s(TmpFile), s(ProcessedTmpFile)])
-            ),
-            (
-                PrintCommand = yes,
-                io.format(ProgressStream,
-                    "%% Invoking system command `%s'...\n",
-                        [s(ProcessOutputRedirected)], !IO),
-                io.flush_output(ProgressStream, !IO)
-            ;
-                PrintCommand = no
-            ),
-            io.call_system.call_system_return_signal(ProcessOutputRedirected,
-                ProcessOutputResult, !IO),
-            io.file.remove_file(TmpFile, _, !IO),
-            (
-                ProcessOutputResult = ok(exited(ProcessOutputStatus)),
-                maybe_write_string(ProgressStream, PrintCommand,
-                    "% done.\n", !IO),
-                ( if ProcessOutputStatus = 0 then
-                    ProcessOutputSucceeded = succeeded
-                else
-                    % The command should have produced output
-                    % describing the error.
-                    ProcessOutputSucceeded = did_not_succeed
-                )
-            ;
-                ProcessOutputResult = ok(signalled(ProcessOutputSignal)),
-                % Make sure the current process gets the signal. Some systems
-                % (e.g. Linux) ignore SIGINT during a call to system().
-                raise_signal(ProcessOutputSignal, !IO),
-                report_error(ProgressStream,
-                    "system command received signal "
-                    ++ int_to_string(ProcessOutputSignal) ++ ".", !IO),
-                ProcessOutputSucceeded = did_not_succeed
-            ;
-                ProcessOutputResult = error(ProcessOutputError),
-                ProcessOutputErrorMsg = io.error_message(ProcessOutputError),
-                report_error(ProgressStream, ProcessOutputErrorMsg, !IO),
-                ProcessOutputSucceeded = did_not_succeed
-            )
-        ;
-            ProcessedTmpFileResult = error(ProcessTmpError),
-            ProcessTmpErrorMsg = io.error_message(ProcessTmpError),
-            report_error(ProgressStream, ProcessTmpErrorMsg, !IO),
-            ProcessOutputSucceeded = did_not_succeed,
-            ProcessedTmpFile = ""
-        )
-    else
-        ProcessOutputSucceeded = succeeded,
-        ProcessedTmpFile = TmpFile
-    ),
-    Succeeded = CommandSucceeded `and` ProcessOutputSucceeded,
-
-    % Write the output to the error stream.
-
-    % XXX Why do we try to do this EVEN WHEN the code above had not Succeeded?
-    io.read_named_file_as_string(ProcessedTmpFile, TmpFileRes, !IO),
-    (
-        TmpFileRes = ok(TmpFileString),
-        io.write_string(CmdOutputStream, TmpFileString, !IO)
-    ;
-        TmpFileRes = error(TmpFileError),
-        report_error(ProgressStream,
-            "error opening command output: " ++ io.error_message(TmpFileError),
-            !IO)
-    ),
-    io.file.remove_file(ProcessedTmpFile, _, !IO),
-    io.set_exit_status(OldStatus, !IO).
-
-make_command_string(String0, QuoteType, String) :-
-    ( if use_win32 then
-        (
-            QuoteType = forward,
-            Quote = " '"
-        ;
-            QuoteType = double,
-            Quote = " """
-        ),
-        string.append_list(["sh -c ", Quote, String0, Quote], String)
-    else
-        String = String0
-    ).
-
-%-----------------------------------------------------------------------------%
-
-    % Are we compiling in a .NET environment?
-    %
-:- pred use_dotnet is semidet.
-:- pragma foreign_proc("C#",
-    use_dotnet,
-    [will_not_call_mercury, promise_pure, thread_safe],
-"
-    SUCCESS_INDICATOR = true;
-").
-% The following clause is only used if there is no matching foreign_proc.
-use_dotnet :-
-    semidet_fail.
-
-    % Are we compiling in a win32 environment?
-    %
-    % If in doubt, use_win32 should succeed. This is only used to decide
-    % whether to invoke Bourne shell command and shell scripts directly,
-    % or whether to invoke them via `sh -c ...'. The latter should work
-    % correctly in a Unix environment too, but is a little less efficient
-    % since it invokes another process.
-    %
-:- pred use_win32 is semidet.
-:- pragma foreign_proc("C",
-    use_win32,
-    [will_not_call_mercury, promise_pure, thread_safe],
-"
-#ifdef MR_WIN32
-    SUCCESS_INDICATOR = 1;
-#else
-    SUCCESS_INDICATOR = 0;
-#endif
-").
-% The following clause is only used if there is no matching foreign_proc.
-% See comment above for why it is OK to just succeed here.
-use_win32 :-
-    semidet_succeed.
-
-%-----------------------------------------------------------------------------%
  %
  % Java command-line utilities.
  %
diff --git a/compiler/notes/compiler_design.html b/compiler/notes/compiler_design.html
index 5f265b0..d788395 100644
--- a/compiler/notes/compiler_design.html
+++ b/compiler/notes/compiler_design.html
@@ -2502,6 +2502,8 @@ from which they can later be restored.
  indent.m provides utilities for dealing with indentation
  in the text that the compiler writes out
  to e.g. target language files and HLDS dumps.
+<li>
+system_command.m contains predicates for invoking commands via the shell.
  </ul>

  <h2>Currently undocumented</h2>


More information about the reviews mailing list