-
Notifications
You must be signed in to change notification settings - Fork 130
Remove need for sudo #36
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
Conversation
Not using sudo is a good thing: it reduces boot time from 20-52s to 1-6s. See: https://docs.travis-ci.com/user/ci-environment/
This directory is write-accessible. /usr/local is not.
|
In order for this to work you probably need to supply the alternative protobuf install paths to the cmake call via -D, otherwise the protobuf will not be found. Probably also need to adjust the cache check, otherwise this will not build correctly on old caches. I'll try a commit that does this. |
| - mkdir -p build | ||
| - cd build | ||
| - cmake .. | ||
| - cmake -D CMAKE_PREFIX_PATH:PATH=${DEPS_DIR}/protobuf/install .. |
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.
Why didn't you write it like CMAKE_PREFIX_PATH=${DEPS_DIR}/protobuf/install:${PATH}? Does the current version do something different?
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.
The -D command-line flag vs. environment variable is mostly just our default practice to avoid reliance on environment variables which are passed through to other processes. The reason we don't append the PATH is that CMAKE_PREFIX_PATH is just a set of additional paths to look for packages, which defaults to empty. cmake will continue to use the PATH environment variable normally in any case.
PATH should not be overwritten!
Changed:
CMAKE_PREFIX_PATH:PATH=${DEPS_DIR}/protobuf/install ..
To:
CMAKE_PREFIX_PATH=${DEPS_DIR}/protobuf/install:${PATH} ..
fix build fail
|
Not sure why the build fails, probably the issue of path separators? Anyway including PATH on the CMAKE_PREFIX_PATH is not what we want, since CMAKE_PREFIX_PATH serves a different purpose, and PATH is used by cmake normally anyways, depending on system settings. |
ghost
left a comment
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.
Works fine!
Not using sudo is a good thing: it reduces boot time from 20-52s to 1-6s.
See: https://docs.travis-ci.com/user/ci-environment/