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

[Paul Bone]

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

It turns out there's a really simple fix for the race condition, and it
keeps the structure of the makefiles the same.  I will prepare a full patch for
this.

diff --git a/deep_profiler/Mmakefile b/deep_profiler/Mmakefile
index c43fc09..703c4f9 100644
--- a/deep_profiler/Mmakefile
+++ b/deep_profiler/Mmakefile
@@ -121,17 +121,25 @@ Mercury.modules: DEEP_FLAGS
 .PHONY: all
 all:   $(MDBCOMP_MODULES) $(ALL_DEEP_MODULES) $(TAGS_FILE_EXISTS)

+# We use the sentinel file .mdbcomp_modules to avoid the race condition that
+# exists because the rule would normally be invoked for each of the mdbcomp
+# modules.  However, we also need to use the no-op action '@' to allow make
+# to see that the timestamps on the $(MDBCOMP_MODULES) have changed and that
+# other things may need rebuilding.
+$(MDBCOMP_MODULES): .mdbcomp_modules
+       @
+
 # We need to start by turning write permission on for each copied file
 # in case some exist, but we need to ignore errors in case some don't exist.
 # The exit 0 is to prevent make itself from printing a message about the
 # (ignored) failure of an action.
 #
 # We could modify the action here to copy only the changed files.
-
-$(MDBCOMP_MODULES): $(MDBCOMP_ORIG_MODULES)
+.mdbcomp_modules : $(MDBCOMP_ORIG_MODULES)
        - at chmod a+w $(MDBCOMP_MODULES) > /dev/null 2>&1; exit 0
        cp $(MDBCOMP_ORIG_MODULES) .
        @chmod a-w $(MDBCOMP_MODULES)
+       touch $@

 #-----------------------------------------------------------------------------#


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