[m-rev.] for review: move all file copying operations to the copy_util module
Julien Fischer
jfischer at opturion.com
Sat Jan 6 18:15:38 AEDT 2024
For review by anyone.
I will be committing this one later this evening, since I want start
building on it.
---------------------
Move all file copying operations to the copy_util module.
Document why the compiler copies files and what mechanisms are available for
doing so.
When copying temporary interface files, do not use the Mercury copy_file
procedure as a fallback when invoking an external file copy command fails.
Use the value of the --install-method option to select whether we use an
external command to copy a file, or instead use a library procedure.
compiler/copy_util.m:
Document why the compiler (as opposed to mmake) copies files.
Document the mechanisms available to it for doing so.
Split copy_file/7 into two predicates: copy_file_to_file/7 and
copy_file_to_directory/7. These two predicate correspond to the
actual file copy use cases in the rest of the compiler.
Document some assumptions made by the rest of the compiler about
how file copying behaves.
Do not fallback to using the file copy procedure written in Mercury if
invoking an external command fails when copying a file to a file.
(When copying a file to a directory, we only ever used an external
command.)
Make the choice of file copy mechanism depend on the value of
the --install-method option.
compiler/make.program_target.m:
Use copy_file_to_directory/7 rather than invoking an external command
here directly.
compiler/module_cmds.m:
Use copy_file_to_file/7 here. Conform to changes in the way errors
are reported during file copying. There are couple of spots marked
XXX FILE COPY, where the caller previously assumed an io.error would
be returned to it -- this probably never happened in practice since
when using an external command invoke_system_command does the error
reporting. I'll tidy this up in a later change.
Julien.
diff --git a/compiler/copy_util.m b/compiler/copy_util.m
index 3a15940..c1c75cf 100644
--- a/compiler/copy_util.m
+++ b/compiler/copy_util.m
@@ -9,7 +9,44 @@
%
% File: copy_util.m.
%
-% This module provides a predicate for copying files.
+% This module provides predicates for copying files. The Mercury compiler
+% copies files for the following reasons:
+%
+% 1. As part of an install target with mmc --make, files are copied into the
+% installation directory. (See make.program_target.m.)
+%
+% 2. On systems where symbolic links are not supported (or when symbolic link
+% support is turned off by the user), files are instead copied.
+%
+% 3. Interface file generation copies a temporary version of the the interface
+% file to the actual version whenever the interface is updated.
+% (See module_cmds.m)
+%
+% The above reasons are why the compiler needs to copy files, mmake also needs
+% to copy files, but it does not use the predicates in this module; it always
+% invokes a shell command.
+%
+% This module uses two strategies for copying a file:
+%
+% 1. Invoking an external command (e.g. cp) in the shell. The command is
+% specified using the --install-command option.
+% (XXX in practice the way this is currently implemented only works for the
+% cp command on Unix-like systems; the various file copying commands on
+% Windows -- e.g. copy, xcopy, robocopy -- do not work.)
+%
+% 2. Calling a library procedure, either provided by the underlying platform
+% (NYI) or using a file copy procedure implemented in Mercury (see the
+% predicate do_copy_file/5 below).
+%
+% The choice of mechanism is controlled by the value of the --install-method
+% option. Regardless of the mechanism, we must ensure that at least some of the
+% file metadata (notably execute bits on those system that use them) is copied.
+% (XXX copying via library procedure does *not* currently do this).
+%
+% XXX this module should eventually also provide a mechanism for copying
+% entire directory trees (see install_directory/7 in make.program_target.m),
+% but currently we only support doing that via invoking an external command
+% in the shell.
%
%-----------------------------------------------------------------------------%
@@ -18,44 +55,106 @@
:- import_module libs.file_util.
:- import_module libs.globals.
+:- import_module libs.maybe_util.
:- import_module io.
%-----------------------------------------------------------------------------%
- % copy_file(Globals, ProgressStream, Source, Destination, Succeeded, !IO).
+ % copy_file(Globals, ProgressStream, SourceFile, DestinationFile,
+ % Succeeded, !IO):
+ %
+ % Copy SourceFile to DestinationFile. Both SourceFile and DestinationFile
+ % must be file paths, not directory paths.
+ % If DestinationFile already exists, it will be overwritten and replaced.
+ %
+ % XXX what if SourceFile = DestinationFile? I (juliensf), don't think
+ % the Mercury compiler actually does a file copy operation like that.
+ %
+:- pred copy_file_to_file(globals::in, io.text_output_stream::in,
+ file_name::in, file_name::in, maybe_succeeded::out, io::di, io::uo) is det.
+
+ % copy_file_to_directory(Globals, ProgressStream, SourceFile,
+ % DestinationDir, Succeeded, !IO):
%
- % XXX A version of this predicate belongs in the standard library.
+ % Copy SourceFile into the directory DestinationDir, that is a file with
+ % the basename of SourceFile will be created in DestinationDir.
+ % This predicate assumes that DestinationDir exists and is writable.
+ % If the destination file already exists, it will be overwritten and
+ % replaced.
%
-:- pred copy_file(globals::in, io.text_output_stream::in,
- file_name::in, file_name::in, io.res::out, io::di, io::uo) is det.
+:- pred copy_file_to_directory(globals::in, io.text_output_stream::in,
+ file_name::in, dir_name::in, maybe_succeeded::out, io::di, io::uo) is det.
%-----------------------------------------------------------------------------%
%-----------------------------------------------------------------------------%
:- implementation.
-:- import_module libs.maybe_util.
:- import_module libs.system_cmds.
:- import_module bool.
+:- import_module dir.
:- import_module int.
+:- import_module string.
%-----------------------------------------------------------------------------%
-copy_file(Globals, ProgressStream, Source, Destination, Res, !IO) :-
- % Try to use the system's cp command in order to preserve metadata.
- Command = make_install_file_command(Globals, Source, Destination),
- invoke_system_command(Globals, ProgressStream, ProgressStream,
- cmd_verbose, Command, Succeeded, !IO),
+copy_file_to_file(Globals, ProgressStream, SourceFile, DestinationFile,
+ Succeeded, !IO) :-
+ globals.get_install_method(Globals, InstallMethod),
(
- Succeeded = succeeded,
- Res = ok
+ InstallMethod = install_method_external_cmd,
+ Command = make_install_file_command(Globals, SourceFile, DestinationFile),
+ invoke_system_command(Globals, ProgressStream, ProgressStream,
+ cmd_verbose, Command, Succeeded, !IO)
+ ;
+ InstallMethod = install_method_internal_code,
+ do_copy_file(SourceFile, DestinationFile, Res, !IO),
+ (
+ Res = ok,
+ Succeeded = succeeded
+ ;
+ Res = error(Error),
+ report_error(ProgressStream,
+ "Could not copy file: " ++ error_message(Error), !IO),
+ Succeeded = did_not_succeed
+ )
+ ).
+
+%-----------------------------------------------------------------------------%
+
+copy_file_to_directory(Globals, ProgressStream, SourceFile, DestinationDir,
+ Succeeded, !IO) :-
+ globals.get_install_method(Globals, InstallMethod),
+ (
+ InstallMethod = install_method_external_cmd,
+ Command = make_install_file_command(Globals, SourceFile, DestinationDir),
+ invoke_system_command(Globals, ProgressStream, ProgressStream,
+ cmd_verbose, Command, Succeeded, !IO)
;
- Succeeded = did_not_succeed,
- do_copy_file(Source, Destination, Res, !IO)
+ InstallMethod = install_method_internal_code,
+ ( if dir.basename(SourceFile, BaseSourceFile) then
+ DestinationFile = DestinationDir / BaseSourceFile,
+ do_copy_file(SourceFile, DestinationFile, Res, !IO),
+ (
+ Res = ok,
+ Succeeded = succeeded
+ ;
+ Res = error(Error),
+ report_error(ProgressStream,
+ "Could not copy file: " ++ error_message(Error), !IO),
+ Succeeded = did_not_succeed
+ )
+ else
+ report_error(ProgressStream,
+ "Could not copy file: source is a root directory.", !IO),
+ Succeeded = did_not_succeed
+ )
).
+%-----------------------------------------------------------------------------%
+
% XXX TODO: copying the file byte-by-byte is inefficient.
% If the OS or platform we are on provides a system call for copying files,
% we should use that in preference to the code below.
diff --git a/compiler/make.program_target.m b/compiler/make.program_target.m
index 7de4f54..a73d7c4 100644
--- a/compiler/make.program_target.m
+++ b/compiler/make.program_target.m
@@ -70,6 +70,7 @@
:- import_module backend_libs.compile_target_code.
:- import_module libs.check_libgrades.
:- import_module libs.compute_grade.
+:- import_module libs.copy_util.
:- import_module libs.file_util.
:- import_module libs.handle_options.
:- import_module libs.options.
@@ -2102,13 +2103,11 @@ maybe_install_library_file(ProgressStream, Globals, Linkage,
install_file(ProgressStream, Globals, FileName, InstallDir, Succeeded, !IO) :-
% XXX MAKE_STREAM
- OutputStream = ProgressStream,
verbose_make_four_part_msg(Globals, "Installing file", FileName,
"in", InstallDir, InstallMsg),
maybe_write_msg(ProgressStream, InstallMsg, !IO),
- Command = make_install_file_command(Globals, FileName, InstallDir),
- invoke_system_command(Globals, ProgressStream, OutputStream,
- cmd_verbose, Command, Succeeded, !IO).
+ copy_file_to_directory(Globals, ProgressStream, FileName,
+ InstallDir, Succeeded, !IO).
:- pred install_directory(io.text_output_stream::in, globals::in,
dir_name::in, dir_name::in, maybe_succeeded::out, io::di, io::uo) is det.
diff --git a/compiler/module_cmds.m b/compiler/module_cmds.m
index 676164e..1d5c241 100644
--- a/compiler/module_cmds.m
+++ b/compiler/module_cmds.m
@@ -260,16 +260,21 @@ copy_dot_tmp_to_base_file_create_file(Globals, ProgressStream,
string.format("%% `%s' has %s.\n", [s(OutputFileName), s(ChangedStr)],
ChangedMsg),
maybe_write_string(ProgressStream, Verbose, ChangedMsg, !IO),
- copy_file(Globals, ProgressStream, TmpOutputFileName, OutputFileName,
- MoveRes, !IO),
+ copy_file_to_file(Globals, ProgressStream, TmpOutputFileName,
+ OutputFileName, MoveRes, !IO),
(
- MoveRes = ok,
+ MoveRes = succeeded,
Result = base_file_new_or_changed
;
- MoveRes = error(MoveError),
- Result = dot_tmp_copy_error,
- io.format(ProgressStream, "Error creating `%s': %s\n",
- [s(OutputFileName), s(io.error_message(MoveError))], !IO)
+ MoveRes = did_not_succeed,
+ Result = dot_tmp_copy_error
+ % copy_file_to_file/7 writes an error message to ProgressStream
+ % if the copy fails.
+ % XXX FILE COPY: we used to generate the following error message
+ % here, but I suspect it wasn't really possible to trigger it.
+ %MoveRes = error(MoveError),
+ %io.format(ProgressStream, "Error creating `%s': %s\n",
+ % [s(OutputFileName), s(io.error_message(MoveError))], !IO)
),
io.file.remove_file(TmpOutputFileName, _, !IO).
@@ -311,10 +316,23 @@ make_symlink_or_copy_file(Globals, ProgressStream,
;
PrintCommand = no
),
- io.file.make_symlink(SourceFileName, DestinationFileName, Result, !IO)
+ io.file.make_symlink(SourceFileName, DestinationFileName, Result, !IO),
+ (
+ Result = ok,
+ Succeeded = succeeded
+ ;
+ Result = error(Error),
+ Succeeded = did_not_succeed,
+ io.progname_base("mercury_compile", ProgName, !IO),
+ io.error_message(Error, ErrorMsg),
+ io.format(ProgressStream, "%s: error %s `%s' to `%s', %s\n",
+ [s(ProgName), s(LinkOrCopy), s(SourceFileName),
+ s(DestinationFileName), s(ErrorMsg)], !IO),
+ io.flush_output(ProgressStream, !IO)
+ )
;
UseSymLinks = no,
- LinkOrCopy = "copying",
+ %LinkOrCopy = "copying",
(
PrintCommand = yes,
io.format(ProgressStream, "%% Copying file `%s' -> `%s'\n",
@@ -323,21 +341,12 @@ make_symlink_or_copy_file(Globals, ProgressStream,
;
PrintCommand = no
),
- copy_file(Globals, ProgressStream, SourceFileName, DestinationFileName,
- Result, !IO)
- ),
- (
- Result = ok,
- Succeeded = succeeded
- ;
- Result = error(Error),
- Succeeded = did_not_succeed,
- io.progname_base("mercury_compile", ProgName, !IO),
- io.error_message(Error, ErrorMsg),
- io.format(ProgressStream, "%s: error %s `%s' to `%s', %s\n",
- [s(ProgName), s(LinkOrCopy), s(SourceFileName),
- s(DestinationFileName), s(ErrorMsg)], !IO),
- io.flush_output(ProgressStream, !IO)
+ % XXX FILE COPY
+ % copy_file_to_file/7 will write an error message to ProgressStream
+ % itself if the file copy fails. We used to generate an error similar
+ % to the symlink case above here.
+ copy_file_to_file(Globals, ProgressStream, SourceFileName,
+ DestinationFileName, Succeeded, !IO)
).
make_symlink_or_copy_dir(Globals, ProgressStream,
More information about the reviews
mailing list