Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add second stage build #56

Merged
merged 11 commits into from
Jul 24, 2016
Merged

Conversation

hjmjohnson
Copy link
Contributor

@uecker I finished adding the rest of the Makefile functionality to cmake this morning.

@mjacobs75 Mathews, These are the patches you need to be able to completely build bart with cmake.

Hans

Only have a dependance on the existence of a fortran
compiler if "LINALG_VENDOR MATCHES "LAPACKE"".  When
using LAPACKE, you need the fortran support libraries
like gfortran to be linked in.
The 2nd stage of the Makefile build was not implemented
in cmake.  When this shortcomming was identified, it was
better understood that each source file is compiled multiple
times with different flags for different purposes.

The rather verbose approach taken to preserve backwards
compatibility, and to avoid multiple conditional compilation
with different flags was to use the source files as templates
that are re-written to the build tree, and then compiled.

This approach facilitates debugging (the code compiled can
be viewed from the build tree, without identiftying the
result of the pre-processor.
The really cool pre-processing magic from misc/cppmap.h
is hard for most developers to descipher, and can be a source
of frustration for developers who wish to expand upon the
bart infrastructure, or contribute new features to bart.
The build_targets.mk files was created to provide a single point
of modification when new targets are added.  This will allow for
CMake builds to automatically identify when a new target is
added from the Makefile build system.
The mat2cfl is build if matlab is found, otherwise
if matlab is not found, or if USE_MATLAB is off, then it
is not built.
The ISMRMRD support is no functional.
Both cmake and Make now can build doc/commands.txt.
@uecker
Copy link
Member

uecker commented Jul 24, 2016

Thank you, Hans!

Is there a workaround for the LAPACKE issue, so that I can try this on my machine?

I will merge this now, but in the long run we should simplify this. It seems that some things which are simple to do with make and shell scripts are rather complicated with cmake.

  • For example reading the list of targets from a file seems very complicated. Would a different/simpler format for this file (the list of targets) help?
  • In bart.c, we could include a config.h with the list of main-functions defined as a macro.
    Would autogenerating the config.h be easier than editing bart.c during the build? (for our build system, config.h could just be empty)
  • I also wonder if we could simply omit some build options for cmake. For example, compiling commands into individual tools is something most people do not need. Just not supporting this option would remove some of the complexity.
  • Special stuff such as Mat2cfl might disappear anyway at some point.

Finally, would it be possible to add a section to .travis.yml where the cmake build is tested?

Martin

@uecker uecker merged commit 8f801bd into mrirecon:master Jul 24, 2016
@hjmjohnson
Copy link
Contributor Author

Martin,

The main reason for the very convoluted CMake is that I wanted to minimize the changes in the Makefile and code.

The cmake configuration could be made tremendously simpler if we did not need to maintain the build structure dictated by the current Makefile.

I would NOT use this cmake file as an indication the complexity of cmake.  It has many hacks that work arounds so that the Makefile system does not need to change.

I will work on the items in your list as time permits, but I am really busy right now making LAPACK updates and other projects. 

Can you please elaborate on the “LAPACKE issues” that you are having? 

I am just starting to use BART, so I did not know how people use it.  In some respects, I just made sure that CMake could do everything that the Makefile did without changing the current build process.

Hans

From: Martin Uecker notifications@github.com
Reply-To: mrirecon/bart reply@reply.github.com
Date: Sunday, July 24, 2016 at 3:24 PM
To: mrirecon/bart bart@noreply.github.com
Cc: Hans Johnson hans.j.johnson@gmail.com, Author author@noreply.github.com
Subject: Re: [mrirecon/bart] Add second stage build (#56)

Thank you, Hans!

Is there a workaround for the LAPACKE issue, so that I can try this on my machine?

I will merge this now, but in the long run we should simplify this. It seems that some things which are simple to do with make and shell scripts are rather complicated with cmake.
For example reading the list of targets from a file seems very complicated. Would a different/simpler format for this file (the list of targets) help?
In bart.c, we could include a config.h with the list of main-functions defined as a macro. Would autogenerating the config.h be easier than editing bart.c during the build? (for our build system, config.h could just be empty)
I also wonder if we could simply omit some build options for cmake. For example, compiling commands into individual tools is something most people do not need. Just not supporting this option would remove some of the complexity.
Special stuff such as Mat2cfl might disappear anyway at some point.
Finally, would it be possible to add a section to .travis.yml where the cmake build is tested?

Martin


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants