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

Implementing cmake build #657

Merged
merged 56 commits into from
Feb 21, 2014
Merged

Implementing cmake build #657

merged 56 commits into from
Feb 21, 2014

Conversation

rhyolight
Copy link
Member

Replacement for PR #645.
Fixes: #647.
Fixes: #661.

@rhyolight rhyolight mentioned this pull request Feb 18, 2014
@rhyolight
Copy link
Member Author

Discussion continues from #645, see that PR for previous conversations.

@oxtopus
Copy link
Contributor

oxtopus commented Feb 19, 2014

Is it possible to encapsulate the git submodule commands into the cmake configuration? I just tried this in a stock VM and it wasn't obvious that I needed to run git submodule init and git submodule update before cmake would run.

@rhyolight
Copy link
Member Author

@oxtopus Fine by me, but if we do that, we need to add back

git:
 -  submodules: false

into the .travis.yml. Also need to take out the instructions to manually git submodule update from the README.

@david-ragazzi
Copy link
Contributor

SOLVED!!!! Now Travis is building fine with GCC!!!

This helped me a lot:
https://groups.google.com/forum/#!msg/trema-dev/8DGcZ-om6Bk/7Vm-OXYsxroJ

The only explanation that I could find think of was that the linker used by Travis CI uses the --as-needed flag. This has a side-effect [1]. If all the libraries requiring libpthread are placed after it in the linker flags, libpthread will be discarded despite being needed.

By placing "-lpthread" in extconf.rb, it ended up near the beginning of the final LDFLAGS and my guess is that it was subsequently ignored by the linker for the aforementioned reason.

I included "-Wl,--no-as-needed " at begining of NTA_PLATFORM_LINKFLAGS variable for Linux platforms. Travis simply was ignoring the libraries which HtmTest and TestEveything would be linked against (-ldl -lpthread, etc ).

@subutai
Copy link
Member

subutai commented Feb 19, 2014

@david-ragazzi Nice work! That's really encouraging.

@david-ragazzi
Copy link
Contributor

@subutai

I include these lines:

if("$ENV{NTAX_DEVELOPER_BUILD}" STREQUAL "")
  set_environment_variable(NTAX_DEVELOPER_BUILD "1" OFF)
else()
  show_environment_variable(NTAX_DEVELOPER_BUILD "$ENV{NTAX_DEVELOPER_BUILD}")
endif()

The code above set NTAX_DEVELOPER_BUILD to 1 for first time, and once set, it only shows its value.

Feel free to change its initial value..

@rhyolight
Copy link
Member Author

I looked through it too. Only used for optimization in a handful of places.


Matt Taylor
OS Community Flag-Bearer
Numenta

On Thu, Feb 20, 2014 at 9:59 AM, Subutai Ahmad notifications@github.comwrote:

@rhyolight https://github.com/rhyolight Just looking into veclib issue.
I'm going to probably propose we get rid of that include. That is an
optimization that is only used in a couple of functions that I don't think
are really being used anyway. I will verify first though.

Reply to this email directly or view it on GitHubhttps://github.com//pull/657#issuecomment-35650024
.

@subutai
Copy link
Member

subutai commented Feb 20, 2014

@david-ragazzi I think it can just show it. It shouldn't set it - that is up to the developer. We also need to add the code to do the actual symlink. In the past this was handled by the script in build_system/pybuild/install.py

@subutai
Copy link
Member

subutai commented Feb 20, 2014

@david-ragazzi The following lines (179-182) are causing problems on some Mac's:

# Check if default C/C++ compiler is GNU or CLANG, otherwise exit with error.  
if(NOT(CMAKE_COMPILER_IS_GNUCXX OR ${CMAKE_CXX_COMPILER} MATCHES "clang"))
  message(FATAL_ERROR "${CMAKE_CXX_COMPILER} compiler not supported yet. If you have GNU or CLANG C++ compiler installed on your machine change $CXX environment variable or pass its location through USER_CXX_COMPILER option.")
endif()

The check fails even if CXX=clang++ and CC=clang. By taking it out the compilation works fine. On my Mac it works fine either way. @rhyolight suggests we take out the check and just put it in the documentation.

What do you think?

