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

Paul Bone paul at bone.id.au
Wed Jan 9 00:40:45 AEDT 2013
Previous message: [m-rev.] for post-commit review: add --enable-minimal-install
Next message: [m-rev.] For review: Pull request 4.
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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?

By review, I intend to verify that the changes fix the problem without
creating new problems.  Make any edits, such as to the git log messages, and
post the whole diff for the resulting changes for review by another Mercury
developer.

Thanks.

From 85cac39633b36d7274a1e33400b5e5d878c389ae Mon Sep 17 00:00:00 2001
From: Nikolay Orlyuk <virkony at gmail.com>
Date: Thu, 5 Jul 2012 23:01:16 +0300
Subject: [PATCH 1/4] Fix for bug 203 - DESTDIR brokes installation

See http://bugs.mercury.csse.unimelb.edu.au/view.php?id=203
Stripped DESTDIR when building boehm_gc/libatomic_ops
Added DESTDIR to default values in scripts/mmc
---
 boehm_gc/build_atomic_ops.sh        |    2 +-
 boehm_gc/build_atomic_ops.sh.cygwin |    2 +-
 scripts/mmc.in                      |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/boehm_gc/build_atomic_ops.sh b/boehm_gc/build_atomic_ops.sh
index faf19d0..638190b 100755
--- a/boehm_gc/build_atomic_ops.sh
+++ b/boehm_gc/build_atomic_ops.sh
@@ -3,4 +3,4 @@ P=`pwd`/libatomic_ops-install
 cd libatomic_ops
 # Mercury-specific: allow additional arguments.
 ./configure --prefix=$P "$@"
-$MAKE CC="$CC" install
+$MAKE CC="$CC" DESTDIR= install
diff --git a/boehm_gc/build_atomic_ops.sh.cygwin b/boehm_gc/build_atomic_ops.sh.cygwin
index 158a3f3..d9b963f 100755
--- a/boehm_gc/build_atomic_ops.sh.cygwin
+++ b/boehm_gc/build_atomic_ops.sh.cygwin
@@ -8,7 +8,7 @@ Q=`mktemp -d`
 ln -s "$P" $Q/dir
 cd $Q/dir/libatomic_ops
 ./configure --prefix=$Q/dir/libatomic_ops-install
-$MAKE CC="$CC" install
+$MAKE CC="$CC" DESTDIR= install
 cd /
 rm $Q/dir
 rmdir $Q
diff --git a/scripts/mmc.in b/scripts/mmc.in
index 510e018..d9d0e8b 100644
--- a/scripts/mmc.in
+++ b/scripts/mmc.in
@@ -14,8 +14,8 @@
 # MERCURY_COMPILER, MERCURY_C_COMPILER, MERCURY_DEFAULT_GRADE,
 # MERCURY_DEFAULT_OPT_LEVEL.
 
-MERCURY_COMPILER=${MERCURY_COMPILER-'@PREFIX@/bin/mercury_compile'}
-MERCURY_CONFIG_DIR=${MERCURY_CONFIG_DIR-${MERCURY_STDLIB_DIR-'@CONFIG_LIBDIR@'}}
+MERCURY_COMPILER=${MERCURY_COMPILER-${DESTDIR}'@PREFIX@/bin/mercury_compile'}
+MERCURY_CONFIG_DIR=${MERCURY_CONFIG_DIR-${DESTDIR}${MERCURY_STDLIB_DIR-'@CONFIG_LIBDIR@'}}
 export MERCURY_COMPILER MERCURY_CONFIG_DIR
 
 # Set the MACOSX_DEPLOYMENT_TARGET environment variable if needed.
-- 
1.7.10


From 6f135ed88734d5b6b090dcedcc4978cad9bfae99 Mon Sep 17 00:00:00 2001
From: Nikolay Orlyuk <virkony at gmail.com>
Date: Sat, 7 Jul 2012 20:53:25 +0300
Subject: [PATCH 2/4] Allow /usr/lib64 override (bug259)

Many linux distributives uses /usr/lib64 for amd64 libraries and
/usr/lib32 for x86. To support those this change allows to override
/usr/lib with standart libdir var in autoconf.

See http://bugs.mercury.csse.unimelb.edu.au/view.php?id=259
---
 scripts/Mmake.vars.in |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/scripts/Mmake.vars.in b/scripts/Mmake.vars.in
