[m-rev.] For review: Pull request 4.

Paul Bone paul at bone.id.au
Wed Jan 9 10:35:38 AEDT 2013


On Wed, Jan 09, 2013 at 10:11:09AM +1100, Julien Fischer wrote:
> On Wed, Jan 9, 2013 at 10:06 AM, Paul Bone <paul at bone.id.au> wrote:
> > On Wed, Jan 09, 2013 at 12:50:24AM +1100, Julien Fischer wrote:
> >> On Wed, Jan 9, 2013 at 12:40 AM, Paul Bone <paul at bone.id.au> wrote:
> >> >
> >> > Nikolay Orlyuk has submitted these four patches for review via github.
> >> >
> >> > The first three are concerned with installation, paths and the DESTDIR
> >> > variable.  The forth fixes an issue with parallel builds.
> >> >
> >> > I'm happy to review the first three, as I introduced the DESTDIR variable.
> >> > Can someone review the last one?
> >>
> >> See my comments in the bug database re: bug #273.  The reason for copying
> >> the files from the mdcomp directory rather than just linking them normally is
> >> because in debugging grades the versions in the slice and deep
> >> profiling directories
> >> need to be compiled differently.
> >>
> >> The problem here is caused by attempting to build the compiler from
> >> scratch by doing
> >>
> >>     $ make PARALLEL=-jN
> >>
> >> The parallel build should work without problems if you do
> >>
> >>    $ mmake depend
> >>    $ mmake -jN
> >>
> >> instead, since it is the depend target that responsible for copying the files
> >> from the mdbcomp directory.  (This is of course precisely what INSTALL{git,_CVS)
> >> says you should do**.)
> >
> > This is my usual practice, but possibly not always our users' current practice.
> 
> One of the reasons for that may be that it currently isn't what the final output
> of configure says.  (And that's usually what someone sees immediately before
> typing "make".)

Agreed.

> >> Invoking make instead of mmake should really be reserved for compiling
> >> the source
> >> distribution IMO.  The problem obviously won't exist for the source
> >> dist. since all the files have
> >> already been copied.
> >>
> >> In short, this change would break our ability to test some grades and
> >> can be worked around
> >> anyway.
> >>
> >> Cheers,
> >> Julien.
> >>
> >> ** configure currently recommends people do parallel builds using make
> >> PARALLEL=-jN.
> >> We should only print that out if there are pre-generated .c files
> >> present; otherwise we
> >> should recommend the use of mmake depend; mmake instead.
> >
> > I wonder if we can solve this with a PHONY target that is a dependency of
> > the 'all' target
> 
> The trick would be that you can't run the depend target if you are
> compiling from
> the source distribution since in that case you presumably don't have a Mercury
> compiler with which to build the dependencies.
> 

But a new target (that is a pre-requeiste for depend and all) doesn't need
to invoke the compiler.  It only needs to copy these files, or ensure that
they have been copied.


-- 
Paul Bone
http://www.bone.id.au



More information about the reviews mailing list