Good news is that without this check and after fixing #663, it works on Matt's and Austin's Mac's as well, so three total.

@david-ragazzi
Copy link
Contributor

@subutai I think is fine. Although a Borland or VC++ will cause trouble if user don't read doc ....

I'm checking symlinks issue.. NTAX_DELELOPER_BUILD is a "boolean" value, right? If true, CMake should create symlinks, if false copy the files..

@david-ragazzi
Copy link
Contributor

@subutai I wrote the code for symlinks. Please look for NTAX_DEVELOPER_BUILD word in CMake file.

I also noted that it doesn't replace a file to link and vice-versa after first configuration. Could CMake remove the entire $NTA folder before CMake starts the copy or linking?

@subutai
Copy link
Member

subutai commented Feb 21, 2014

@david-ragazzi Awesome! I just tried it out and it works great.

I also noted that it doesn't replace a file to link and vice-versa after first configuration. Could CMake remove the entire $NTA folder before CMake starts the copy or linking?

Hmm, that's a good question. If you remove $NTA every time you run CMake, is there a problem with that?

@rhyolight
Copy link
Member Author

I usually remove and recreate the $NTA directory when I do a clean build. I think it should be okay.

rhyolight and others added 3 commits February 20, 2014 22:11
Reverts commit 5bd2dfa. We changed the
process so that Travis will not automatically update submodules, instead
CMake does it. So the build script needs to do the same again, too.
For some reason it is not reliable on Mac's. Will address in documentation for now.
@subutai
Copy link
Member

subutai commented Feb 21, 2014

@david-ragazzi We are ok to merge this once build passes. After that I will continue some more Grok tests, and we can start on the next phase!

@subutai
Copy link
Member

subutai commented Feb 21, 2014

I usually remove and recreate the $NTA directory when I do a clean build. I think it should be okay.

OK, so running CMake is the equivalent of doing a clean build? I was wondering about that. If so, it should also clean out the build directory. For incremental builds the user can just run make right?

@david-ragazzi
Copy link
Contributor

OK, so running CMake is the equivalent of doing a clean build? I was wondering about that. If so, it should also clean out the build directory. For incremental builds the user can just run make right?

@subutai Yes. At CMake file we could simply put a command to delete $NTA (future build/release) directory to accomodate new changes in a clean folder. This would save a lot of code, mainly in symlinks issues!

@subutai
Copy link
Member

subutai commented Feb 21, 2014

@david-ragazzi Great, thanks. That is all very clean and simple.

subutai added a commit that referenced this pull request Feb 21, 2014
@subutai subutai merged commit 38a31ea into numenta:master Feb 21, 2014
@subutai
Copy link
Member

subutai commented Feb 21, 2014

@david-ragazzi Really nice job on this PR! I have a question. It looks like all the CPP/HPP files are explicitly listed in the CMakeLists.txt file. Is that necessary or can we eventually just list directories and it can automatically pick up the files? Currently each directory has a unittests sub-directory, so perhaps we have to wait until we remove those?

@scottpurdy
Copy link
Contributor

I just cleaned everything out and followed the README instructions and it worked well. Great work!

@david-ragazzi
Copy link
Contributor

It looks like all the CPP/HPP files are explicitly listed in the CMakeLists.txt file. Is that necessary or can we eventually just list directories and it can automatically pick up the files? Currently each directory has a unittests sub-directory, so perhaps we have to wait until we remove those?

@subutai In future, we could automate the extraction of these files names. There is a macro in CMake file that transverse a folder and get list of all files that fit a given criteria. I created this macro to get .cpp and .hpp files used by TestEverything project.

@rhyolight
Copy link
Member Author

@david-ragazzi said:

@subutai In future, we could automate the extraction of these files names. There is a macro in CMake file that transverse a folder and get list of all files that fit a given criteria.

Let's make this part of the discussion for numenta/nupic.core-legacy#4.

@subutai
Copy link
Member

subutai commented Feb 24, 2014

@david-ragazzi Thanks for the explanation.

@rhyolight rhyolight deleted the david-patch-2 branch August 19, 2016 13:11
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.

Update CMake to update submodule Update nupic build for CMake
6 participants