index f808d21..e5d782a 100644
--- a/scripts/Mmake.vars.in
+++ b/scripts/Mmake.vars.in
@@ -691,26 +691,30 @@ else
     DESTDIR_AND_SLASH=$(DESTDIR)/
 endif
 
-INSTALL_PREFIX		= $(DESTDIR_AND_SLASH)@prefix@
+prefix = @prefix@
+exec_prefix = @exec_prefix@
+libdir = @libdir@
+
+INSTALL_PREFIX		= $(DESTDIR_AND_SLASH)$(prefix)
 INSTALL_BINDIR		= $(INSTALL_PREFIX)/bin
-INSTALL_LIBDIR		= $(INSTALL_PREFIX)/lib/mercury
+INSTALL_LIBDIR		= $(DESTDIR_AND_SLASH)$(libdir)/mercury
 INSTALL_INFO_DIR	= $(INSTALL_PREFIX)/info
-INSTALL_DVI_DIR		= $(INSTALL_PREFIX)/lib/mercury/doc
-INSTALL_TEXT_DIR	= $(INSTALL_PREFIX)/lib/mercury/doc
-INSTALL_PS_DIR		= $(INSTALL_PREFIX)/lib/mercury/doc
-INSTALL_PDF_DIR		= $(INSTALL_PREFIX)/lib/mercury/doc
+INSTALL_DVI_DIR		= $(INSTALL_LIBDIR)/doc
+INSTALL_TEXT_DIR	= $(INSTALL_LIBDIR)/doc
+INSTALL_PS_DIR		= $(INSTALL_LIBDIR)/doc
+INSTALL_PDF_DIR		= $(INSTALL_LIBDIR)/doc
 INSTALL_MAN_DIR		= $(INSTALL_PREFIX)/man
-INSTALL_HTML_DIR	= $(INSTALL_PREFIX)/lib/mercury/html
-INSTALL_MDB_DOC_DIR	= $(INSTALL_PREFIX)/lib/mercury/mdb
-INSTALL_ELISP_DIR	= $(INSTALL_PREFIX)/lib/mercury/elisp
+INSTALL_HTML_DIR	= $(INSTALL_LIBDIR)/html
+INSTALL_MDB_DOC_DIR	= $(INSTALL_LIBDIR)/mdb
+INSTALL_ELISP_DIR	= $(INSTALL_LIBDIR)/elisp
 INSTALL_CGI_DIR		= $(DESTDIR_AND_SLASH)@CGIDIR@
 
-FINAL_INSTALL_PREFIX	= @prefix@
+FINAL_INSTALL_PREFIX	= $(prefix)
 FINAL_INSTALL_BINDIR 	= $(FINAL_INSTALL_PREFIX)/bin
-FINAL_INSTALL_LIBDIR	= $(FINAL_INSTALL_PREFIX)/lib/mercury
+FINAL_INSTALL_LIBDIR	= $(libdir)/mercury
 FINAL_INSTALL_INFO_DIR	= $(FINAL_INSTALL_PREFIX)/info
 FINAL_INSTALL_MAN_DIR	= $(FINAL_INSTALL_PREFIX)/man
-FINAL_INSTALL_ELISP_DIR	= $(FINAL_INSTALL_PREFIX)/lib/mercury/elisp
+FINAL_INSTALL_ELISP_DIR	= $(FINAL_INSTALL_LIBDIR)/elisp
 FINAL_INSTALL_CGI_DIR	= @CGIDIR@
 
 # You should not need to override anything below here
-- 
1.7.10


From c65df722c186a0c373af6ca4dddc9a714a21fdf1 Mon Sep 17 00:00:00 2001
From: Nikolay Orlyuk <virkony at gmail.com>
Date: Sat, 7 Jul 2012 23:36:07 +0300
Subject: [PATCH 3/4] Add man, info and lisp dirs configuring (bug259)

See http://bugs.mercury.csse.unimelb.edu.au/view.php?id=259
---
 configure.ac          |    3 +++
 scripts/Mmake.vars.in |   19 +++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3309b09..9469df6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1164,6 +1164,9 @@ AC_SUBST(LINK_OPT_SEP)
 AC_SUBST(FIX_PATH_FOR_CC)
 AC_SUBST(CYGPATH)
 
