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 installer for libnnfdm library #59

Merged
merged 2 commits into from
Oct 10, 2022
Merged

Add installer for libnnfdm library #59

merged 2 commits into from
Oct 10, 2022

Conversation

mcfadden8
Copy link
Contributor

@mcfadden8 mcfadden8 commented Oct 5, 2022

Added cmake bits to allow for creation and installation of the following to a specified "install" directory:

install/include/nnfdm/client.h
install/lib/libnnfdm.a
install/share/cmake/nnfdm/nnfdm-targets.cmake
install/share/cmake/nnfdm/nnfdm-targets-release.cmake
install/share/cmake/nnfdm/nnfdm-config.cmake

install(TARGETS mylib
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
# Generate install of when running 'make install':
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is missing something.
Generate install of <what?> when running make install:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good question. I probably should have left this comment out. But the subsequent 'install' cmake directories will be used by cmake to generate the install make target so that running "make install" will yield the following files. The piece I wanted to capture was to depict where the installation would go and what the layout would be.

Perhaps I should just remove the entire comment since it may all be derived by the four install directives below?

Copy link
Contributor

@ajfloeder ajfloeder Oct 5, 2022

Choose a reason for hiding this comment

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

I'd prefer you leave the comment section in, maybe remove the word of?
Generate install when running 'make install':

#
# Compile with position independant code if we are set to run as a shared library
#
set_target_properties(nnfdm PROPERTIES POSITION_INDEPENDANT_CODE ${BUILD_SHARED_LIBS})
Copy link

Choose a reason for hiding this comment

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

should this be POSITION_INDEPENDENT_CODE ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! This was left in by mistake (by me). I'll take it it out.

Also, address comment feedback from PR review comments.

Signed-off-by: Marty McFadden <[email protected]>
@ajfloeder ajfloeder merged commit 83b4e8a into NearNodeFlash:master Oct 10, 2022
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.

5 participants