From e8e89dfd37d84b7758a449b5e7dc780484747523 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 17 Nov 2023 14:19:50 -0500 Subject: [PATCH 1/2] Remove deprecated kj::mvCapture calls to avoid warnings There's no downside to this change since c++17 is already required. Fixes #87 --- include/mp/proxy-io.h | 8 ++++---- include/mp/proxy-types.h | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/mp/proxy-io.h b/include/mp/proxy-io.h index 1f3b4ccb..b0deeb4b 100644 --- a/include/mp/proxy-io.h +++ b/include/mp/proxy-io.h @@ -312,7 +312,7 @@ class Connection // to the EventLoop TaskSet to avoid "Promise callback destroyed itself" // error in cases where f deletes this Connection object. m_on_disconnect.add(m_network.onDisconnect().then( - kj::mvCapture(f, [this](F&& f) { m_loop.m_task_set->add(kj::evalLater(kj::mv(f))); }))); + [this, f = std::move(f)]() mutable { m_loop.m_task_set->add(kj::evalLater(kj::mv(f))); })); } EventLoop& m_loop; @@ -547,11 +547,11 @@ template void _Listen(EventLoop& loop, kj::Own&& listener, InitImpl& init) { auto* ptr = listener.get(); - loop.m_task_set->add(ptr->accept().then(kj::mvCapture(kj::mv(listener), - [&loop, &init](kj::Own&& listener, kj::Own&& stream) { + loop.m_task_set->add(ptr->accept().then( + [&loop, &init, listener = kj::mv(listener)](kj::Own&& stream) mutable { _Serve(loop, kj::mv(stream), init); _Listen(loop, kj::mv(listener), init); - }))); + })); } //! Given stream file descriptor and an init object, handle requests on the diff --git a/include/mp/proxy-types.h b/include/mp/proxy-types.h index f41ae58e..38170d8c 100644 --- a/include/mp/proxy-types.h +++ b/include/mp/proxy-types.h @@ -102,7 +102,7 @@ void CustomBuildField(TypeList<>, template kj::Promise JoinPromises(kj::Promise&& prom1, kj::Promise&& prom2) { - return prom1.then(kj::mvCapture(prom2, [](kj::Promise prom2) { return prom2; })); + return prom1.then([prom2 = kj::mv(prom2)]() mutable { return kj::mv(prom2); }); } //! PassField override for mp.Context arguments. Return asynchronously and call @@ -118,10 +118,10 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn& auto future = kj::newPromiseAndFulfiller(); auto& server = server_context.proxy_server; int req = server_context.req; - auto invoke = MakeAsyncCallable(kj::mvCapture(future.fulfiller, - kj::mvCapture(server_context.call_context, - [&server, req, fn, args...](typename ServerContext::CallContext call_context, - kj::Own> fulfiller) { + auto invoke = MakeAsyncCallable( + [&server, req, fn, args..., + fulfiller = kj::mv(future.fulfiller), + call_context = kj::mv(server_context.call_context)]() mutable { const auto& params = call_context.getParams(); Context::Reader context_arg = Accessor::get(params); ServerContext server_context{server, call_context, req}; @@ -155,7 +155,7 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn& fulfiller_dispose->reject(kj::mv(*exception)); }); } - }))); + }); auto thread_client = context_arg.getThread(); return JoinPromises(server.m_context.connection->m_threads.getLocalServer(thread_client) From 962e68135642c4108a17aff422460047a33e9d4f Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 17 Nov 2023 14:22:24 -0500 Subject: [PATCH 2/2] mpgen: Avoid deprecated SchemaParser::parseDiskFile call This avoids deprecation warnings, and is potentially a little safer since it sandboxes the parsing and makes it an error to import paths outside the source directory. This change does have some downsides, though. It complicates include logic, and also drops compatibility with capnproto versions before 0.7.0 that don't have the new SchemaParser::parseFromDirectory method which was added in https://github.com/capnproto/capnproto/commit/c1fe2b0339a1499c354eb85a2a78c63b2ce449f3 It also adds a hard dependency on the kj/filesystem.h library which was previously optional. Fixes #39 --- cmake/capnp_compat.cmake | 16 ++++++---------- src/mp/gen.cpp | 38 +++++++++++++++++++++++++++----------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/cmake/capnp_compat.cmake b/cmake/capnp_compat.cmake index 588a1ec2..50d9cec2 100644 --- a/cmake/capnp_compat.cmake +++ b/cmake/capnp_compat.cmake @@ -2,11 +2,8 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -# CMake target definitions for backwards compatibility with Ubuntu bionic -# capnproto 0.6.1 package (https://packages.ubuntu.com/bionic/libcapnp-dev) - -include(CheckIncludeFileCXX) -include(CMakePushCheckState) +# Define capnp_PREFIX if not defined to avoid issue on macos +# https://github.com/chaincodelabs/libmultiprocess/issues/26 if (NOT DEFINED capnp_PREFIX AND DEFINED CAPNP_INCLUDE_DIRS) get_filename_component(capnp_PREFIX "${CAPNP_INCLUDE_DIRS}" DIRECTORY) @@ -16,6 +13,10 @@ if (NOT DEFINED CAPNPC_OUTPUT_DIR) set(CAPNPC_OUTPUT_DIR "${CMAKE_CURRENT_BINARY_DIR}") endif() +# CMake target definitions for backwards compatibility with Ubuntu bionic +# capnproto 0.6.1 package (https://packages.ubuntu.com/bionic/libcapnp-dev) +# https://github.com/chaincodelabs/libmultiprocess/issues/27 + if (NOT DEFINED CAPNP_LIB_CAPNPC AND DEFINED CAPNP_LIB_CAPNP-RPC) string(REPLACE "-rpc" "c" CAPNP_LIB_CAPNPC "${CAPNP_LIB_CAPNP-RPC}") endif() @@ -53,8 +54,3 @@ if (NOT TARGET CapnProto::kj-async AND DEFINED CAPNP_LIB_KJ-ASYNC) add_library(CapnProto::kj-async SHARED IMPORTED) set_target_properties(CapnProto::kj-async PROPERTIES IMPORTED_LOCATION "${CAPNP_LIB_KJ-ASYNC}") endif() - -cmake_push_check_state() -set(CMAKE_REQUIRED_INCLUDES ${CMAKE_REQUIRED_INCLUDES} ${CAPNP_INCLUDE_DIRS}) -check_include_file_cxx("kj/filesystem.h" HAVE_KJ_FILESYSTEM) -cmake_pop_check_state() diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp index a027ab33..22212629 100644 --- a/src/mp/gen.cpp +++ b/src/mp/gen.cpp @@ -126,7 +126,9 @@ bool BoxedType(const ::capnp::Type& type) void Generate(kj::StringPtr src_prefix, kj::StringPtr include_prefix, kj::StringPtr src_file, - kj::ArrayPtr import_paths) + const std::vector& import_paths, + const kj::ReadableDirectory& src_dir, + const std::vector>& import_dirs) { std::string output_path; if (src_prefix == ".") { @@ -176,7 +178,11 @@ void Generate(kj::StringPtr src_prefix, } capnp::SchemaParser parser; - auto file_schema = parser.parseDiskFile(src_file, src_file, import_paths); + auto directory_pointers = kj::heapArray(import_dirs.size()); + for (size_t i = 0; i < import_dirs.size(); ++i) { + directory_pointers[i] = import_dirs[i].get(); + } + auto file_schema = parser.parseFromDirectory(src_dir, kj::Path::parse(output_path), directory_pointers); std::ofstream cpp_server(output_path + ".proxy-server.c++"); cpp_server << "// Generated by " PROXY_BIN " from " << src_file << "\n\n"; @@ -611,20 +617,30 @@ int main(int argc, char** argv) exit(1); } std::vector import_paths; -#ifdef HAVE_KJ_FILESYSTEM + std::vector> import_dirs; auto fs = kj::newDiskFilesystem(); auto cwd = fs->getCurrentPath(); -#endif + kj::Own src_dir; + KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(argv[1]))) { + src_dir = kj::mv(*dir); + } else { + throw std::runtime_error(std::string("Failed to open src_prefix prefix directory: ") + argv[1]); + } for (size_t i = 4; i < argc; ++i) { - import_paths.emplace_back(argv[i]); + KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(argv[i]))) { + import_paths.emplace_back(argv[i]); + import_dirs.emplace_back(kj::mv(*dir)); + } else { + throw std::runtime_error(std::string("Failed to open import directory: ") + argv[i]); + } } for (const char* path : {CMAKE_INSTALL_PREFIX "/include", capnp_PREFIX "/include"}) { -#ifdef HAVE_KJ_FILESYSTEM - KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(path))) { import_paths.emplace_back(path); } -#else - import_paths.emplace_back(path); -#endif + KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(path))) { + import_paths.emplace_back(path); + import_dirs.emplace_back(kj::mv(*dir)); + } + // No exception thrown if _PREFIX directories do not exist } - Generate(argv[1], argv[2], argv[3], {import_paths.data(), import_paths.size()}); + Generate(argv[1], argv[2], argv[3], import_paths, *src_dir, import_dirs); return 0; }