+lispdir='${datarootdir}/emacs/site-lisp'
+AC_SUBST(lispdir)
+
 #-----------------------------------------------------------------------------#
 # Check for `-lm': some systems, e.g. MacOS X (Darwin), don't have it.
 # The result of this check may be overridden below.
diff --git a/scripts/Mmake.vars.in b/scripts/Mmake.vars.in
index e5d782a..4d8224e 100644
--- a/scripts/Mmake.vars.in
+++ b/scripts/Mmake.vars.in
@@ -691,30 +691,37 @@ else
     DESTDIR_AND_SLASH=$(DESTDIR)/
 endif
 
+# autoconf provided vars
 prefix = @prefix@
 exec_prefix = @exec_prefix@
 libdir = @libdir@
+datarootdir = @datarootdir@
+mandir = @mandir@
+infodir = @infodir@
+lispdir = @lispdir@
+
+# actual vars used for installation
 
 INSTALL_PREFIX		= $(DESTDIR_AND_SLASH)$(prefix)
 INSTALL_BINDIR		= $(INSTALL_PREFIX)/bin
 INSTALL_LIBDIR		= $(DESTDIR_AND_SLASH)$(libdir)/mercury
-INSTALL_INFO_DIR	= $(INSTALL_PREFIX)/info
+INSTALL_INFO_DIR	= $(infodir)
 INSTALL_DVI_DIR		= $(INSTALL_LIBDIR)/doc
 INSTALL_TEXT_DIR	= $(INSTALL_LIBDIR)/doc
 INSTALL_PS_DIR		= $(INSTALL_LIBDIR)/doc
 INSTALL_PDF_DIR		= $(INSTALL_LIBDIR)/doc
-INSTALL_MAN_DIR		= $(INSTALL_PREFIX)/man
+INSTALL_MAN_DIR		= $(DESTDIR_AND_SLASH)$(mandir)
 INSTALL_HTML_DIR	= $(INSTALL_LIBDIR)/html
 INSTALL_MDB_DOC_DIR	= $(INSTALL_LIBDIR)/mdb
-INSTALL_ELISP_DIR	= $(INSTALL_LIBDIR)/elisp
+INSTALL_ELISP_DIR	= $(DESTDIR_AND_SLASH)$(lispdir)
 INSTALL_CGI_DIR		= $(DESTDIR_AND_SLASH)@CGIDIR@
 
 FINAL_INSTALL_PREFIX	= $(prefix)
 FINAL_INSTALL_BINDIR 	= $(FINAL_INSTALL_PREFIX)/bin
 FINAL_INSTALL_LIBDIR	= $(libdir)/mercury
-FINAL_INSTALL_INFO_DIR	= $(FINAL_INSTALL_PREFIX)/info
-FINAL_INSTALL_MAN_DIR	= $(FINAL_INSTALL_PREFIX)/man
-FINAL_INSTALL_ELISP_DIR	= $(FINAL_INSTALL_LIBDIR)/elisp
+FINAL_INSTALL_INFO_DIR	= $(infodir)
+FINAL_INSTALL_MAN_DIR	= $(mandir)
+FINAL_INSTALL_ELISP_DIR	= $(lispdir)
 FINAL_INSTALL_CGI_DIR	= @CGIDIR@
 
 # You should not need to override anything below here
-- 
1.7.10


From 02f884a6b55de4ce0cfc34bf363c3cd341733363 Mon Sep 17 00:00:00 2001
From: Nikolay Orlyuk <virkony at gmail.com>
Date: Sat, 5 Jan 2013 20:12:45 +0200
Subject: [PATCH 4/4] fix bug273 (race cond. in slice and deep_profile)

Removed mdbcomp modules copy-in. Instead just include mdbcomp in search
subdirs.

See http://bugs.mercury.csse.unimelb.edu.au/view.php?id=273
---
 deep_profiler/DEEP_FLAGS.in |    3 +++
 deep_profiler/Mmakefile     |   39 +++++---------------------------------
 slice/Mmakefile             |   44 +++++++------------------------------------
 slice/SLICE_FLAGS.in        |    3 +++
 4 files changed, 18 insertions(+), 71 deletions(-)

