[m-rev.] for review: move all file copying operations to the copy_util module

[Julien Fischer]

On Sat, 6 Jan 2024, Zoltan Somogyi wrote:

> On 2024-01-06 18:15 +11:00 AEDT, "Julien Fischer" <jfischer at opturion.com> wrote:
>>     Split copy_file/7 into two predicates: copy_file_to_file/7 and
>>     copy_file_to_directory/7.
>
> copy_file_to_directory is fine, but copy_file_to_file sounds weird.
> I propose copy_file_to_file_NAME.

Done.

>>     Do not fallback to using the file copy procedure written in Mercury
>
> s/fallback/fall back/

Fixed.

>> +% This module provides predicates for copying files. The Mercury compiler
>> +% copies files for the following reasons:
>
> s/:/./

Fixed.

> I would rewrite the following points using active voice.

Done.

>> +% 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
>
> the the

Fixed.

>> +% 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.
>
> mmake does not use the compiler's code at all except by invoking mmc as a whole,
> so it is strange to mention this.

The point I was trying to get across is that what's in this module has
no effect on mmake; I've deleted that paragraph.

>> +% 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.
>
> I have ideas about improving the wording of the above text, but will make the
> edits myself after you commit. If you leave this section alone until I have done it,
> you won't get a conflict.
>
>>  %-----------------------------------------------------------------------------%
>>
>> @@ -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.
>
> Which means that documenting this requirement, and calling
> expect_not($pred, unify(SourceFile, DestinationFile)), should work.

Yes, I intend to add that and do some bootchecking and other testing. As
a said in the comment, I'm pretty sure we don't every generate such
calls to copy_file, but I'm not 100% certain.

> The rest is fine.

Thanks for that -- I've committed it with the changes above.

Julien.