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

[Warwick Harvey]

Fergus wrote:
> On 23-Jul-1999, Warwick Harvey <wharvey at cs.monash.edu.au> wrote:
> > +EXTRA_LIB_DIRS = /home/wharvey/mercury/libdirs/extras/lib/mercury
> > +EXTRA_LIBRARIES = global
> 
> I know this is just a sample diff, not one that you're going to commit,
> but shouldn't that be
> 
> 	EXTRA_LIB_DIRS = ..
> 
> rather than using a hard-coded path name?  I think now that ml
> expands relative path names there should be no need to use an
> absolute path here.

No, it shouldn't be `..'.  I did consider changing it to a relative path 
after fixing that problem, but couldn't think of anything sensible, since 
the path specified should be the path to the installation hierarchy, not to 
the source directory.  I expect that when the installation side of things is 
done, I'll set it to `$(MERCURY_INSTALL_PREFIX)/extras' or something else 
appropriate.

> > Index: scripts/mmake.in
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/scripts/mmake.in,v
> > retrieving revision 1.28
> > diff -u -r1.28 mmake.in
> > --- mmake.in	1999/07/20 03:39:18	1.28
> > +++ mmake.in	1999/07/23 02:46:57
> > @@ -66,6 +66,8 @@
> >  MMAKE_VARS=${MMAKE_VARS=$MMAKE_DIR/Mmake.vars}
> >  MMAKE_RULES=${MMAKE_RULES=$MMAKE_DIR/Mmake.rules}
> >  MERCURY_INT_DIR=${MERCURY_INT_DIR=@LIBDIR@/ints}
> > +MERCURY_EXTRA_INT_DIRS=${MERCURY_EXTRA_INT_DIRS=}
> > +MERCURY_EXTRA_INIT_LIB_DIRS=${MERCURY_EXTRA_INIT_LIB_DIRS=}
> >  MERCURY_DEFAULT_GRADE=${MERCURY_DEFAULT_GRADE=@DEFAULT_GRADE@}
> >  MKTEMP=@MKTEMP@
> 
> Adding these variable definitions here doesn't make sense, because
> those definitions are not used.  Perhaps you intended to also
> export them?

I know it doesn't make sense.  The first time around I included the 
following in the message requesting a review:

  Please note that I also have not been able to figure out why the 
definitions
  of `MERCURY_EXTRA_INT_DIRS' and `MERCURY_EXTRA_MOD_LIB_DIRS' in `mmake.in' 
  are required for `VPATH' to work.  If somebody could enlighten me, I'd
  appreciate it.  :-)

Early on in my work on this stuff, I couldn't seem to get VPATH to work 
correctly without having them there.  Beats me why.  Commenting them out now 
seems to leave it still working, so I don't know why I had a problem back 
then.  Either I was mistaken, or something else screwy was going on.  
Anyway, I've removed them.

> > Index: util/mkinit.c
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/util/mkinit.c,v
> > retrieving revision 1.52
> > diff -u -r1.52 mkinit.c
> > --- mkinit.c	1999/06/29 19:08:32	1.52
> > +++ mkinit.c	1999/07/23 02:46:57
> > @@ -23,6 +23,8 @@
> >  #include	<string.h>
> >  #include	<ctype.h>
> >  #include	<errno.h>
> > +#include	<unistd.h>
> > +#include	<sys/stat.h>
> 
> Those header files are part of POSIX, but they are not standard ANSI/ISO C,
> so you should avoid using them if possible.
> If not, then you really ought to use autoconf to check that they exist,
> and protect the #include statements with #ifdefs, and do something
> appropriate in the case when they don't exist.

Are these files not used in `library/io.m'?  `io__stream_file_size/4' uses 
`fstat()' to determine the size of a file...  So I figured the approach I'd 
taken was "safe".

Of course, if you want to fix io.m, I daresay I'd be happy to apply similar 
changes to mkinit.  ;-)

> Anyway, I think at very least it's worth abstracting the code to check
> whether a file exists into a separate function.
> 
> 	static bool file_exists(const char *filename);

Makes sense.

> > +		case 'I':
[...]
> It would be good to add a comment here saying "append the directory
> name into the end of the list" or something like that.

Good idea.

Please find below a relative diff of mkinit.c.

Warwick

--- util/mkinit.c.orig  Sat Jul 24 19:23:34 1999
+++ util/mkinit.c       Sat Jul 24 19:44:00 1999
@@ -239,6 +239,7 @@
 static void usage(void);
 static void do_path_search(void);
 static char *find_init_file(const char *basename);
+static bool file_exists(const char *filename);
 static void output_headers(void);
 static void output_sub_init_functions(void);
 static void output_main_init_function(void);
@@ -334,6 +335,10 @@
                        break;
 
                case 'I':
+                       /*
+                       ** Add the directory name to the end of the
+                       ** search path for `.init' files.
+                       */
                        tmp_slist = (String_List *)
                                        checked_malloc(sizeof(String_List));
                        tmp_slist->next = NULL;
@@ -410,7 +415,6 @@
 static char *
 find_init_file(const char *basename)
 {
-       struct stat buf;
        char *filename;
        char *dirname;
        String_List *dir_ptr;
@@ -418,7 +422,7 @@
        int baselen;
        int len;
 
-       if (stat(basename, &buf) == 0) {
+       if (file_exists(basename)) {
                /* File is in current directory, so no search required */
                return NULL;
        }
@@ -437,7 +441,7 @@
                filename[dirlen] = '/';
                strcpy(filename + dirlen + 1, basename);
 
-               if (stat(filename, &buf) == 0)
+               if (file_exists(filename))
                        return filename;
 
                free(filename);
@@ -445,6 +449,19 @@
 
        /* Did not find file */
        return NULL;
+}
+
+       /*
+       ** Check whether a file exists.
+       ** At some point in the future it may be worth making this
+       ** implementation more portable.
+       */
+static bool
+file_exists(const char *filename)
+{
+       struct stat buf;
+
+       return (stat(filename, &buf) == 0);
 }
 
 /*--------------------------------------------------------------------------
-*/


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