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

[Warwick Harvey]

Fergus wrote:
> 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 ;-)

:-)  Fair enough.

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

Ideally.  :-)

> The documentation in the `Using libraries' section of the user manual
> should be updated to mention it.

OK.  Separate change?

RCS file: /home/mercury1/repository/mercury/doc/user_guide.texi,v
retrieving revision 1.176
diff -u -r1.176 user_guide.texi
--- user_guide.texi     1999/06/01 09:44:47     1.176
+++ user_guide.texi     1999/06/17 04:09:53
@@ -739,6 +752,7 @@
 MLLIBS = -lmypackage -lmyotherlib $(EXTRA_MLLIBS)
 C2INITFLAGS = $(MYPACKAGE_DIR)/mypackage.init \
               $(MYOTHERLIB_DIR)/myotherlib.init
+%_init.c: $(C2INITFLAGS)
 
 # This line may be needed if @samp{--intermodule-optimization}
 # is in @samp{MCFLAGS}. @samp{-I} options should be added for any other
@@ -757,6 +771,8 @@
 The extra arguments to @samp{c2init} specified in the @samp{C2INITLFLAGS}
 variable tell @samp{c2init} where to find the @samp{.init} files for the
 libraries, so that it can generate appropriate initialization code.
+The following line declares the dependence of that initialization code on
+those @samp{.init} files.
 The @samp{-I} options in @samp{MGNUCFLAGS} tell the C preprocessor where
 to find the header files for the libraries.
 

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

This seems more problematic.  (I did think about this one.)  The issue (to 
me) is that exactly which file the executable depends on depends upon the 
grade used.  "Ideally" the system should know which grade each file is 
compiled in, and know they need to be rebuilt if the grade is changed.  
Currently this is handled by the user knowing that every time they change 
grade, they need to do an "mmake clean".  :-)  So I suppose one could argue 
that if the grade is changed then Mmake can assume that the relevant files 
have been deleted, and so depending on the library corresponding to the 
current grade is not inappropriate.

Of course, if any of the interface files used were changed when the library 
was rebuilt (or perhaps the optimisation files, which are more likely to 
have changed), then the executable is going to be re-linked anyway, because 
some of its modules will be re-built.  So there seem to me to be two classes 
of change where a dependency on the library would cause a re-link when one 
would not have occurred otherwise: when an unused/irrelevant part of the 
library was changed, and when the implementation was changed without 
altering the interface (e.g. a bug fix).  In the former case the lack of 
re-link is probably not a problem, but in the latter case it could be.

Anyway, I'm not really too fussed either way on this one (assuming the 
implementation is not too curly --- e.g. getting grade-dependent paths into 
the VPATH variable), but I would advocate that if it is deemed that these 
kinds of dependencies are worth adding for user-specified libraries, then 
they are also worth adding for the system libraries (which presumably would 
be easier to do, but is not done currently).

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

OK.  I'll take another look at it once I've sorted out the other stuff, and 
if I still can't see an obvious way to do it, I will comment it as you 
suggest.

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

That's why I put it in.  There are a number of other features that I thought 
could be useful (e.g. being able to specify whether these extra libraries 
should be linked before or after others, being able to override libraries 
rather than just add extras, etc.), but decided that it would be better to 
get a useful subset actually finished and committed rather than worrying 
about them now.  Once this stuff is in use, we can determine from experience 
what is useful, rather than trying to guess it in advance and spending lots 
of time implementing features which turned out to be unimportant (or even 
useless because of something unforseen).

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

OK, will do.

> The difference is just that `@samp{foo}' generates
> 
> 	`foo'
> 
> whereas `@code{foo}' generates
> 
> 	foo
> 
> without the quotes.

Only in the printed manual.  :-)  (For info files, they come out the same 
--- both according to the Texinfo documentation and looking at the output.)

Anyway, the point is that I've now read the Texinfo documentation, and know 
the difference.  :-)

> > +++ 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.

Oops.  I'd had a nagging feeling there was something I'd left that needed 
coming back to, but missed it when going over the diff.  :-)  Sorry.

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

Sure.  So why doesn't it do that already?  ;-)

(I had tried to "solve" this problem in a similar way in Mmake, but found it 
problematic.  For this and other things I in particular had trouble with 
patsubst's lack of respect for quotes, which meant spaces inside backquotes 
were treated as word separators.  In the other cases I ended up using the 
shell command to get around it, but that did not work here because the shell 
command is executed before the patsubst is processed, when it needed to be 
the other way around.)

The way I was actually thinking of doing it was using `cd $dir ; pwd`, which 
would work for both absolute and relative paths (and avoids `..'s in the 
path), but has the drawback of potentially "corrupting" an absolute path.

Anyway, I can change ml to "absolutify" relative paths.  Should this be a 
separate change?

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

Yup, sounds good.

> > -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 ...)'.

Sure does (see above comment about patsubst, spaces and quoting).  I'm not 
sure I'd like to try doing all this stuff without the benefit of patsubst 
and friends.  Do you see a reasonable alternative?

> > +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.

Sure.  I'd thought that since it wouldn't ever be used without them (well, 
OK, it's conceivable it might) it might be easier to specify it just once.  
But your GRADE_STRING suggestion gets us both advantages.

> > +++ 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).

Yes.  I borrowed the term "module" from the place the `.init' files are 
installed: lib/mercury/modules.  Later I realised it was ambiguous since one 
Mercury source file is also a module, but at the time couldn't think of a 
better name to use.  `init-file' is probably a better choice, but the `-i' 
option for c2init is already taken.  I don't have a problem with only having 
long options `--init-file' and `--init-file-directory' since I'll always be 
doing this using Mmake and never having to specify it directly, but others 
may want a shorter option.  Any suggestions?  `-f'/`-F'?

Anyway, I've revised the documentation:

        -m <initfile>, --module <initfile>
                Include initialization of code listed in the \`.init' file
                <initfile> (if the file does not exist in the current
                directory it is searched for in the directories specified
                with the \`-M' option).

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

It should be an `.init' file.  The difference is that with `--module', the 
extra paths are searched to find it.  I didn't think it was a good idea to 
search the extra paths for *all* files specified on the command line.

> > +# Search the extra module directories to locate the extra modules
> > +# and expand to full path names
[...]
> 
> It would be more efficient to do that search in mkinit.c,

Sure, but it was easier to write in shell, with the added bonus that I 
didn't have to worry about C memory management.  (And guess who's fault it 
is that I'm reluctant to exercise my C writing skills much these days?  ;-)

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

Good.  :-)

Anyway, I'll fix all the things that are clear, and post another diff when 
some of the other issues are resolved.  What kind of diff will you want?

Thanks,
Warwick

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