[m-rev.] for review: fix problems with mmc --make

[Julien Fischer]

On Fri, 2 Dec 2005, Peter Wang wrote:

> Estimated hours taken: 8
> Branches: main
>
> Fix some problems with `mmc --make'.
>
> compiler/compile_target_code.m:
> 	In `link' predicate, after the file has been created in a grade
> 	subdirectory it would try to symlink or copy the file into the user
> 	directory.  This would fail if a file of the same name already existed
> 	there, so remove it before attempting the symlink/copy.
>
> compiler/make.program_target.m:
> 	Prevent `mmc --make libFOO' from trying to produce a shared library
> 	if that is unsupported on the current architecture.
>
s/that is/they are/

> 	Disable the logic to conditionally generate `.mh' files so as to avoid
> 	trouble with Mmake, which expects them to always exist.
>
> 	Fix a typo that tried to install `.mih' files into `int' directories
> 	instead of `ints'.
>
> compiler/modules.m:
> 	Make `make_symlink_or_copy_file' write a newline after errors.
>
> Index: compiler/compile_target_code.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/compile_target_code.m,v
> retrieving revision 1.77
> diff -u -r1.77 compile_target_code.m
> --- compiler/compile_target_code.m	28 Nov 2005 02:30:18 -0000	1.77
> +++ compiler/compile_target_code.m	2 Dec 2005 05:00:45 -0000
> @@ -1373,6 +1373,7 @@
>          globals__io_set_option(use_grade_subdirs, bool(yes), !IO),
>
>          io__set_output_stream(ErrorStream, OutputStream, !IO),
> +        io__remove_file(UserDirFileName, _, !IO),

Don't ignore the result of this call.   Something like the following would
be better:

	io__remove_file(UserDirFileName, RemoveResult, !IO),
	(
		RemoveResult = ok
	;
		RemoveResult = error(RemoveError)
		... call unexpected/2 etc...
	)

> Index: compiler/make.program_target.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/make.program_target.m,v
> retrieving revision 1.34
> diff -u -r1.34 make.program_target.m
> --- compiler/make.program_target.m	28 Nov 2005 04:11:45 -0000	1.34
> +++ compiler/make.program_target.m	2 Dec 2005 05:18:00 -0000
> @@ -516,10 +516,17 @@
>                  InitSucceeded = yes,
>                  make_linked_target(MainModuleName - static_library,
>                      StaticSucceeded, !Info, !IO),
> +                shared_libraries_supported(SharedLibsSupported, !IO),
>                  (
>                      StaticSucceeded = yes,
> -                    make_linked_target(MainModuleName - shared_library,
> -                        Succeeded, !Info, !IO)
> +                    (
> +                        SharedLibsSupported = yes,
> +                        make_linked_target(MainModuleName - shared_library,
> +                            Succeeded, !Info, !IO)
> +                    ;
> +                        SharedLibsSupported = no,
> +                        Succeeded = yes
> +                    )
>                  ;
>                      StaticSucceeded = no,
>                      Succeeded = no
> @@ -545,6 +552,14 @@
>          )
>      ).
>
> +:- pred shared_libraries_supported(bool::out, io::di, io::uo) is det.
> +
> +shared_libraries_supported(Supported, !IO) :-
> +    globals__io_lookup_string_option(library_extension, LibExt, !IO),
> +    globals__io_lookup_string_option(shared_library_extension, SharedLibExt,
> +        !IO),
> +    Supported = (if LibExt \= SharedLibExt then yes else no).
> +
This does not seem particularly robust.  I think a better approach here would
be to have configure set a macro in mercury_conf.h (MR_HAVE_SHARED_LIB or
something) and test that here.

>  %-----------------------------------------------------------------------------%
>
>  :- pred install_library(module_name::in, bool::out,
> @@ -626,10 +641,13 @@
>
>          globals__io_get_target(Target, !IO),
>          (
> -            % `.mh' files are only generated for modules containing
> +            % `.mh' files are (were) only generated for modules containing
>              % `:- pragma export' declarations.
> -            ( Target = c ; Target = asm ),
> -            Imports ^ contains_foreign_export = contains_foreign_export
> +            % But `.mh' files are expected by Mmake so always generate them,
> +            % otherwise there is trouble using libraries installed by
> +            % `mmc --make' with Mmake.
> +            ( Target = c ; Target = asm )
> +            % Imports ^ contains_foreign_export = contains_foreign_export

I take it mmc --make doesn't always require .mh?  If so please mark
that comment with an XXX and mention that if we ever phase out mmake
we can get rid of this behaviour.

The rest of that looks fine.

Cheers,
Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------