search

numenta / nupic.core-legacy

Implementation of core NuPIC algorithms in C++ (under construction)
http://numenta.org
GNU Affero General Public License v3.0
272 stars 278 forks source link

Missing dependency `zlib` or more in `external/common/include` #48

Closed utensil closed 10 years ago

utensil commented 10 years ago

UPDATE

The title has been changed to "Missing dependency zlib or more in external/common/include " to described the issue better, the original title was "Missing dependency checks in CMake such as zlib".

Original issue

While trying to give merged #47 a try in a koding.com VM(which is basically a Ubuntu variant) using gcc version 4.7.3 (Ubuntu/Linaro 4.7.3-1ubuntu1) and numenta/nupic.core's master@HEAD, I got the following compilation error:

utensilsong@vm-1:~/projects/nupic.core/build/scripts$ make -j3

...omitting some output...

[ 15%] Building CXX object CMakeFiles/nupic_core.dir/main/os/FStream.cpp.o
/home/utensilsong/projects/nupic.core/src/main/os/FStream.cpp:46:18: fatal error: zlib.h: No such file or directory
compilation terminated.
make[2]: *** [CMakeFiles/nupic_core.dir/main/os/FStream.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/nupic_core.dir/all] Error 2
make: *** [all] Error 2

It's trivial to fix it by sudo apt-get install zlibc zlib1g zlib1g-dev under Ubuntu, but this got me thinking, should CMake do some extra dependency checks? Don't know if there's any other dependencies, because I tried cleaning the VM environment and no other dependency problems surfaced.

cc @david-ragazzi @rhyolight

rhyolight commented 10 years ago

Good catch. @david-ragazzi what do you think?

utensil commented 10 years ago

I believe this can be done by CMake command find_package (ZLIB REQUIRED).

utensil commented 10 years ago

Oh no, it's not the reason I thought.

While trying to locate a place to put find_package (ZLIB REQUIRED), I noticed in https://github.com/numenta/nupic.core/blob/master/src/CMakeLists.txt line 457 , it reads:

set(LIB_STATIC_Z z)
add_library(${LIB_STATIC_Z} STATIC IMPORTED)
set_property(TARGET ${LIB_STATIC_Z} PROPERTY IMPORTED_LOCATION "${REPOSITORY_DIR}/external/${NTA_PLATFORM_OS}/lib/libz.${STATIC_LIB_EXTENSION}")

And in line 351, it reads:

#
# Include directories of headers
# Allows us to find includes for external libraries and enables
# #include <nta/common/...>
#
set(NTA_INCLUDEFLAGS "-I${REPOSITORY_DIR} -I${PROJECT_BUILD_RELEASE_DIR}/include -isystem${REPOSITORY_DIR}/external/common/include -isystem${REPOSITORY_DIR}/external/${NTA_PLATFORM_OS}/include")

I've checkd external, it seems a pre-built libz.a is included in external for all OSes, but no corresponding zlib.h at all. On a computer that this header does exist, it would compile, but might result in mismatched header and binary library.

david-ragazzi commented 10 years ago

I've checkd external, it seems a pre-built libz.a is included in external for all OSes, but no corresponding zlib.h at all. On a computer that this header does exist, it would compile, but might result in mismatched header and binary library.

@utensil The headers also are provided at external/common/include.

david-ragazzi commented 10 years ago

Good catch. @david-ragazzi what do you think?

Actually I think we should continue using the static libraries located at external folder. This avoid several issues of compatibility and yet help the user saving a time that would be spent downloading them (and compiling them, in case of Windows)...

utensil commented 10 years ago

@david-ragazzi commented 6 hours ago

The headers also are provided at external/common/include.

This comment leads me to find in https://github.com/numenta/nupic/tree/master/external/common/include , zlib.h and some other headers are present, but in https://github.com/numenta/nupic.core/tree/master/external/common/include , zlib is NOT present. This is the real casue of the build failure I'm experiencing.

utensil commented 10 years ago

There're also other headers missing:

I understand some of them should be removed to minimize the dependency, and was removed intentionally in #47 . This raised 2 questions:

I'll look into them.

utensil commented 10 years ago

It seems this should be fixed before merging numenta/nupic#751 .

breznak commented 10 years ago

On Fri, Apr 4, 2014 at 3:00 AM, utensil notifications@github.com wrote:

