Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Build ngraph-tf not only on Centos #396

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Build ngraph-tf not only on Centos #396

wants to merge 8 commits into from

Conversation

samolisov
Copy link

When ngraph-tf is being built on a 64-bits OS another than Centos,
the directory with libraries ('build/artifacts/lib64') could not be
found since CMakeLists.txt and *.cmake files make a reference to 'lib64'
if and only if the host OS is Centos:

if(OS_VERSION STREQUAL ""centos"")

But if the host OS is not Centos (SUSE for example), the condition is
always false and the 'lib' directory instead of 'lib64' will be
looked for. To fix the situation the condition is changed to something
similar to:

if(EXISTS ${NGRAPH_INSTALL_DIR}/lib64)

In order to create the right one of the 'build/install/lib64' or
'build/install/lib' directories, the 'GNUInstallDirs' CMake module is
used on the Linux platform (the idea is stolen from NGraph). The
'NGTF_INSTALL_LIB_DIR' option is set to
'${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}':

set(NGTF_INSTALL_LIB_DIR ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR})

Signed-off-by: Pavel Samolysov [email protected]

When ngraph-tf is being built on a 64-bits OS another than Centos,
the directory with libraries ('build/artifacts/lib64') could not be
found since CMakeLists.txt and *.cmake files make a reference to 'lib64'
if and only if the host OS is Centos:

if(OS_VERSION STREQUAL "\"centos\"")

But if the host OS is not Centos (SUSE for example), the condition is
always false and the 'lib' directory instead of 'lib64' will be
looked for. To fix the situation the condition is changed to something
similar to:

if(EXISTS ${NGRAPH_INSTALL_DIR}/lib64)

In order to create the right one of the 'build/install/lib64' or
'build/install/lib' directories, the 'GNUInstallDirs' CMake module is
used on the Linux platform (the idea is stolen from NGraph). The
'NGTF_INSTALL_LIB_DIR' option is set to
'${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}':

set(NGTF_INSTALL_LIB_DIR ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR})

Signed-off-by: Pavel Samolysov <[email protected]>
Copy link
Contributor

@avijit-nervana avijit-nervana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What OSs you tested this (in addition to CentOS and Ubuntu)?

python/CreatePipWhl.cmake Outdated Show resolved Hide resolved
avijit-nervana and others added 5 commits January 18, 2019 22:05
Since some 64-bits OS (Ubuntu Linux, anything else?) use 'lib' instead
of 'lib64' to store libraries, the comment about 'lib64/lib' detection
in the 'CreatePipWhl.cmake' file has been changed.

Signed-off-by: Pavel Samolysov <[email protected]>
@samolisov
Copy link
Author

@avijit-nervana I've updates the comment. Could you please merge the PR if there is no concerns more? I'm working on OpenSUSE and making manual changing after each git pull makes me unhappy :-(

@samolisov
Copy link
Author

@avijit-nervana What can we do to merge the PR? Do I have to sign any contribution agreement or something like this?

@SleepProgger
Copy link

Same problem on Manjaro.
Creating a link from lib64 to lib works, but i would really appreciate not to have to do this.

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

Successfully merging this pull request may close these issues.

3 participants