diff --git a/deep_profiler/DEEP_FLAGS.in b/deep_profiler/DEEP_FLAGS.in
index d73d03a..cda923a 100644
--- a/deep_profiler/DEEP_FLAGS.in
+++ b/deep_profiler/DEEP_FLAGS.in
@@ -6,6 +6,7 @@
 -I../library
 -I../browser
 -I../ssdb
+-I../mdbcomp
 --c-include-directory ../boehm_gc
 --c-include-directory ../boehm_gc/include
 --c-include-directory ../runtime
@@ -15,6 +16,8 @@
 --c-include-directory ../browser/Mercury/mihs
 --c-include-directory ../ssdb
 --c-include-directory ../ssdb/Mercury/mihs
+--c-include-directory ../mdbcomp
+--c-include-directory ../mdbcomp/Mercury/mihs
 --c-include-directory ../trace
 --csharp-flag -keyfile:../mercury.snk
 --no-java-classpath
diff --git a/deep_profiler/Mmakefile b/deep_profiler/Mmakefile
index c43fc09..2bdf85c 100644
--- a/deep_profiler/Mmakefile
+++ b/deep_profiler/Mmakefile
@@ -54,23 +54,7 @@ else
 	INSTALL=nothing
 endif
 
-VPATH = $(LIBRARY_DIR) $(SSDB_DIR)
-
-#-----------------------------------------------------------------------------#
-
-MDBCOMP_MODULES = \
-	feedback.automatic_parallelism.m \
-	feedback.m \
-	mdbcomp.m \
-	mdbcomp.goal_path.m \
-	prim_data.m \
-	program_representation.m \
-	rtti_access.m \
-	shared_utilities.m \
-	slice_and_dice.m \
-	trace_counts.m
-
-MDBCOMP_ORIG_MODULES = $(patsubst %,$(MDBCOMP_DIR)/%,$(MDBCOMP_MODULES))
+VPATH = $(LIBRARY_DIR) $(MDBCOMP_DIR) $(SSDB_DIR)
 
 #-----------------------------------------------------------------------------#
 
@@ -107,9 +91,9 @@ endif
 nothing:
 
 .PHONY: depend
-depend:	$(MDBCOMP_MODULES) $(DEPEND)
+depend:	$(DEPEND)
 
-$(DEPEND): DEEP_FLAGS $(MDBCOMP_MODULES) Mercury.modules
+$(DEPEND): DEEP_FLAGS Mercury.modules
 
 # This directory contains source files for which the module
 # name doesn't match the file name, so smart recompilation
@@ -119,19 +103,7 @@ Mercury.modules: DEEP_FLAGS
 	$(MC) $(ALL_GRADEFLAGS) $(ALL_MCFLAGS) -f *.m
 
 .PHONY: all
-all:	$(MDBCOMP_MODULES) $(ALL_DEEP_MODULES) $(TAGS_FILE_EXISTS)
-
-# 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)
-	- at chmod a+w $(MDBCOMP_MODULES) > /dev/null 2>&1; exit 0
-	cp $(MDBCOMP_ORIG_MODULES) .
-	@chmod a-w $(MDBCOMP_MODULES)
+all:	$(ALL_DEEP_MODULES) $(TAGS_FILE_EXISTS)
 
 #-----------------------------------------------------------------------------#
 
@@ -231,8 +203,7 @@ cs: $(mdprof_procrep.cs) $(cs_subdir)mdprof_procrep_init.c
 #-----------------------------------------------------------------------------#
 
 realclean_local:
-	rm -f .deep_tags tags DEEP_FLAGS DEEP_FLAGS.date \
-		$(MDBCOMP_MODULES) mdbcomp.*.err
+	rm -f .deep_tags tags DEEP_FLAGS DEEP_FLAGS.date
 
 #-----------------------------------------------------------------------------#
 
diff --git a/slice/Mmakefile b/slice/Mmakefile
index cdfca10..7ef40e4 100644
--- a/slice/Mmakefile
+++ b/slice/Mmakefile
@@ -42,23 +42,7 @@ INTS	= $(patsubst %,%.ints,$(MERCURY_MAIN_MODULES))
 INT3S	= $(patsubst %,%.int3s,$(MERCURY_MAIN_MODULES))
 CHECKS	= $(patsubst %,%.check,$(MERCURY_MAIN_MODULES))
 
