[m-rev.] for post-commit review: adding some XXX to make.library_install.m

[Zoltan Somogyi]

The code regrouping is trivial and does not need review; I am
seeking feedback on the new XXXs. There are two.

One is about install directory names. Some code in this module still
constructs the names of directories containing files with a given extension
by taking the extension string and adding an "s" suffix. Some other code
has been updated. This means that the current code of this module

- will  try to create a symlink for a directory names "analysiss" for containing
  .analysis files,

- but if it fails, it will create an actual directory named NOT "analysiss",
  but "analyses", our new and better name.

I am pretty sure that this came about because when we switched to the "analyses"
name, I grepped the source for "analysiss" and replaced all occurrences
with "analyses", but of course this did not find the code that adds an "s"
to the end of an argument that happended to have the value "analysis".

I expect that the only reason this has not been a problem is that we don't
actually use .analysis files.

The other XXX is about the symlink vs actual directory question. Calls to
the maybe_make_symlink predicate can "fail" for two reasons:

1 because use_symlinks is not enabled, or
2 because use_symlinks is enabled, we try to create a symlink, and the attempt fails.

The second XXX describes why I think the existing code handles any failures
of the second kind incorrectly.

I see two possible fixes.

The simpler fix assumes that we don't need to recover from any failure
of the second kind; that simply detecting and reporting such failures,
and then stopping the install, is fine.

The more complex fix replaces the simple LinkSucceeded outputs of
make_install_dir and make_grade_install_dirs with a map from directory name
to a flag of a bespoke type that says *for each directory that the given predicate
creates* whether it was created as a symlink or as an actual directory.

The second fix is more robust, but given that I don't recall seeing any issues
with the existing code, which *would* have issues if the simpler fix wouldn't work,
maybe the second fix is overkill.

Have you guys seen any such issues? Do you think the first fix is good enough?
I myself prefer the second fix, since the extra complexity is very small.

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Log.mli1
Type: application/octet-stream
Size: 59 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20240817/00f0dec1/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.mli1
Type: application/octet-stream
Size: 8761 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20240817/00f0dec1/attachment-0001.obj>