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

Zoltan Somogyi zoltan.somogyi at runbox.com
Sat Jan 6 19:06:11 AEDT 2024


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.

>     Do not fallback to using the file copy procedure written in Mercury

s/fallback/fall back/

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

s/:/./

I would rewrite the following points using active voice.

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

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

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

The rest is fine.

Zoltan.


More information about the reviews mailing list