Skip to content

Commit

Permalink
Merged PR 317123: Completely remove boost.
Browse files Browse the repository at this point in the history
1) Add napa::filesystem.
2) Add UT for napa::utils::string and napa::filesystem
3) Remove boost from code and build files.
  • Loading branch information
daiyip authored and Daiyi Peng committed Jul 7, 2017
1 parent 8fe1039 commit 08767c1
Show file tree
Hide file tree
Showing 30 changed files with 1,321 additions and 302 deletions.
34 changes: 0 additions & 34 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,40 +13,6 @@ set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${PROJECT_SOURCE_DIR}/bin)
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

# Find boost
set(REQUIRED_BOOST_COMPONENTS "filesystem;system;thread")
set(Boost_USE_STATIC_LIBS ON)
set(Boost_USE_MULTITHREADED ON)
set(Boost_USE_STATIC_RUNTIME OFF)
if (CMAKE_JS_VERSION)
# Include BoostLib module

if(WIN32)
execute_process(COMMAND cmd /c "npm prefix -g" RESULT_VARIABLE ERR OUTPUT_VARIABLE NPM_GLOBAL_PATH OUTPUT_STRIP_TRAILING_WHITESPACE)
if(ERR)
message(FATAL_ERROR "Failed to get npm global installation path. ${ERR}, ${NPM_GLOBAL_PATH}")
endif(ERR)
string(REPLACE "\\" "/" NPM_GLOBAL_PATH ${NPM_GLOBAL_PATH})
else()
# TODO: For MacOS it may be different. Need to verify.
execute_process(COMMAND npm prefix -g RESULT_VARIABLE ERR OUTPUT_VARIABLE NPM_GLOBAL_PATH OUTPUT_STRIP_TRAILING_WHITESPACE)
if(ERR)
message(FATAL_ERROR "Failed to get npm global installation path. ${ERR}, ${NPM_GLOBAL_PATH}")
endif(ERR)
string(CONCAT NPM_GLOBAL_PATH ${NPM_GLOBAL_PATH} "/lib")
endif()

list(APPEND CMAKE_MODULE_PATH "${NPM_GLOBAL_PATH}/node_modules/boost-lib/cmake")
include(BoostLib)

require_boost_libs(">= 1.62.0" "${REQUIRED_BOOST_COMPONENTS}")
else()
find_package(Boost 1.62.0 REQUIRED "${REQUIRED_BOOST_COMPONENTS}")
if(NOT Boost_FOUND)
message(FATAL_ERROR "Could not find the boost package. Try setting BOOST_ROOT variable")
endif()
endif()

# Build napa shared library.
add_subdirectory(src)

Expand Down
22 changes: 17 additions & 5 deletions cpp-test/component/worker-tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,20 @@ class TestTask : public Task {
// Make sure V8 it initialized exactly once.
static NapaInitializationGuard _guard;

TEST_CASE("worker runs setup complete callback", "[scheduler-worker]") {
std::mutex mutex;
std::condition_variable cv;

auto worker = std::make_unique<Worker>(0, ZoneSettings(), [&cv](WorkerId) {
cv.notify_one();
}, [](WorkerId) {});

bool setupCompleted = (cv.wait_for(lock, std::chrono::milliseconds(1000)) == std::cv_status::no_timeout);
REQUIRE(setupCompleted == true);
}

TEST_CASE("worker runs scheduled task", "[scheduler-worker]") {
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {});
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {}, [](WorkerId) {});

auto task = std::make_shared<TestTask>();
worker->Schedule(task);
Expand All @@ -49,7 +61,7 @@ TEST_CASE("worker notifies idle condition", "[scheduler-worker]") {
std::mutex mutex;
std::condition_variable cv;

auto worker = std::make_unique<Worker>(0, ZoneSettings(), [&cv](WorkerId) {
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {}, [&cv](WorkerId) {
cv.notify_one();
});

Expand All @@ -62,7 +74,7 @@ TEST_CASE("worker notifies idle condition", "[scheduler-worker]") {
}

TEST_CASE("worker runs all tasks before shutting down", "[scheduler-worker]") {
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {});
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {}, [](WorkerId) {});

auto task = std::make_shared<TestTask>();

Expand All @@ -76,7 +88,7 @@ TEST_CASE("worker runs all tasks before shutting down", "[scheduler-worker]") {
}

TEST_CASE("worker runs javascript task", "[scheduler-worker]") {
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {});
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {}, [](WorkerId) {});

