[m-dev.] For review: using alternate installation dirs

[Fergus Henderson]

On 14-Jun-1999, Warwick Harvey <wharvey at cs.monash.edu.au> wrote:
> Anyway, at this stage I've only implemented the *use* of these directories
> once installed; installing to them is still to be done.  I don't intend to
> commit this stuff until the second part is done, unless somebody convinces
> me doing so would be worthwhile.

I think it might well be worthwhile, because it would make using libraries
easier, even if installing them is no easier.  In other words, this change
is useful in its own right.  (In addition, committing it now it would
reduce the likely number of cvs conflicts that you need to resolve when
you finally commit it, and would also be useful in case you get run over
by a bus or succumb to the dark side ;-)

> The first issue is what (if anything) to do about the dependency between the 
> `_init.c' file for the program being built and the `.init' files of the 
> libraries.  It is not mentioned in the `Using libraries' section of the user 
> manual, and similar dependencies on the `.init' files for the standard 
> libraries are also ignored.  However, a number of sample programs in the 
> extras directory declare it (references, trailed_update, clpr --- but they 
> probably all inherited it from the one source, whichever it was that came 
> first).

Well, the dependency is real, so ideally it should be included.
The documentation in the `Using libraries' section of the user manual
should be updated to mention it.

There should also be a dependency for the final executable (or .so file)
on the library files (.a or .so) that it is linked with.

The only time that it is really _crucial_ to include those dependencies
are when the `.init' file or the library files in question reside in
the same directory as the program using the library.  In that case,
if the dependencies aren't included, then when you type
`mmake realclean; mmake depend; mmake', mmake may try to build the files
that depend on the `.init' file before the `.init' has been created,
resulting in a build error.  (This was once the case with the clpr examples,
which used to be in the same directory as the clpr library.  The references
and trailed_update Mmakefiles were almost certainly copied from
clpr/Mmakefile.)

So if this is too difficult to implement, then for now I suggest that you
just document it with XXX comments in the appropriate places.

> Certainly ignoring the dependency is easier than including it.  :-)  Though
> adding it is not too hard, as long as one doesn't also want to be able to
> specify which libraries to link on a program-by-program basis (i.e. using
> something like `EXTRA_LIBRARIES-foo = bar') --- this appears to make it
> downright nasty.

I think specifying which libraries to link on a program-by-program basis
is something which might be needed quite often.

> While doing this I discovered that adding extra C2INITFLAGS for a program 
> probably doesn't work as desired.  To add extra flags for the program `foo', 
> one has to define `C2INITFLAGS-foo_init.c' rather than `C2INITFLAGS-foo'.  
> (Debugging Mmakefiles sux :-).  I presume this wasn't intended?  I can fix 
> this as a separate change if it is desired.

Yes, it's not intended.

There is a similar issue with GRADEFLAGS, where you need to set both
`GRADEFLAGS-foo' and also `GRADEFLAGS-foo_init' (e.g. this is done in
tests/hard_coded/Mmakefile).  I had a look at this issue a while ago
and didn't see any easy way of fixing it.  But the technique used in
your proposed fix (shown below) looks great.

	> +# The above should probably be redefined as below
	> +#TARGET_C2INITFLAGS = \
	> +#  $(maybe-target-C2INITFLAGS-$(findstring undefined,\
	> +#		  $(origin C2INITFLAGS-$(subst _init.c,,$@))))
	> +#maybe-target-C2INITFLAGS- = $(C2INITFLAGS-$(subst _init.c,,$@))
	> +#maybe-target-C2INITFLAGS-undefined =

So yes, it would be great if you could fix that problem -- for GRADEFLAGS
as well as C2INITFLAGS -- as a separate change.