-VPATH = $(LIBRARY_DIR) $(SSDB_DIR)
-
-#-----------------------------------------------------------------------------#
-
-MDBCOMP_MODULES = \
-	feedback.automatic_parallelism.m \
-	feedback.m \
-	mdbcomp.m \
-	mdbcomp.goal_path.m \
-	prim_data.m \
-	program_representation.m \
-	rtti_access.m \
-	shared_utilities.m \
-	slice_and_dice.m \
-	trace_counts.m
-
-MDBCOMP_ORIG_MODULES = $(patsubst %,$(MDBCOMP_DIR)/%,$(MDBCOMP_MODULES))
+VPATH = $(LIBRARY_DIR) $(MDBCOMP_DIR) $(SSDB_DIR)
 
 #-----------------------------------------------------------------------------#
 
@@ -82,26 +66,12 @@ endif
 #-----------------------------------------------------------------------------#
 
 .PHONY: depend
-depend:	$(MDBCOMP_MODULES) $(DEPENDS)
+depend:	$(DEPENDS)
 
-$(DEPENDS): SLICE_FLAGS $(MDBCOMP_MODULES)
+$(DEPENDS): SLICE_FLAGS
 
 .PHONY: all
-all:	$(MDBCOMP_MODULES) $(MERCURY_MAIN_MODULES) $(TAGS_FILE_EXISTS)
-
-#-----------------------------------------------------------------------------#
-#
-# 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)
-	- at chmod a+w $(MDBCOMP_MODULES) > /dev/null 2>&1; exit 0
-	cp $(MDBCOMP_ORIG_MODULES) .
-	@chmod a-w $(MDBCOMP_MODULES)
+all:	$(MERCURY_MAIN_MODULES) $(TAGS_FILE_EXISTS)
 
 #-----------------------------------------------------------------------------#
 
@@ -157,7 +127,8 @@ ints:	$(INTS)
 tags:	.slice_tags
 
 .slice_tags: $(MTAGS) $(mslice.ms) $(mdice.ms) $(mtc_union.ms) $(mcov.ms) \
-		$(mtc_diff.ms) $(wildcard $(LIBRARY_DIR)/*.m)
+		$(mtc_diff.ms) $(wildcard $(LIBRARY_DIR)/*.m) \
+		$(wildcard $(MDBCOMP_DIR)/*.m)
 	$(MTAGS) $(mslice.ms) $(mdice.ms) $(mtc_union.ms) $(mcov.ms) \
 		$(mtc_diff.ms) $(LIBRARY_DIR)/*.m
 	@touch .slice_tags
@@ -194,8 +165,7 @@ ils:	$(mslice.ils) $(mdice.ils) $(mtc_union.ils) $(mcov.ils) $(mtc_diff.ils)
 #-----------------------------------------------------------------------------#
 
 realclean_local:
-	rm -f tags SLICE_FLAGS SLICE_FLAGS.date \
-		$(MDBCOMP_MODULES) mdbcomp.*.err
+	rm -f tags SLICE_FLAGS SLICE_FLAGS.date
 
 #-----------------------------------------------------------------------------#
 
diff --git a/slice/SLICE_FLAGS.in b/slice/SLICE_FLAGS.in
index 2a4321c..78bde9e 100644
--- a/slice/SLICE_FLAGS.in
+++ b/slice/SLICE_FLAGS.in
@@ -6,6 +6,7 @@
 -I../library
 -I../browser
 -I../ssdb
+-I../mdbcomp
 --c-include-directory ../boehm_gc
 --c-include-directory ../boehm_gc/include
 --c-include-directory ../runtime
@@ -15,6 +16,8 @@
 --c-include-directory ../browser/Mercury/mihs
 --c-include-directory ../ssdb
 --c-include-directory ../ssdb/Mercury/mihs
+--c-include-directory ../mdbcomp
+--c-include-directory ../mdbcomp/Mercury/mihs
 --c-include-directory ../trace
 --csharp-flag -keyfile:../mercury.snk
 --no-java-classpath
-- 
1.7.10

-- 
Paul Bone
http://www.bone.id.au
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 835 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20130109/83c958fe/attachment.sig>
Previous message: [m-rev.] for post-commit review: add --enable-minimal-install
Next message: [m-rev.] For review: Pull request 4.
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
More information about the reviews mailing list