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.

Sign up for GitHub

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

bump cpp-httplib version to v0.7.6 latest #883

Merged
merged 5 commits into from
Sep 8, 2020
Merged

bump cpp-httplib version to v0.7.6 latest #883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0d90649
bump cpp-httplib version to v0.7.6 latest
breznak Sep 7, 2020
01a1c0f
Merge branch 'master_community' into bump_httplib
breznak Sep 8, 2020
7b557f7
cmake: apply SYSTEM includes where applicable
breznak Sep 8, 2020
8d3f815
httplib: special treatment to fix includes, use pragma ignored
breznak Sep 8, 2020
606cb75
httplib: set_timeout_sec() deprecated
breznak Sep 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ distribution packages as listed and rename them as indicated. Copy these to
| cereal.tar.gz | https://github.com/USCiLab/cereal/archive/v1.2.2.tar.gz |
| sqlite3.tar.gz | https://www.sqlite.org/2020/sqlite-autoconf-3320300.tar.gz |
| digestpp.zip | https://github.com/kerukuro/digestpp/archive/36fa6ca2b85808bd171b13b65a345130dbe1d774.zip |
| cpp-httplib.zip(*node4)| https://github.com/yhirose/cpp-httplib/archive/v0.5.5.zip |
| cpp-httplib.zip(*node4)| https://github.com/yhirose/cpp-httplib/archive/v0.7.6.zip |

