add missing openssl system dep to folly #2067
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ahornby
force-pushed
the
pr2067
branch
2 times, most recently
from
7767035 to
db64174
Compare
This was referenced Sep 23, 2023
ahornby
force-pushed
the
pr2067
branch
2 times, most recently
from
5552ad2 to
c0387f1
Compare
Allow getdeps to free up some disk from the runner and intermediate build steps as looks like folly linux CI link step might be failing with out of disk space. Its an optional feature of getdeps with ia new --free-up-disk option to enable. When enabled it removes intermediate build state once deps have built, and also the massive unused android sdk from the github runners. Regenerated CI yaml with: ./build/fbcode_builder/getdeps.py generate-github-actions --src-dir=. --free-up-disk --os-type=linux --output-dir=.github/workflows folly
Summary:
macOS build was failing with:
* missing header includes, added them
macOS test run was failing with:
* libcpp exception text has changed from "with" to "due to" which was breaking ExceptionTest.cpp, update the expectation to cover both.
* FileUtilDetail.cpp was only enabling custom temp dir on linux, breaking FileUtilTest.cpp. Enable custom temp dir for non-windows (i.e. mac and other unix) to fix it.
Test Plan:
CI. Before, build error on vector
```
CMakeFiles/tdigest_benchmark.dir/folly/stats/test/TDigestBenchmark.cpp.o -MF CMakeFiles/tdigest_benchmark.dir/folly/stats/test/TDigestBenchmark.cpp.o.d -o CMakeFiles/tdigest_benchmark.dir/folly/stats/test/TDigestBenchmark.cpp.o -c /Users/runner/work/folly/folly/folly/stats/test/TDigestBenchmark.cpp
In file included from /Users/runner/work/folly/folly/folly/stats/test/TDigestBenchmark.cpp:17:
/Users/runner/work/folly/folly/folly/stats/DigestBuilder.h:59:25: error: implicit instantiation of undefined template 'std::vector<double>'
std::vector<double> buffer;
```
then build error on array
```
CMakeFiles/constexpr_math_test.dir/folly/test/ConstexprMathTest.cpp.o -c /Users/runner/work/folly/folly/folly/test/ConstexprMathTest.cpp
/Users/runner/work/folly/folly/folly/test/ConstexprMathTest.cpp:38:29: error: implicit instantiation of undefined template 'std::array<unsigned long, 7>'
std::array<res_t, size> res{};
```
then test failues
After, works
Found that ubuntu 20.04 and 22.04 LTS googletest was too old to build the folly tests, so updated the googletest manifest.
Exercise it by regenerating the github actions CI, which also speeds up the CI by not rebuilding boost each time.
Test plan:
build with system dependencies and without --no-tests
```
./build/fbcode_builder/getdeps.py --allow-system-packages install-system-deps --recursive folly
./build/fbcode_builder/getdeps.py --allow-system-packages build --src-dir=. folly
```
Before, error
```
FAILED: CMakeFiles/exception_wrapper_test.dir/folly/test/ExceptionWrapperTest.cpp.o
/usr/bin/c++ -DBOOST_ALL_NO_LIB -DFOLLY_XLOG_STRIP_PREFIXES=\"/home/alex/local/folly:/home/alex/local/tmp/ubuntu-20.04/fbcode_builder_getdeps-ZhomeZalexZlocalZfollyZbuildZfbcode_builder/build/folly\" -DGFLAGS_IS_A_DLL=0 -D_GNU_SOURCE -D_REENTRANT -I/home/alex/local/folly -I. -isystem /home/alex/local/tmp/ubuntu-20.04/fbcode_builder_getdeps-ZhomeZalexZlocalZfollyZbuildZfbcode_builder/installed/fmt-LkF3PGJrv9IqRFUd-zFENY3ah_TiYc7ZX3G020GUw-I/include -O2 -g -DNDEBUG -g -finput-charset=UTF-8 -fsigned-char -Wall -Wno-deprecated -Wno-deprecated-declarations -Wno-sign-compare -Wno-unused -Wuninitialized -Wunused-label -Wunused-result -Wshadow-compatible-local -Wno-noexcept-type -faligned-new -fopenmp -pthread -std=gnu++17 -MD -MT CMakeFiles/exception_wrapper_test.dir/folly/test/ExceptionWrapperTest.cpp.o -MF CMakeFiles/exception_wrapper_test.dir/folly/test/ExceptionWrapperTest.cpp.o.d -o CMakeFiles/exception_wrapper_test.dir/folly/test/ExceptionWrapperTest.cpp.o -c /home/alex/local/folly/folly/test/ExceptionWrapperTest.cpp
In file included from /usr/include/gtest/gtest.h:375,
from /usr/include/gmock/internal/gmock-internal-utils.h:47,
from /usr/include/gmock/gmock-actions.h:51,
from /usr/include/gmock/gmock.h:59,
from /home/alex/local/folly/folly/portability/GMock.h:32,
from /home/alex/local/folly/folly/test/ExceptionWrapperTest.cpp:23:
/home/alex/local/folly/folly/test/ExceptionWrapperTest.cpp: In member function ‘virtual void ExceptionWrapper_throw_test_Test::TestBody()’:
/home/alex/local/folly/folly/test/ExceptionWrapperTest.cpp:72:7: error: ‘ThrowsMessage’ was not declared in this scope
72 | ThrowsMessage<std::runtime_error>(StrEq("payload")));
```
After, tests build
Regenerated actions with:
```
./build/fbcode_builder/getdeps.py --allow-system-packages generate-github-actions --free-up-disk --src-dir=. --os-type=linux --output-dir=.github/workflows folly
```
folly is is missing a linux system dep for openssl. Add one so that install-system-deps knows to install openssl rpms/debs. openssl getdeps manifest already relies on system openssl on linux so its ok for folly to depend on it Test plan: run on a fresh ubuntu 20.04 toolbox which doesn't have openssl already: ``` ./build/fbcode_builder/getdeps.py --allow-system-packages install-system-deps --recursive folly ./build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests --src-dir=. folly ``` Before, error ``` CMake Error at /usr/share/cmake-3.16/Modules/FindPackageHandleStandardArgs.cmake:146 (message): Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the system variable OPENSSL_ROOT_DIR (missing: OPENSSL_CRYPTO_LIBRARY OPENSSL_INCLUDE_DIR) (Required is at least version "1.1.1") Call Stack (most recent call first): /usr/share/cmake-3.16/Modules/FindPackageHandleStandardArgs.cmake:393 (_FPHSA_FAILURE_MESSAGE) /usr/share/cmake-3.16/Modules/FindOpenSSL.cmake:447 (find_package_handle_standard_args) CMake/folly-deps.cmake:81 (find_package) CMakeLists.txt:144 (include) ``` After, works
|
lets assume people just install openssl |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add missing openssl system dep to folly
folly is is missing a linux system dep for openssl. Add one so that install-system-deps knows to install openssl rpms/debs.
openssl getdeps manifest already relies on system openssl on linux so its ok for folly to depend on it
Test plan:
run on a fresh ubuntu 20.04 toolbox which doesn't have openssl already:
Before, error
After, works
Stack created with Sapling. Best reviewed with ReviewStack.