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

feat(nest): update nest extension installation dir #782

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sanjayankur31
Copy link
Contributor

The /nest bit seems to be included in NEST_LIBDIR now, so does not need to be explicitly added:

https://github.com/nest/nest-extension-module/blob/master/CMakeLists.txt#L178

I expect nest-config --libdir provides the complete path already.

The `/nest` bit seems to be included in `NEST_LIBDIR` now, so does not
need to be explicitly added:

https://github.com/nest/nest-extension-module/blob/master/CMakeLists.txt#L178

I expect `nest-config --libdir` provides the complete path already.
@coveralls
Copy link

Coverage Status

coverage: 71.994% (+0.009%) from 71.985% when pulling c6de29f on sanjayankur31:feat/nest-install-dir into 75cacd6 on NeuralEnsemble:master.

@apdavison
Copy link
Member

@sanjayankur31 for me nest-config --libdir gives just "lib". This is for installing into a Python venv (-DCMAKE_INSTALL_PREFIX=$VIRTUAL_ENV).

Do you see something different?

@sanjayankur31
Copy link
Contributor Author

I haven't tested just in a virtualenv. This is for the Fedora build where we explicitly set the CMAKE variables etc.. We do set -DCMAKE_INSTALL_LIBDIR:PATH=/usr/lib64. So, the shared objects end up into %{_prefix}/{lib,lib4} without a /nest suffix:

$ nest-config --prefix
/usr

$ nest-config --libdir
/usr/lib64

$ rpmls nest | grep lib
..
lrwxrwxrwx  /usr/lib64/libnest.so.3
-rwxr-xr-x  /usr/lib64/libnest.so.3.7
lrwxrwxrwx  /usr/lib64/libsli.so.3
-rwxr-xr-x  /usr/lib64/libsli.so.3.7
lrwxrwxrwx  /usr/lib64/libsli_readline.so.3
-rwxr-xr-x  /usr/lib64/libsli_readline.so.3.7

I do see that in the latest code for the nest-extension-module, they don't use the /nest/ suffix either:
https://github.com/nest/nest-extension-module/blob/7273612bef402838df69a13f46f9f9be09fd9ea9/CMakeLists.txt#L147

but maybe this patch isn't really required upstream (and we just carry it downstream so that pynn matches what we do with nest)?

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.

3 participants