* note1: Version 0.2.2 of libyaml is broken so use the master for the repository.
* note3: Boost is not required for any compiler that supports C++17 with `std::filesystem` (MSVC2017, gcc-8, clang-9).
Expand Down
50 changes: 25 additions & 25 deletions bindings/py/cpp_src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ target_link_libraries(${algorithms_shared_lib} PRIVATE
target_compile_options(${algorithms_shared_lib} PUBLIC ${INTERNAL_CXX_FLAGS})
target_compile_definitions(${algorithms_shared_lib} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${algorithms_shared_lib} PRIVATE
${PYTHON_INCLUDE_DIRS}
${pybind11_INCLUDE_DIRS}
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
${CORE_LIB_INCLUDES}
${PROJECT_SOURCE_DIR}
SYSTEM ${PYTHON_INCLUDE_DIRS}
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @dkeeney for the advice!,

We could catch all of them in one place by including SYSTEM at the beginning of the list at the point where the EXTERNAL_INCLUDES list is built which is line 93 of extern/bootstrap.cmake.

I tried this, by adding "SYSTEM" to the set(EXTERNAL_INCLUDES ...) but that didn't work as intended.

On line 450 and 464 of src/CMakeLists.txt, I did not specify SYSTEM for EXTERNAL_INCLUDES.

..so I fixed all ocurences of EXTERNAL_INCLUDES in target_include_dirs() with SYSTEM, that works.
But not for httplib, unfortunately. Is it included in the EXTERNAL_INCLUDES, or does it use a different means of including?

SYSTEM ${pybind11_INCLUDE_DIRS}
SYSTEM ${EXTERNAL_INCLUDES}
)


Expand All @@ -149,12 +149,12 @@ target_link_libraries(${sdr_shared_lib} PRIVATE
target_compile_options(${sdr_shared_lib} PUBLIC ${INTERNAL_CXX_FLAGS})
target_compile_definitions(${sdr_shared_lib} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${sdr_shared_lib} PRIVATE
${PYTHON_INCLUDE_DIRS}
${pybind11_INCLUDE_DIRS}
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
${PROJECT_SOURCE_DIR}
)
SYSTEM ${PYTHON_INCLUDE_DIRS}
SYSTEM ${pybind11_INCLUDE_DIRS}
SYSTEM ${EXTERNAL_INCLUDES}
)


set(encoders_shared_lib encoders)
Expand All @@ -166,12 +166,12 @@ target_link_libraries(${encoders_shared_lib} PRIVATE
target_compile_options(${encoders_shared_lib} PUBLIC ${INTERNAL_CXX_FLAGS})
target_compile_definitions(${encoders_shared_lib} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${encoders_shared_lib} PRIVATE
${PYTHON_INCLUDE_DIRS}
${pybind11_INCLUDE_DIRS}
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
${CORE_LIB_INCLUDES}
${PROJECT_SOURCE_DIR}
)
SYSTEM ${PYTHON_INCLUDE_DIRS}
SYSTEM ${pybind11_INCLUDE_DIRS}
SYSTEM ${EXTERNAL_INCLUDES}
)


set(engine_shared_lib engine_internal)
Expand All @@ -183,12 +183,12 @@ target_link_libraries(${engine_shared_lib} PRIVATE
target_compile_options(${engine_shared_lib} PUBLIC ${INTERNAL_CXX_FLAGS})
target_compile_definitions(${engine_shared_lib} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${engine_shared_lib} PRIVATE
${PYTHON_INCLUDE_DIRS}
${pybind11_INCLUDE_DIRS}
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
${PROJECT_SOURCE_DIR}
)
${CORE_LIB_INCLUDES}
${PROJECT_SOURCE_DIR}
SYSTEM ${PYTHON_INCLUDE_DIRS}
SYSTEM ${pybind11_INCLUDE_DIRS}
SYSTEM ${EXTERNAL_INCLUDES}
)


set(math_shared_lib math)
Expand All @@ -200,12 +200,12 @@ target_link_libraries(${math_shared_lib} PRIVATE
target_compile_options(${math_shared_lib} PUBLIC ${INTERNAL_CXX_FLAGS})
target_compile_definitions(${math_shared_lib} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${math_shared_lib} PRIVATE
${PYTHON_INCLUDE_DIRS}
${pybind11_INCLUDE_DIRS}
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
${PROJECT_SOURCE_DIR}
)
${CORE_LIB_INCLUDES}
${PROJECT_SOURCE_DIR}
SYSTEM ${PYTHON_INCLUDE_DIRS}
SYSTEM ${pybind11_INCLUDE_DIRS}
SYSTEM ${EXTERNAL_INCLUDES}
)

install(TARGETS
sdr
Expand Down
2 changes: 1 addition & 1 deletion external/cpp-httplib.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
if(EXISTS "${REPOSITORY_DIR}/build/ThirdParty/share/cpp-httplib.zip")
set(URL "${REPOSITORY_DIR}/build/ThirdParty/share/cpp-httplib.zip")
else()
set(URL https://github.com/yhirose/cpp-httplib/archive/v0.5.6.zip)
set(URL https://github.com/yhirose/cpp-httplib/archive/v0.7.6.zip)
endif()

message(STATUS "obtaining cpp-httplib")
Expand Down
14 changes: 7 additions & 7 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ target_compile_options( ${src_executable_hello} PUBLIC ${INTERNAL_CXX_FLAGS})
target_compile_definitions(${src_executable_hello} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${src_executable_hello} PRIVATE
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
SYSTEM ${EXTERNAL_INCLUDES}
)

###add_custom_target(benchmark_hello
Expand All @@ -373,7 +373,7 @@ else()
target_compile_definitions(${src_dyn_executable_hello} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${src_dyn_executable_hello} PRIVATE
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
SYSTEM ${EXTERNAL_INCLUDES}
)
endif()

Expand All @@ -392,7 +392,7 @@ target_compile_options( ${src_executable_napi_hello} PUBLIC ${INTERNAL_CXX_FLAGS
target_compile_definitions(${src_executable_napi_hello} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${src_executable_napi_hello} PRIVATE
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
SYSTEM ${EXTERNAL_INCLUDES}
)

#########################################################
Expand All @@ -410,7 +410,7 @@ target_compile_options( ${src_executable_napi_hello_database} PUBLIC ${INTERNAL_
target_compile_definitions(${src_executable_napi_hello_database} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${src_executable_napi_hello_database} PRIVATE
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
SYSTEM ${EXTERNAL_INCLUDES}
)

#########################################################
Expand All @@ -429,7 +429,7 @@ target_link_libraries(${src_executable_mnistsp}
target_compile_definitions(${src_executable_mnistsp} PRIVATE MNIST_DATA_LOCATION=${mnist_SOURCE_DIR})
target_include_directories(${src_executable_mnistsp} PRIVATE
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
SYSTEM ${EXTERNAL_INCLUDES}
)

###########################################################
Expand All @@ -447,7 +447,7 @@ target_link_libraries(${src_executable_mnistsp}
target_include_directories(${src_executable_rest_server} PRIVATE
${CORE_LIB_INCLUDES}
examples/rest
${EXTERNAL_INCLUDES}
SYSTEM ${EXTERNAL_INCLUDES}
)

set(src_executable_rest_client rest_client)
Expand All @@ -461,7 +461,7 @@ target_link_libraries(${src_executable_mnistsp}
target_compile_definitions(${src_executable_rest_client} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_include_directories(${src_executable_rest_client} PRIVATE
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES}
SYSTEM ${EXTERNAL_INCLUDES}
)

############ INSTALL ######################################
Expand Down
13 changes: 10 additions & 3 deletions src/examples/rest/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,17 @@
//


#include <httplib.h>
#include <iostream>
#include <cstring>

// save diagnostic state
#pragma GCC diagnostic push
// turn off the specific warning. Can also use "-Wall"
#pragma GCC diagnostic ignored "-Wsign-compare"
#include <httplib.h>
Copy link
Member Author

Choose a reason for hiding this comment

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

the SYSTEM include did not fix the warnings generated in httplib (I wonder why?), so I had to hack with #pragma here and in server_core.hpp.

Copy link

Choose a reason for hiding this comment

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

the SYSTEM include did not fix the warnings

I don't know of a way to get around the warnings other than with the #pragma.

// turn back on the compiler warnings
#pragma GCC diagnostic pop

//#define CA_CERT_FILE "./ca-bundle.crt"
#define DEFAULT_PORT 8050
#define DEFAULT_HOST "127.0.0.1"
Expand Down Expand Up @@ -78,7 +85,7 @@ int main(int argc, char **argv) {

VERBOSE << "Connecting to server: " + serverHost + " port: " << port << std::endl;
httplib::Client client(serverHost.c_str(), port);
client.set_timeout_sec(30); // The time it waits for a network connection.
//client.set_timeout_sec(30); // The time it waits for a network connection.
Copy link
Member Author

Choose a reason for hiding this comment

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

@Zbysekz did you find out if there's a replacement for the httplib.set_timeout_sec(), or if we just stop using that function?

Copy link

Choose a reason for hiding this comment

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

Asked question yhirose/cpp-httplib#642

Copy link

Choose a reason for hiding this comment

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

We have an answer: set_connection_timeout() is the substitute.
@breznak this PR is closed maybe starting another? I am not sure about consequences, or what timeout is by default

Copy link
Member Author

Choose a reason for hiding this comment

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

nice! sorry for merging early. Yes, we can start a new PR where you can revert commit 606cb75d00cb376 and then change it to the new signature.

I am not sure about consequences, or what timeout is by default

nor do i. our tests passed even without it. maybe in the server it makes sense, in the unit tests 30sec is imho too much.


// request "Hello World" to see if we are able to connect to the server.
VERBOSE << "GET /hi" << std::endl;
Expand Down Expand Up @@ -161,4 +168,4 @@ int main(int argc, char **argv) {
}
VERBOSE << "Anomaly Score: " << res->body << std::endl;
return 0;
}
}
15 changes: 12 additions & 3 deletions src/examples/rest/server_core.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,22 @@
#include <cctype>
#include <chrono>
#include <cstdio>
#include <htm/engine/Network.hpp>
#include <htm/engine/RESTapi.hpp>
#include <httplib.h>
#include <iomanip>
#include <sstream>
#include <string>

// save diagnostic state
#pragma GCC diagnostic push
// turn off the specific warning. Can also use "-Wall"
#pragma GCC diagnostic ignored "-Wsign-compare"
#include <httplib.h>
// turn back on the compiler warnings
#pragma GCC diagnostic pop

#include <htm/engine/Network.hpp>
#include <htm/engine/RESTapi.hpp>


#define SERVER_CERT_FILE "./cert.pem"
#define SERVER_PRIVATE_KEY_FILE "./key.pem"
#define DEFAULT_PORT 8050 // default port to listen on
Expand Down
4 changes: 2 additions & 2 deletions src/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,10 @@ target_link_libraries(${unit_tests_executable}
${INTERNAL_LINKER_FLAGS}
)
target_include_directories(${unit_tests_executable} PRIVATE
${gtest_INCLUDE_DIRS}
examples/rest
${CORE_LIB_INCLUDES}
${EXTERNAL_INCLUDES})
SYSTEM ${gtest_INCLUDE_DIRS}
SYSTEM ${EXTERNAL_INCLUDES})
target_compile_definitions(${unit_tests_executable} PRIVATE ${COMMON_COMPILER_DEFINITIONS})
target_compile_options(${unit_tests_executable} PUBLIC ${INTERNAL_CXX_FLAGS})
add_dependencies(${unit_tests_executable} ${core_library})
Expand Down
7 changes: 3 additions & 4 deletions src/test/unit/engine/RESTapiTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include <thread>
#include <chrono>

#include <httplib.h>
#include <examples/rest/server_core.hpp>

namespace testing {
Expand Down Expand Up @@ -120,7 +119,7 @@ class RESTapiTest : public ::testing::Test {
serverThreadObj = std::thread (&RESTapiTest::serverThread, this); // start REST server
std::this_thread::sleep_for(std::chrono::milliseconds(1)); // yield to give server time to start
client.reset(new httplib::Client(host, port));
client->set_timeout_sec(30);
//client->set_timeout_sec(30);
}

virtual void TearDown() {
Expand All @@ -145,7 +144,7 @@ class RESTapiTest : public ::testing::Test {
TEST_F(RESTapiTest, helloWorld) {
// Client thread.
Value vm;
client->set_timeout_sec(30);
//client->set_timeout_sec(30);

// request "Hello World" to see if we are able to connect to the server.
auto res = client->Get("/hi");
Expand All @@ -165,7 +164,7 @@ TEST_F(RESTapiTest, example) {
char message[1000];
Value vm;

client->set_timeout_sec(30);
//client->set_timeout_sec(30);

// Configure a NetworkAPI example
// See Network.configure() for syntax.
Expand Down