[m-rev.] for post-commit review: implement PROPOSED searches for .a, .init, .jar and .dll files

Julien Fischer jfischer at opturion.com
Wed Dec 4 23:37:50 AEDT 2024


On Wed, 4 Dec 2024 at 02:58, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:

> On Tue, 3 Dec 2024 19:10:19 +1100, Julien Fischer <jfischer at opturion.com> wrote:
> > > I think that the conceptually cleanest solution of this bug is to make
> > > the --libgrade option serve only the second purpose, eliminating (1)
> > > from the preference order entirely. We would compute the set of installed
> > > stdlib grades in handle_options.m just after we compute chosen_stdlib_dir,
> > > and record the result in a new slot in the globals structure.
> >
> > That seems a reasonable suggestion.
>
> Then I will implement it, including the include/exclude-component bits.
>
> But before I do, I wanted to separate out the code that sets up chosen_stdlib_dir.
> The attached diff does this. It has no logic changes that need to be reviewed,
> but it does have a big new XXX that describes an issue that would arise if/when
> we started using chosen_stdlib_dir more widely. I am requesting feedback
> on that XXX, and particularly on the two pathways forward it describes.

> diff --git a/compiler/handle_options.m b/compiler/handle_options.m
> index 940d79829..de95f64a6 100644
> --- a/compiler/handle_options.m
> +++ b/compiler/handle_options.m

> @@ -2760,10 +2837,59 @@ handle_directory_options(OpMode, MaybeEnvOptFileMerStdLibDir,
>      ),
>
>      % Add the standard library directory.
> +    %
> +    % XXX We should be looking up the value of the chosen_stdlib_dir option,
> +    % not the mercury_standard_library_directory option. However, doing that
> +    % results in the failure of the foreign_decl_line_number and gh72_errors
> +    % test cases in tests/invalid.
> +    %
> +    % The symptom of those failures is an error message about not being able
> +    % to access the nonexistent directory /usr/local/mercury-DEV/lib/mercury
> +    % to check the set of installed grades. (That directory is the default
> +    % install prefix set by the configure script.)
> +    %
> +    % The reason why we get *only* those two failures (in a C grade) is that
> +    %
> +    % - in the test directories that want to generate (and then test)
> +    %   executables, mmake invokes mmc only to generate target code,
> +    %   with the C compiler then being invoked by mmake itself,
> +    %   which makes mmc's op_mode opmau_generate_code(opmcg_target_code_only),

Although that does raise the question of what happens in the non-C grades?
(When we bootcheck in those grade the test suite will be run with mmake's
--use-mmc-make option.)

> +    % - in the test directories that want to check error messages,
> +    %   by default we invoke mmc with --errorcheck-only,
> +    %   which makes mmc's op_mode opmau_errorcheck_only, and
> +    %
> +    % - both of those op_modes will cause the if-then-else at the top of
> +    %   this predicate to set libgrade_install_check to "no".
> +    %
> +    % The reason why foreign_decl_line_number and gh72_errors fail is that
> +    % they both have --no-errorcheck-only as a module-specific mmc flag,
> +    % which causes that same if-then-else to leave libgrade_install_check
> +    % with its default value of "yes".
> +    %
> +    % I (zs) see two ways to avoid this problem. One way would be to decouple
> +    % the notions of
> +    %
> +    % - the location of the standard library for libgrade_install_checks
> +    %   during bootchecks in a workspace, and
> +    %
> +    % - the location where a "mmake install" in that workspace
> +    %   would put the the standard library
> +    %
> +    % by specifying a real, existing stdlib location (probably the stdlib
> +    % of the installed compiler) during bootchecks.

That seems likely to result in difficult-to-debug bugs ...

> +    % The other way would be to decide that we don't want to do
> +    % libgrade_install_checks during bootchecks at all, except possibly
> +    % for a few tests that specifically want to test the
> +    % libgrade_install_check code itself. We would then specify
> +    % --no-libgrade-install-check as the default for the tests directory.

... consequently, I would prefer this second approach.

Julien.


More information about the reviews mailing list