auto task = std::make_shared<TestTask>([]() {
auto isolate = v8::Isolate::GetCurrent();
Expand All @@ -102,7 +114,7 @@ TEST_CASE("worker runs javascript task", "[scheduler-worker]") {
}

TEST_CASE("worker runs javascript with stack overflow", "[scheduler-worker]") {
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {});
auto worker = std::make_unique<Worker>(0, ZoneSettings(), [](WorkerId) {}, [](WorkerId) {});

auto task = std::make_shared<TestTask>([]() {
auto isolate = v8::Isolate::GetCurrent();
Expand Down
15 changes: 7 additions & 8 deletions cpp-test/module/module-loader/module-loader-tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@
#include <napa/module/module-internal.h>
#include <napa/v8-helpers.h>
#include <platform/dll.h>
#include <platform/filesystem.h>
#include <platform/platform.h>
#include <utils/string.h>
#include <zone/scheduler.h>
#include <zone/napa-zone.h>
#include <v8/array-buffer-allocator.h>
#include <v8/v8-common.h>

#include <boost/filesystem.hpp>

#include <chrono>
#include <condition_variable>
#include <iostream>
Expand Down Expand Up @@ -177,8 +176,8 @@ TEST_CASE("require.resolve", "[module-loader]") {

auto result = RunScript(oss.str(), [](v8::Local<v8::Value> run) {
v8::String::Utf8Value value(run);
auto filePath = boost::filesystem::current_path() / "jsmodule.js";
return filePath.string().compare(*value) == 0;
auto filePath = filesystem::CurrentDirectory() / "jsmodule.js";
return filePath.String().compare(*value) == 0;
});
REQUIRE(result);

Expand Down Expand Up @@ -214,7 +213,7 @@ TEST_CASE("process", "[module-loader]") {

result = RunScript(oss.str(), [](v8::Local<v8::Value> run) {
v8::String::Utf8Value value(run);
return boost::filesystem::exists(boost::filesystem::path(*value));
return filesystem::Exists(filesystem::Path(*value));
});
REQUIRE(result);

Expand All @@ -223,7 +222,7 @@ TEST_CASE("process", "[module-loader]") {

result = RunScript(oss.str(), [](v8::Local<v8::Value> run) {
v8::String::Utf8Value value(run);
return dll::ThisLineLocation() == boost::filesystem::path(*value).string();
return dll::ThisLineLocation() == *value;
});
REQUIRE(result);

Expand Down Expand Up @@ -297,8 +296,8 @@ TEST_CASE("resolve modules", "[module-loader]") {
}

TEST_CASE("resolve full path modules", "[module-loader]") {
auto filePath = boost::filesystem::current_path() / "tests\\sub\\sub1\\file2.js";
auto filePathString = filePath.string();
auto filePath = filesystem::CurrentDirectory() / "tests\\sub\\sub1\\file2.js";
auto filePathString = filePath.String();
utils::string::ReplaceAll(filePathString, "\\", "\\\\");

std::ostringstream oss;
Expand Down
25 changes: 12 additions & 13 deletions cpp-test/unit/module/module-resolver-tests.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
#include "catch.hpp"

#include <module/loader/module-resolver.h>
#include <platform/filesystem.h>
#include <platform/platform.h>

#include <boost/filesystem.hpp>

#include <sstream>

using namespace napa;
Expand All @@ -17,9 +16,9 @@ namespace {
// Set environment variables before module resolver is initialized.
static bool setNodePath = []() {
std::ostringstream oss;
oss << (boost::filesystem::current_path() / "resolve-env").string()
oss << (filesystem::CurrentDirectory() / "resolve-env").String()
<< platform::ENV_DELIMITER
<< (boost::filesystem::current_path() / "child" / "node_modules" / "child").string();
<< (filesystem::CurrentDirectory() / "child" / "node_modules" / "child").String();
return platform::SetEnv("NODE_PATH", oss.str().c_str());
}();
REQUIRE(setNodePath);
Expand All @@ -38,7 +37,7 @@ namespace {
}

void ResolveIt(const char* module,
const boost::filesystem::path& expected,
const filesystem::Path& expected,
ModuleType type) {
auto detail = GetModuleResolver().Resolve(module);

Expand All @@ -47,20 +46,20 @@ namespace {
}

void ResolveIt(const char* module,
const boost::filesystem::path& expected,
const filesystem::Path& expected,
ModuleType type,
const boost::filesystem::path& package) {
const filesystem::Path& package) {
auto detail = GetModuleResolver().Resolve(module);

REQUIRE(detail.fullPath == expected);
REQUIRE(detail.type == type);
REQUIRE(detail.packageJsonPath == package.string());
REQUIRE(detail.packageJsonPath == package.String());
}

} // Namespace of anonymous namespace.

TEST_CASE("resolve node modules correctly.", "[module-resolver]") {
auto currentPath = boost::filesystem::current_path();
auto currentPath = filesystem::CurrentDirectory();

SECTION("resolve built-in or core modules") {
// Set build-in and core modules.
Expand Down Expand Up @@ -110,14 +109,14 @@ TEST_CASE("resolve node modules correctly.", "[module-resolver]") {

// Starts with "../" and a file exists.
std::ostringstream oss;
oss << "../" << currentPath.filename().string() << "/resolve-file";
oss << "../" << currentPath.Filename().String() << "/resolve-file";
ResolveIt(oss.str().c_str(), currentPath / "resolve-file", ModuleType::JAVASCRIPT);

// Starts with "/", but a file doesn't exist.
ResolveIt("/resolve-file", "", ModuleType::NONE);

// Use absolute path and a file exists.
ResolveIt((currentPath / "resolve-file").string().c_str(),
ResolveIt((currentPath / "resolve-file").c_str(),
currentPath / "resolve-file",
ModuleType::JAVASCRIPT);
}
Expand Down Expand Up @@ -149,7 +148,7 @@ TEST_CASE("resolve node modules correctly.", "[module-resolver]") {

// Starts with "../" and a directory exists.
std::ostringstream oss;
oss << "../" << currentPath.filename().string() << "/resolve-directory/resolver";
oss << "../" << currentPath.Filename().String() << "/resolve-directory/resolver";
ResolveIt(oss.str().c_str(),
currentPath / "resolve-directory" / "resolver" / "resolve-file",
ModuleType::JAVASCRIPT,
Expand All @@ -159,7 +158,7 @@ TEST_CASE("resolve node modules correctly.", "[module-resolver]") {
ResolveIt("/resolve-directory/resolver", "", ModuleType::NONE);

// Use absolute path and a file exists.
ResolveIt((currentPath / "resolve-directory" / "resolver").string().c_str(),
ResolveIt((currentPath / "resolve-directory" / "resolver").c_str(),
currentPath / "resolve-directory" / "resolver" / "resolve-file",
ModuleType::JAVASCRIPT,
currentPath / "resolve-directory" / "resolver" / "package.json");
Expand Down
2 changes: 1 addition & 1 deletion docs/api/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Napa.JS follows [Node.JS' approach](https://nodejs.org/api/modules.html) to supp
But there are also differences:
1) C++ module that is designed/implemented for Napa.JS can run on Node.JS (need different compile flags to produce '.napa' and '.node'). But not vice versal.
2) Napa.JS hasn't supported all Node.JS API yet. We follow an [incremental approach](./node-api.md) to add Node.JS built-ins and core modules that are needed for computation heavy tasks. You can access full capabilities of Node exposed via [Node zone](./zone.md#node-zone).
3) Napa.JS doesn't provide `uv` functionalities, instead built-ins and core modules are implemented in `boost`.
3) Napa.JS doesn't provide `uv` functionalities, instead built-ins and core modules have its own implementation.
4) Napa.JS supports embed mode. C++ modules need separate compilation between Node mode and embed mode.


Expand Down
2 changes: 0 additions & 2 deletions node/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ set_target_properties(${TARGET_NAME} PROPERTIES PREFIX "" SUFFIX ".node")
# Include directories
target_include_directories(${TARGET_NAME} PRIVATE
${CMAKE_JS_INC}
${Boost_INCLUDE_DIRS}
${PROJECT_SOURCE_DIR}/src
${PROJECT_SOURCE_DIR}/src/module/core-modules/napa)

Expand All @@ -29,5 +28,4 @@ target_compile_definitions(${TARGET_NAME} PRIVATE BUILDING_NODE_EXTENSION NAPA_B
# Link libraries
target_link_libraries(${TARGET_NAME} PRIVATE
${PROJECT_NAME}
${Boost_LIBRARIES}
${CMAKE_JS_LIB})
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
},
"scripts": {
"install": "cmake-js compile",
"buildd": "cmake-js compile --debug",
"prepublish": "tsc -p lib",
"pretest": "tsc -p test",
"test": "mocha test --recursive",
Expand Down
6 changes: 1 addition & 5 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ target_include_directories(${TARGET_NAME}
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}
${PROJECT_SOURCE_DIR}/third-party
${Boost_INCLUDE_DIRS}
PUBLIC
${PROJECT_SOURCE_DIR}/inc)

# Compiler definitions
target_compile_definitions(${TARGET_NAME} PRIVATE NAPA_EXPORTS BUILDING_NAPA_EXTENSION NAPA_BINDING_EXPORTS)

# Link libraries
target_link_libraries(${TARGET_NAME} PRIVATE ${Boost_LIBRARIES})
target_link_libraries(${TARGET_NAME} PRIVATE)

if (CMAKE_JS_VERSION)
# Building Napa as an npm package for node usage (using exported v8 from node.exe)
Expand Down Expand Up @@ -69,7 +68,4 @@ endif()

if (WIN32)
target_link_libraries(${TARGET_NAME} PRIVATE winmm.lib)

# Define this to fix boost asio related warning ('Please define _WIN32_WINNT or _WIN32_WINDOWS appropriately')
target_compile_definitions(${TARGET_NAME} PRIVATE _WIN32_WINNT=0x0501)
endif()
3 changes: 1 addition & 2 deletions src/api/api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
#include "v8/v8-common.h"
#include "zone/napa-zone.h"
#include "zone/node-zone.h"
#include "zone/worker-context.h"

#include <napa-log.h>

#include <boost/filesystem.hpp>

#include <atomic>
#include <fstream>
#include <sstream>
Expand Down
20 changes: 9 additions & 11 deletions src/module/core-modules/node/file-system-helpers.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// TODO: we should remove this macro once we have our own filesystem implementation.
#define _CRT_SECURE_NO_DEPRECATE
#include "file-system-helpers.h"
#include <platform/filesystem.h>

#include <boost/filesystem.hpp>

#include <algorithm>
#include <functional>
#include <memory>
#include <sstream>

Expand All @@ -14,7 +15,7 @@ using namespace napa::module;
namespace {

std::string GetFileFullPath(const std::string& file) {
return boost::filesystem::absolute(file).string();
return filesystem::Path(file).Absolute().Normalize().String();
}

} // End of anonymous namespace.
Expand Down Expand Up @@ -77,8 +78,8 @@ void file_system_helpers::WriteFileSync(const std::string& filename, const char*
}

void file_system_helpers::MkdirSync(const std::string& directory) {
boost::filesystem::path path(GetFileFullPath(directory));
if (!boost::filesystem::exists(path) && !boost::filesystem::create_directory(path))
filesystem::Path path(GetFileFullPath(directory));
if (!filesystem::IsDirectory(path) && !filesystem::MakeDirectory(path))
{
std::ostringstream oss;
oss << "The directory: " << directory << " doesn't exist, and can't be created.";
Expand All @@ -87,17 +88,14 @@ void file_system_helpers::MkdirSync(const std::string& directory) {
}

bool file_system_helpers::ExistsSync(const std::string& path) {
auto fullPath = GetFileFullPath(path);
return boost::filesystem::exists(fullPath);
return filesystem::Exists(GetFileFullPath(path));
}

std::vector<std::string> file_system_helpers::ReadDirectorySync(const std::string& directory) {
boost::filesystem::path path(GetFileFullPath(directory));

std::vector<std::string> names;
for (const auto& entry : boost::filesystem::directory_iterator(path)) {
names.emplace_back(entry.path().filename().string());
filesystem::PathIterator iterator(GetFileFullPath(directory));
while (iterator.Next()) {
names.emplace_back(iterator->Filename().String());
}

return names;
}
Loading

0 comments on commit 08767c1

Please sign in to comment.