[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