Fix: install MANA binaries and libraries correctly with make install
#188
Conversation
bae068a to
e3d761f
Compare
There was a problem hiding this comment.
@JainTwinkle: I have only partially reviewed this PR but we need some significant reversal especially around changes involving DESTDIR variable. This is a user-supplied variable for staged install and we shouldn't change it.
| @@ -46,7 +46,7 @@ default: display-build-env add-git-hooks mana_prereqs | |||
| all: default | |||
|
|
|||
| mana: dmtcp | |||
There was a problem hiding this comment.
I am not sure why we are doing a make install here. Shouldn't that go under the install target?
Also, should we just remove this target altogether.
There was a problem hiding this comment.
@karya0 , I'm not sure I understand your point here. We want to capture the default target for 'make'. The conventional name for that would be default and not install. Under that target, it's possible to call make install if desired. Something like that seems to be happening here.
| @@ -63,8 +63,9 @@ create-dirs: | |||
|
|
|||
| install: all create-dirs | |||
| cd dmtcp && make DESTDIR=$(DESTDIR) $(INSTALL_FLAGS) | |||
| cd mpi-proxy-split && make DESTDIR=$(DESTDIR) $(INSTALL_FLAGS) | |||
| cd manpages && make DESTDIR=$(DESTDIR) $(INSTALL_FLAGS) | |||
| cd mpi-proxy-split && make DESTDIR=$(prefix) $(INSTALL_FLAGS) | |||
There was a problem hiding this comment.
We shouldn't set DESTDIR to prefix. These are two separate variables and typically the final location is $(DESTDIR)$prefix
| @@ -45,6 +45,11 @@ endif | |||
| ifndef DMTCP_ROOT | |||
| DMTCP_ROOT=${MANA_ROOT}/dmtcp | |||
| endif | |||
|
|
|||
| ifeq ($(DESTDIR),) | |||
There was a problem hiding this comment.
Let's not touch DESTDIR at all -- this is something that's supplied as part of the make command itself.
There was a problem hiding this comment.
@JainTwinkle and @karya0 , As a general comment, do we still want the user to be able to build DMTCP inside the MANA distribution? Or do we want to hide the build of DMTCP behind abstractions so that the typical user will not be able to build DMTCP within MANA?
I worry that having one distribution for MANA only (can't easily biuld DMTCP) and one distribution for DMTCP could lead eventually to accidental divergence of the two codes. I have no problem if 'make' builds MANA by default. But then, should we also offer make dmtcp, which would build only the DMTCP binaries/libraries?
Or are we assuming that a build of MANA will always build DMTCP correctly for the general DMTCP case, and so there's no need to directly test DMTCP alone?
| @@ -46,7 +46,7 @@ default: display-build-env add-git-hooks mana_prereqs | |||
| all: default | |||
|
|
|||
| mana: dmtcp | |||
There was a problem hiding this comment.
@karya0 , I'm not sure I understand your point here. We want to capture the default target for 'make'. The conventional name for that would be default and not install. Under that target, it's possible to call make install if desired. Something like that seems to be happening here.
The current
make installdoes not install MANA executables and libraries to the installation directory (provided by--prefix).With this PR,
makeandmake -j installwork properly. Though it works. One doesn't have to callmake manato build MANA anymore (makeis enough).Catch: Sometimes,
make -j mana installthrow errors due to missing dependencies in theDMTCPsubmodule. For now, one should either usemake -j installor usemake -j mana && make -j installto install correctly.