-
Notifications
You must be signed in to change notification settings - Fork 103
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: Add installation targets for libbarretenberg, wasm & headers #185
Conversation
I found a couple more things that needed to change to install all headers got installed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Could you add something to the readme about how to install?
963e40e
to
f663ecd
Compare
f663ecd
to
80317d9
Compare
INTERFACE | ||
FILE_SET HEADERS | ||
FILES ${HEADER_FILES} | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only necessary here because common is not a barretenberg_module
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct!
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
FILE_SET HEADERS DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment explaining what this is doing (especially for EXPORT and each DESTINATION arg). I had to look it up and I'm sure others will too. Still not 100% sure I know what this command is doing after doing a bit of digging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do in a follow up PR.
…ztecProtocol/barretenberg#185) * feat: Add installation targets for libbarretenberg, wasm & headers * chore: Disable installation targets for gtest & benchmark * Collect tcc files for headers * Collect common headers for install target * chore(docs): Add installation section to README.
…ztecProtocol/barretenberg#185) * feat: Add installation targets for libbarretenberg, wasm & headers * chore: Disable installation targets for gtest & benchmark * Collect tcc files for headers * Collect common headers for install target * chore(docs): Add installation section to README.
Description
Closes #176
This adds install targets for
libbarretenberg.a
,barretenberg.wasm
and all the headers. It uses an "interface library" to collect headers as the subdirectories call thebarretenberg_module
function. I couldn't come up with a better way to collect the headers for installation because they exist in the subdirectories (instead of something like aninclude/
directory where the public headers exist, such as https://github.com/google/benchmark/tree/main/include/benchmark), which means that we promote all header files instead of just public ones.We should open a follow up issue to differentiate public from private headers.
With installation targets, we can now do
cmake --install build --prefix "/usr/local"
and it'll install like a normal C++ library.I also added the
INSTALL_BARRETENBERG
option in emulation of googletest and benchmark because usingFetchContent_MakeAvailable
will add them as install targets. You can see how theBENCHMARK_ENABLE_INSTALL
andINSTALL_GTEST
are turned off now too.This is the minimal setup we need to have installation targets, but many more improvements can be made so projects can consume this via other CMake builds, pkg-config, etc. These improvements are currently unnecessary for Noir to use barretenberg, but I can create follow up issues.
Checklist:
/markdown/specs
have been updated.@brief
describing the intended functionality.