There're also other headers missing:

  • google
  • libpng12
  • jpeglib.h
  • png.h
  • pngconf.h
  • zconf.h

I understand some of them should be removed to minimize the dependency, and was removed intentionally. This raised 2 questions:

@utensil Also check if you don;t find some of them in external/{platform}include/

rhyolight commented 10 years ago

You guys are talking C-talk, which is still Greek to me, so let me know if you need help. :wink:

david-ragazzi commented 10 years ago 
There're also other headers missing:

    - google
    - libpng12
    - jpeglib.h
    - png.h
    - pngconf.h
    - zconf.h

I don't include these headers propositally, as nupic.core don't need these libraries.. I tested the remotion of each one to see which library is used or not by nupic.core..

utensil commented 10 years ago

@david-ragazzi Removing a header and see if the build fails is not enough. zlib header is available on most machines, so it will usually compile, but not always. That's why I'm getting this error.

I'll further verify other headers.

david-ragazzi commented 10 years ago

But I didn't remove zlib headers.. Furthermore, I removed the headers and also the static libraries for test them..

utensil commented 10 years ago

@david-ragazzi , you did remove zlib headers, as the diff shows:

utensilsong@vm-1:~/projects$ diff -r nupic/external/ david-ragazzi/nupic.core/external/
Only in nupic/external/common: bin
Only in nupic/external/common/include: google
Only in nupic/external/common/include: jpeglib.h
Only in nupic/external/common/include: libpng12
Only in nupic/external/common/include: pngconf.h
Only in nupic/external/common/include: png.h
Only in nupic/external/common/include: zconf.h
Only in nupic/external/common/include: zlib.h
Only in nupic/external/common: requirements.txt
Only in nupic/external/common: share
Only in nupic/external/darwin64: bin
Only in nupic/external/darwin64/lib: libjpeg.a
Only in nupic/external/licenses: LICENSE.libjpeg-6b.txt
Only in nupic/external/licenses: LICENSE.pcre-7.1.txt
Only in nupic/external/licenses: LICENSE.swig-1.3.31.txt
Only in nupic/external/licenses: LICENSE.tweepy-2.1.txt
Only in nupic/external/linux32: bin
Only in nupic/external/linux32/lib: python2.6
Only in nupic/external/linux32/lib: python2.7
Only in nupic/external/linux32arm: bin
Only in nupic/external/linux32arm/include: jpeglib.h
Only in nupic/external/linux32arm/include: libpng12
Only in nupic/external/linux32arm/include: pngconf.h
Only in nupic/external/linux32arm/include: png.h
Only in nupic/external/linux32arm/include: zconf.h
Only in nupic/external/linux32arm/include: zlib.h
Only in nupic/external/linux32arm/lib: libjpeg.a
Only in nupic/external/linux32arm/lib: python2.7
Only in nupic/external/linux32arm: share
Only in nupic/external/linux32armv7: bin
Only in nupic/external/linux32armv7/include: zconf.h
Only in nupic/external/linux32armv7/include: zlib.h
Only in nupic/external/linux64: bin
Only in nupic/external/linux64/lib: libjpeg.a
Only in david-ragazzi/nupic.core/external/: win32

Is it that you had kept it locally, but did not commit?

utensil commented 10 years ago

@david-ragazzi please check this, this issue remains valid.

utensil commented 10 years ago

I've reproduced this issue on travis, see https://travis-ci.org/utensil/nupic.core/jobs/24867408 :

By removing /usr/include/zlib.h and /home/travis/.nvm/v0.10.26/include/node/zlib.h, we have a failed build:

/home/travis/build/utensil/nupic.core/src/main/os/FStream.cpp:46:18: fatal error: zlib.h: No such file or directory
compilation terminated.
ghost commented 10 years ago

I had to delete zlib.h to get this to build. zlib 1.2.3 is from 2005, what linux distribution doesn't have zlib btw; how does adding an ancient header file solve that problem?

utensil commented 10 years ago

@olavks Can you describe your OS, compiler?

As for why providing the zlib, this issue is about being consistency with the current way of nupic.core handling external dependencies: to include in external directory and not to depend on it implicitly. While whether this way is the best is discuss-able, #97 is simply putting the files that were in nupic back, which means that probably this is a problem already existed in the old nupic code base.

utensil commented 10 years ago

@olavks I've opened an issue for you problem, let's discuss it further there.