search

nerscadmin / IPM

Integrated Performance Monitoring for High Performance Computing
http://ipm-hpc.org
GNU Lesser General Public License v2.1
81 stars 35 forks source link

Re-enable out of source build #11

Closed pkestene closed 8 years ago

pkestene commented 8 years ago

This is a minor issue; I just made minimal changes to re-enable out-of-source build:

swfrench commented 8 years ago

Many thanks for contributing. It looks like this takes care of some of the remaining issues w.r.t. VPATH builds discussed in PR#7. I'll take closer look at this in the coming days and get back to you if I see any issues.

pkestene commented 8 years ago

I'm sorry, I must admit that I realized afterwards that my modifications are redundant with PR #7; though, the solution is slightly different regarding modifying make_wrappers script: as I'm not familliar with perl, I just used autoconf/automake to modify it at configuration time. You'll have to choose which solution is the more appropriate.

swfrench commented 8 years ago

Hi @pkestene - thanks again for the PR, and no worries about the collision with PR #7 (yours is slightly more complete w.r.t. updating the .am files tests/). Just a couple of comments:

  1. It looks like etc/Makefile.am might still have some trouble with out of source builds. If you change

    WRAPPER_HELPER =  ipm_key_*

    on line 6 to something like

    WRAPPER_HELPER =  $(top_srcdir)/etc/ipm_key_*

    this should work.

  2. It seems that the bin/make_wrappers.in script still writes generated header files to include/ in the top_srcdir. It might be cleaner to create a local include/ in the top_builddir and write them to there.

Item 2 is slightly more involved than 1. This will require changes to bin/make_wrappers.in (reverting some earlier changes, and potentially adding a check that include/ exists) and src/Makefile.am (updating the paths for GEN.*.h files to $(top_builddir)/include as well as adding -I$(top_builddir)/include to INCLUDES).

Let me know if you're interested in pursuing these. If not, I can take care of them.

pkestene commented 8 years ago

Hi Scott,

Other issues :

best

swfrench commented 8 years ago

Hi @pkestene -

  1. Good idea using sysconf_DATA. This will indeed allow install / uninstall to do right thing.
  2. Yes, simply putting all of the generated files in $(top_builddir)/src sounds like a fine solution.
  3. Thanks also for the fixes for CUDA support.

Let me test this out on a couple more platforms, and barring any unexpected issues I'll merge later today.

nerscadmin commented 8 years ago

Hi,

I'd like to second Scott's thanks. Much appreciated.

-David On Nov 2, 2015 8:47 AM, "Scott French" notifications@github.com wrote:

Hi @pkestene https://github.com/pkestene -

  1. Good idea using sysconf_DATA. This will indeed allow install / uninstall to do right thing.
  2. Yes, simply putting all of the generated files in $(top_builddir)/src sounds like a fine solution.
  3. Thanks also for the fixes for CUDA support.

Let me test this out on a couple more platforms, and barring any unexpected issues I'll merge later today.

— Reply to this email directly or view it on GitHub https://github.com/nerscadmin/IPM/pull/11#issuecomment-153079405.

swfrench commented 8 years ago

Hi @pkestene -

Thanks again for your contributions to IPM!

In regard to your question re: write_linkertable in bin/make_wrappers - it should be safe to leave this commented out as you have. Functionality that depends on linkwrap.txt has long been disabled (only vestigial references remain in the old test/Makefile_* to be removed shortly).

Cheers