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

Julien Fischer jfischer at opturion.com
Mon Aug 19 14:36:59 AEST 2024


On Sat, 17 Aug 2024 at 23:17, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
>
> 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.

So looking at the man page for symlink() on Linux**, it an fail for two
main classes of reason:

1. There is an issue with creating the symlink (insufficient permissions,
I/O error etc).

2. There is an issue with the argument paths (e.g. too long).

The install (if it even gets that far) should definitely be stopped in the first
case. Since the compiler is constructing the argument paths, the second
case shouldn't really be an issue.  (That said, if there are size limits
involved, they could potentially be a problem; that said I don't
recall it arising
in practice.)

** and presumably similar on everything but Windows (and we don't enable
the use of symlinks there anyway).

> 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?

I haven't seen any.

> Do you think the first fix is good enough?
> I myself prefer the second fix, since the extra complexity is very small.

Either will work.

Julien.


More information about the reviews mailing list