[m-rev.] for review: auto detection of library grades

[Julien Fischer]

On Tue, 3 Sep 2013, Paul Bone wrote:

> On Wed, Aug 28, 2013 at 05:47:00PM +1000, Julien Fischer wrote:
>>
>> For review by anyone.
>>
>>
>> compiler/options.m:
>> 	Add a new option `--auto-detect-libgrades' that can be used to control
>> 	the new feature.  For now, this is a developer-only option and is
>> 	disabled by default -- I will document it properly when it is enabled
>> 	by default.
>
> Why disable this by default?  Is there any specific reason why it might
> break things?

Nothing specific I can think of, but I thought I would leave it until the
change has bootstrapped anyway.  My intention is to eventually remove
the libgrade settings from Mercury configuration file entirely.

> Or do you want to enable it after 13.05.x?

It's not being committed on the 13.05 branch, so it will necessarily
only be enabled after 13.05.x ;-)

The bigger picture here is that this is part of a set of changes that
will hopefully make it easier to deploy Mercury (especially on Windows)
and also to modify existing Mercury installations in various ways (e.g.
add a new library grade, change the C compiler from the one the
installation was configured with etc etc.)  I was intending all of this
stuff for the next release, not 13.05.x.

>> diff --git a/compiler/mercury_compile.m b/compiler/mercury_compile.m
>> index 9e690bb..8623595 100644
>> --- a/compiler/mercury_compile.m
>> +++ b/compiler/mercury_compile.m
>> +%
>> +% Library grade detection.
>> +%
>> +
>> +:- pred auto_detect_libgrades(globals::in, maybe(list(string))::in,
>> +    list(string)::out, io::di, io::uo) is det.
>> +
>> +auto_detect_libgrades(Globals, MaybeConfigMerStdLibDir, GradeOpts, !IO) :-
>> +    globals.lookup_bool_option(Globals, auto_detect_libgrades, AutoDetect),
>> +    (
>> +        AutoDetect = yes,
>> +        ( if
>> +            % Was the standard library directory set on the command line?
>> +            % +            globals.lookup_maybe_string_option(Globals,
>> +                mercury_standard_library_directory, MaybeStdLibDir),
>> +            MaybeStdLibDir = yes(MerStdLibDir)
>
> Some strange formatting in your patch.

I don't think that was me -- it appears to be some combination of git
diff and / or the mailer.

>> +        then
>> +            do_auto_detect_libgrades(MerStdLibDir, GradeOpts, !IO)
>> +        else if
>> +            % Was the standard library directory set using the
>> +            % MERCURY_STDLIB_DIR variable?
>> +            MaybeConfigMerStdLibDir = yes([MerStdLibDir])
>> +        then
>> +            do_auto_detect_libgrades(MerStdLibDir, GradeOpts, !IO)
>> +        else
>> +            GradeOpts = [] +        )
>
> You can factor out common code by putting both ITE conditions in a
> disjunction and then using a single then branch.

Addressed elsewhere in this thread.

>> +:- pred do_auto_detect_libgrades(string::in, list(string)::out,
>> +    io::di, io::uo) is det.
>> +
>> +do_auto_detect_libgrades(StdLibDir, GradeOpts, !IO) :-
>> +    ModulesDir = StdLibDir / "modules",
>> +    dir.foldl2(do_auto_detect_libgrade, ModulesDir, [], MaybeGradeOpts, !IO),
>> +    (
>> +        MaybeGradeOpts = ok(GradeOpts0),
>> +        (
>> +            GradeOpts0 = [],
>> +            GradeOpts = GradeOpts0
>> +        ;
>> +            GradeOpts0 = [_ | _],
>> +            % Override any --libgrades settings from Mercury.config.
>> +            GradeOpts = ["--no-libgrade" | GradeOpts0]
>> +        )
>> +    ;
>> +        MaybeGradeOpts = error(_, _),
>> +        GradeOpts = []
>> +    ).
>> +
>> +:- pred do_auto_detect_libgrade(string::in, string::in, io.file_type::in,
>> +    bool::out, list(string)::in, list(string)::out, io::di, io::uo) is det.
>> +
>> +do_auto_detect_libgrade(DirName, FileName, FileType, Continue, !GradeOpts, !IO) :-
>
> The above line is over 80 charicters long.

Fixed.

> I beleive our coding standards specify something like 74 or 76.

78.

>  There are some other overlong lines as well.

Fixed.

>> +    (
>> +        FileType = directory,
>> +        (
>> +            % We do not generate .init files for the non-C grades so just check
>> +            % for directories in StdLibDir / "modules" containing the name of
>> +            % their base grade.
>> +            %
>> +            ( string.prefix(FileName, "csharp")
>> +            ; string.prefix(FileName, "erlang")
>> +            ; string.prefix(FileName, "java")
>> +            )
>> +        ->
>> +            !:GradeOpts = ["--libgrade", FileName | !.GradeOpts]
>> +        ;
>> +            % For C grades, we check for the presence of the .init file for
>> +            % mer_std to test whether the grade is present or not.
>> +            %
>> +            InitFile = DirName / FileName / "mer_std.init",
>> +            io.check_file_accessibility(InitFile, [read], Result, !IO),
>> +            (
>> +                Result = ok,
>> +                !:GradeOpts = ["--libgrade", FileName | !.GradeOpts]
>> +            ;
>> +                Result = error(_)
>> +            )
>> +        ),
>
> Do we have to handle any of the experimental backends like the bytecode
> backends?

Is there a separate bytecode grade?  The others probably aren't terribly
important until they actually work.  (I guess "il" should probably be
in there though.)

>> diff --git a/compiler/options.m b/compiler/options.m
>> index 40121d8..755a994 100644
>> --- a/compiler/options.m
>> +++ b/compiler/options.m
>> @@ -5786,6 +5789,11 @@ options_help_build_system -->
>>          "\ta directory. The given command will be invoked as",
>>          "\t`<command> <option> <source> <target>'",
>>          "\tto install each directory. The default option is `-r'.",
>> +
>> +        % `--auto-detect-libgrades' is a developer-only that controls
>> +        % whether the compiler should scan the installation directory
>> +        % to determine what standard library grades are available.
>> +
>
> developer-only option that
>
> s/what/which/

Done.

Cheers,
Julien.