> Anyway, here's the diff.  As well as the usual kinds of comments, I'd 
> particularly like it if somebody could examine the TeXinfo to make sure I 
> haven't done anything stupid or naughty (in general, as well as with regard 
> to Mercury standards) because I've not done any TeXinfo hacking before.  
> (Also, the existing documentation I browsed in an attempt to deduce what to 
> do seems to be inconsistent as to what goes in @code{}, what goes in 
> @samp{}, and what doesn't need either --- are there guidelines for this?)

The TexInfo documentation for `@samp' and `@code' gives guidelines for when
they should be used.  You should read those.  But basically:

	- anything which is source code or command lines should go
	  in either @code or @samp
	- @samp has a stronger emphasis; generally the first occurrence
	  of every term should go in @samp, but if a term is repeated a lot
	  then subsequent occurrences can go in @code.

The difference is just that `@samp{foo}' generates

	`foo'

whereas `@code{foo}' generates

	foo

without the quotes.

> +++ user_guide.texi	1999/06/14 03:59:36
> @@ -519,6 +519,18 @@
>  Options to pass to the c2init program.
>  (Note that compilation model options should be
>  specified in @code{GRADEFLAGS}, not in @code{C2INITFLAGS}.)
> +
> + at item EXTRA_LIBRARIES
> +A list of extra Mercury libraries to link into any programs or libraries
> +that you are building.
> +Libraries should be specified using their base name; that is, without any
> + at code{lib} prefix or extension.
> +For example the library including the files @file{libfoo.a} and @file{foo.init}
> +WWW

That looks incomplete.

> +libraries, etc.  Note that for shared libraries to work properly, absolute
> +paths must be given for @samp{EXTRA_LIB_DIRS}.

Wouldn't it be straight-forward for `ml' to check whether the shared library
directories specified are absolute paths, and if not, to prepend the output
of `pwd' to them?

This would not necessarily do the right thing in the case of nfs-mounted
file systems mounted in different directories on different hosts,
but for most people that would not be an issue.

> Index: scripts/Mmake.vars.in
> +EXTRA_C_LIB_DIRS	= \
> +	$(patsubst %,%/lib/$(shell $(MCOGS))/@FULLARCH@,$(EXTRA_LIB_DIRS)) \
> +	$(patsubst %,%/lib/@FULLARCH@,$(EXTRA_LIB_DIRS))

I think it would be better (more readable, and possible also more flexible)
to have a variable GRADE_STRING which was defined as

	GRADE_STRING = $(shell $(MCOGS))

and to use that here.

> -MMAKE_VPATH	= $(MERCURY_INT_DIR)
> +# We need to substitute all spaces with colons for the VPATH to work.
> +# Getting a space to be recognised as the first argument of the subst
> +# function is problematic; hence the `$(nullstring)' hack.
> +nullstring	=
> +MMAKE_VPATH	= $(subst $(nullstring) ,:,$(strip \
> +			$(MERCURY_EXTRA_INT_DIRS) $(MERCURY_INT_DIR)\
> +			$(MERCURY_EXTRA_MOD_LIB_DIRS)))

Hmm.  One thing that bears thinking about is supporting path names
containing spaces -- on Windows, it's common practice to install
things into a subdirectory of the "Program Files" directory.
Perhaps it would be appropriate to put an XXX comment here
saying that it won't handle directory names containing spaces.
I guess the same applies to all uses of `$(patsubst ...)'.

>  MC		= mmc
> -ALL_MCFLAGS	= $(MCFLAGS) $(EXTRA_MCFLAGS) $(TARGET_MCFLAGS)
> +ALL_MCFLAGS	= $(MCFLAGS) $(EXTRA_MCFLAGS) $(TARGET_MCFLAGS) $(LIB_MCFLAGS)
>  MCFLAGS		=
>  EXTRA_MCFLAGS	=
> +LIB_MCFLAGS	= $(patsubst %,-I %,$(EXTRA_INT_DIRS))

>  MCS		= $(MC) --split-c-files --compile-only
>  MCG		= $(MC) --compile-to-c
> @@ -41,15 +61,24 @@
>  MCSI		= $(MC) --make-short-interface
>  MCOI		= $(MC) --make-optimization-interface
>  MCTOI		= $(MC) --make-transitive-optimization-interface
> +MCOGS		= $(MC) --output-grade-string $(ALL_GRADEFLAGS)

For consistency with the other definitions of MCxyz variables,
I think $(ALL_GRADEFLAGS) should go in place(s) that use $MCOGS
rather than in its definition.

> +++ c2init.in	1999/06/14 03:59:36
> @@ -41,6 +41,11 @@
>  		(declared in \"init.h\") that can be called from C code.
>  		(A more fine-grained interface is also available;
>  		see \"init.h\" for details.)
> +	-m <module>, --module <module>
> +		Include initialization of code listed in the file <module>.

The documentation here is not very clear.

What kind of file is `<module>' supposed to be?
Given just that documentation, one could easily assume
it is the name of a Mercury module (`.m' file).

>From reading the source code, it looks like you're supposed to pass
the name of a `.init' file or a `.c' file.  What's the difference
between passing them on the command line as arguments and
passing them as `--module' options?  Why do both possibilities exist?

> +# Search the extra module directories to locate the extra modules
> +# and expand to full path names
> +EXTRA_MOD_LIB_MODS=
> +for module in $extra_modules 'kludge for broken shells' ; do
> +    if [ "$module" = 'kludge for broken shells' ] ; then
> +    	true
> +    else
> +    	if [ -e "$module" ] ; then
> +	    # Appropriate absolute or relative path specified
> +	    qualified_module=$module
> +	else
> +	    for dir in $extra_mod_dirs $MERCURY_MOD_LIB_DIR \
> +	    		'sentinel for when not found' ; do
> +		if [ "$dir" = 'sentinel for when not found' ] ; then
> +		    # Pass it through as is and hope mkinit knows how to
> +		    # deal with it.
> +		    qualified_module=$module
> +		else
> +		    if [ -e "$dir/$module" ] ; then
> +		    	# Found it
> +			qualified_module=$dir/$module
> +			break
> +		    fi
> +		fi
> +	    done
> +	fi
> +	EXTRA_MOD_LIB_MODS="$EXTRA_MOD_LIB_MODS $qualified_module"
> +    fi
> +done

It would be more efficient to do that search in mkinit.c,
because that way you don't need to invoke a new process
(`[', a.k.a. `test') every time you want to test existence of a file.
On some systems, /bin/sh optimizes this case, but on many systems,
including Windows with Cygwin, it doesn't.  On Windows, process
invocation is quite expensive.

But this is a minor point; I'd rather you commit it as is and implement the
support for installing libraries before we worry about optimizing it.

Apart from that, it looks good.
But this change will need another round of reviewing.

Cheers,
	Fergus.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------