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

Paul Bone paul at bone.id.au
Wed Jan 9 10:06:22 AEDT 2013


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.

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


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



More information about the reviews mailing list