-
Notifications
You must be signed in to change notification settings - Fork 200
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
Remove sudo in build #232
Remove sudo in build #232
Conversation
strong concept ACK. It looks like the commits are missing a DCO, and it does look like a large portion of the CI workflows need to be updated. |
This looks great, but I believe there might be a small issue to resolve: I get linker errors from the static
|
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.
needs rebase, and potentially a fix regarding another dependency (see comment above).
Just going to make a comment for future reference: this removes sudo in configure, our build script does not require sudo |
Not to pile on too much here but I'd suggest renaming |
merged via #249 |
Currently,
./scripts/configure.sh
requires root privileges to build OpenCBDC. These privileges are used for three purposes:a) installing build environment (e.g. clang, gmake on macOS, etc);
b) installing certain packaged dependencies (e.g. GoogleTest/
libgtest-dev
); andc) after fetching and compiling sources from the internet, installing certain libraries (e.g. LevelDB, NuRaft, Lua, etc) into OS default install path (usually,
/usr/lib
).This is suboptimal in multiple ways. First, invocation of sudo is spread all the way through the script, making this dependency harder to audit. Second, installing user-compiled dependencies in
/usr/lib
can interfere with OS package management (e.g., when user has already installed Lua) and makes it harder to develop two OpenCBDC instances side by side, if one of them, for example, wants to bump up its dependencies (e.g. have a never version of evmc).The two commits below try to amend the above as follows:
./scripts/install-build-tools.sh
. This script requiressudo
../scripts/configure.sh
to install dependencies in a separate, user-owned prefix directory (currently, this is calledprefix
and lives alongsidebuild
directory). This makes installation of these dependencies (c) not rely on root privileges and separates them between different instances of OpenCBDC (and system itself)../scripts/build.sh
andCMakeLists.txt
to include this prefix directory in dependency search.If this looks good, I can also produce appropriate changes to the documentation (README.md).