Skip to content

Commit

Permalink
Merge pull request #52 from simpsont-oci/portability_improvements
Browse files Browse the repository at this point in the history 
CI and Portability Improvements
  • Loading branch information
@simpsont-oci
simpsont-oci authored Sep 22, 2022
2 parents e09ecc8 + dcaaa1b commit ee4f24f
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 123 deletions.
61 changes: 25 additions & 36 deletions .github/workflows/build.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ jobs:
fail-fast: false
matrix:
node_version:
- 12
- 14
- 16
- 18
opendds_branch:
- master
- latest-release
Expand All @@ -33,6 +33,25 @@ jobs:
- {os: macos-latest, dds_security: 0}
- {os: windows-2019, dds_security: 1}
- {os: windows-2019, dds_security: 0}
include:
- node_version: 8
opendds_branch: latest-release
m: {os: ubuntu-latest, security: 0}
- node_version: 8
opendds_branch: latest-release
m: {os: windows-2019, security: 0}
- node_version: 10
opendds_branch: latest-release
m: {os: ubuntu-latest, security: 1}
- node_version: 10
opendds_branch: latest-release
m: {os: windows-2019, security: 1}
- node_version: 12
opendds_branch: latest-release
m: {os: ubuntu-latest, security: 1}
- node_version: 12
opendds_branch: latest-release
m: {os: windows-2019, security: 1}

runs-on: ${{ matrix.m.os }}
steps:
Expand All @@ -57,6 +76,10 @@ jobs:
ref: ${{ matrix.opendds_branch }}
path: OpenDDS
fetch-depth: 1
- name: 'Set Up node'
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node_version }}
- name: 'Install ssl and xerces (ubuntu)'
if: ${{ matrix.m.dds_security == 1 && matrix.m.os == 'ubuntu-latest' }}
run: |-
Expand All @@ -82,11 +105,6 @@ jobs:
- name: 'Set Up Problem Matcher (ubuntu / macos)'
if: ${{ matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest' }}
uses: ammaraskar/[email protected]
- name: 'Set Up node (windows)'
if: ${{ matrix.m.os == 'windows-2019' }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node_version }}
- name: 'Set environment variables (ubuntu / macos)'
if: ${{ matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest' }}
shell: bash
Expand All @@ -95,7 +113,6 @@ jobs:
echo "TAO_ROOT=$GITHUB_WORKSPACE/ACE_TAO/TAO" >> $GITHUB_ENV
echo "DDS_ROOT=$GITHUB_WORKSPACE/OpenDDS" >> $GITHUB_ENV
echo "MPC_ROOT=$GITHUB_WORKSPACE/MPC" >> $GITHUB_ENV
echo "NVM_DIR=$GITHUB_WORKSPACE/nvm" >> $GITHUB_ENV
echo "npm_config_devdir=$GITHUB_WORKSPACE/opendds-node-gyp-devdir" >> $GITHUB_ENV
echo "LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$GITHUB_WORKSPACE/ACE_TAO/ACE/lib:$GITHUB_WORKSPACE/OpenDDS/lib" >> $GITHUB_ENV
if [ ${{ matrix.node_version }} == 16 ]; then
Expand All @@ -120,7 +137,6 @@ jobs:
echo "TAO_ROOT=$GITHUB_WORKSPACE\\ACE_TAO\\TAO" >> $GITHUB_ENV
echo "DDS_ROOT=$GITHUB_WORKSPACE\\OpenDDS" >> $GITHUB_ENV
echo "MPC_ROOT=$GITHUB_WORKSPACE\\MPC" >> $GITHUB_ENV
echo "NVM_DIR=$GITHUB_WORKSPACE\\nvm" >> $GITHUB_ENV
echo "npm_config_devdir=$GITHUB_WORKSPACE\\opendds-node-gyp-devdir" >> $GITHUB_ENV
if [ ${{ matrix.m.dds_security }} == 1 ]; then
CONFIG_OPTIONS+=" --security";
Expand Down Expand Up @@ -154,38 +170,17 @@ jobs:
cd OpenDDS
call setenv.cmd
msbuild -p:Configuration=Debug,Platform=x64 -t:${{ env.BUILD_TARGETS }} -m DDS_TAOv2.sln
- name: 'Checkout nvm'
uses: actions/checkout@v3
with:
repository: nvm-sh/nvm.git
path: ${{ env.NVM_DIR }}
ref: v0.39.0
fetch-depth: 1
- name: 'Install node, Build (ubuntu / macos)'
- name: 'Install (ubuntu / macos)'
if: ${{ matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest' }}
shell: bash
run: |-
set -e
cd ${{ env.NVM_DIR }}
[ -s "./nvm.sh" ] && \. "./nvm.sh" # loads nvm
cd $GITHUB_WORKSPACE
nvm install ${{ matrix.node_version }}
nvm use ${{ matrix.node_version }}
echo "node:$(node -v) npm:$(npm -v)"
echo "Build ========== ========== =========="
npm install
${{ env.ACE_ROOT }}/bin/mwc.pl -type gnuace -exclude ACE_TAO,OpenDDS
make -j2 || make
- name: 'Test (ubuntu / macos)'
if: ${{ matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest' }}
shell: bash
run: |-
set -e
cd ${{ env.NVM_DIR }}
[ -s "./nvm.sh" ] && \. "./nvm.sh" # loads nvm
cd $GITHUB_WORKSPACE
nvm use ${{ matrix.node_version }}
echo "node:$(node -v) npm:$(npm -v)"
cd test
./run_test.pl cpp2node
./run_test.pl node2cpp
Expand All @@ -197,12 +192,6 @@ jobs:
if: ${{ (matrix.m.os == 'ubuntu-latest' || matrix.m.os == 'macos-latest') && matrix.m.dds_security == 1 }}
shell: bash
run: |-
set -e
cd ${{ env.NVM_DIR }}
[ -s "./nvm.sh" ] && \. "./nvm.sh" # loads nvm
cd $GITHUB_WORKSPACE
nvm use ${{ matrix.node_version }}
echo "node:$(node -v) npm:$(npm -v)"
cd test
./run_test.pl cpp2node --rtps --secure
./run_test.pl node2cpp --rtps --secure
Expand Down
8 changes: 5 additions & 3 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
** If using security, you may need to ensure that OpenDDS is built with the same version of OpenSSL used by Node.js

## Tested platforms:
* Node LTS versions 12, 14, 16
* Node LTS versions 8, 10, 12, 14, 16, 18
* Linux (Ubuntu 20.04) x86_64 using gcc 9.4.0
* Linux (Ubuntu 22.04) x86_64 using gcc 11.2.0 (w/ openssl-1.1.1q)
* Windows (Server 2022) x86_64 using Visual Studio Enterprise 2022
Expand Down Expand Up @@ -40,9 +40,11 @@ $ node-gyp configure build

* Remove requirement for V8 TypeSupport generation and NodeQosConversion
** uses ValueReader/Writer Implementation for NaN / Node.js
** requires OpenDDS 3.22
** requires OpenDDS 3.22, will not build with older versions of OpenDDS
** slightly changes QoS object formatting to align with IDL types
* Adds support and test coverage for Linux / MacOS / Windows
* Improved version support and CI test coverage
** Good Coverage of Node.js 14, 16, 18 (Linux / macOS / Windows)
** Limited Coverage of Node.js 8, 10, 12 (Linux / Windows)

### Version 0.1.1

Expand Down
110 changes: 59 additions & 51 deletions src/NodeValueReader.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
#include "NodeValueReader.h"

#include <codecvt>
#include <locale>

#if NODE_MAJOR_VERSION > 10 || (NODE_MAJOR_VERSION == 10 && NODE_MINOR_VERSION >= 4)
#define HAS_BIGINT
#endif

#ifdef HAS_BIGINT
// This is a bit of a hack to make Nan::To work for BigInt
// It should be OK as long as we avoid trying to reference BigInt before 10.4
namespace Nan {
namespace imp {

template<>
struct ToFactory<v8::BigInt> : ToFactoryBase<v8::BigInt> {
static inline return_t convert(v8::Local<v8::Value> val) {
v8::Isolate *isolate = v8::Isolate::GetCurrent();
v8::EscapableHandleScope scope(isolate);
return scope.Escape(val->ToBigInt(isolate->GetCurrentContext()).FromMaybe(v8::Local<v8::BigInt>()));
}
};

}
}
#endif

namespace NodeOpenDDS {

NodeValueReader::NodeValueReader(v8::Local<v8::Object> obj)
Expand Down Expand Up @@ -29,13 +55,9 @@ bool NodeValueReader::begin_struct_member(OpenDDS::XTypes::MemberId& member_id,
Nan::MaybeLocal<v8::Value> mlv = Nan::Get(property_names_.ToLocalChecked(), current_index_);
if (!mlv.IsEmpty()) {
current_property_name_ = mlv.ToLocalChecked();
v8::Local<v8::String> name = v8::Local<v8::String>::Cast(current_property_name_);
if (!name.IsEmpty()) {
std::string str(name->Utf8Length(v8::Isolate::GetCurrent()), 0);
name->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]);
if (helper.get_value(member_id, str.c_str())) {
return true;
}
Nan::Utf8String name(current_property_name_);
if (helper.get_value(member_id, *name)) {
return true;
}
}
++current_index_;
Expand Down Expand Up @@ -67,13 +89,9 @@ bool NodeValueReader::begin_discriminator()
Nan::MaybeLocal<v8::Value> mlv = Nan::Get(property_names_.ToLocalChecked(), current_index_);
if (!mlv.IsEmpty()) {
current_property_name_ = mlv.ToLocalChecked();
v8::Local<v8::String> name = v8::Local<v8::String>::Cast(current_property_name_);
if (!name.IsEmpty()) {
std::string str(name->Utf8Length(v8::Isolate::GetCurrent()), 0);
name->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]);
if (str == "$discriminator") {
return true;
}
Nan::Utf8String name(current_property_name_);
if (std::strcmp(*name, "$discriminator") == 0) {
return true;
}
}
++current_index_;
Expand All @@ -95,13 +113,9 @@ bool NodeValueReader::begin_union_member()
Nan::MaybeLocal<v8::Value> mlv = Nan::Get(property_names_.ToLocalChecked(), current_index_);
if (!mlv.IsEmpty()) {
current_property_name_ = mlv.ToLocalChecked();
v8::Local<v8::String> name = v8::Local<v8::String>::Cast(current_property_name_);
if (!name.IsEmpty()) {
std::string str(name->Utf8Length(v8::Isolate::GetCurrent()), 0);
name->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]);
if (str != "$discriminator") {
return true;
}
Nan::Utf8String name(current_property_name_);
if (std::strcmp(*name, "$discriminator") != 0) {
return true;
}
}
++current_index_;
Expand Down Expand Up @@ -192,10 +206,7 @@ bool NodeValueReader::read_uint16(ACE_CDR::UShort& value)

bool NodeValueReader::read_int32(ACE_CDR::Long& value)
{
bool result = primitive_helper<v8::Integer>(value, &v8::Value::IsNumber, strtol);
if (result) {
}
return result;
return primitive_helper<v8::Integer>(value, &v8::Value::IsNumber, strtol);
}

bool NodeValueReader::read_uint32(ACE_CDR::ULong& value)
Expand All @@ -207,15 +218,15 @@ bool NodeValueReader::read_int64(ACE_CDR::LongLong& value)
{
Nan::MaybeLocal<v8::Value> mlvai = use_name_ ? Nan::Get(current_object_, current_property_name_) : Nan::Get(current_object_, current_index_);
if (!mlvai.IsEmpty()) {
// This doesn't work because NaN seems to lack conversion support to BigInt
// Not super surprising, since BigInt support wasn't introduced until Node.js 10.4
//{
// Nan::MaybeLocal<v8::BigInt> tov = Nan::To<v8::BigInt>(mlvai.ToLocalChecked());
// if (!tov.IsEmpty()) {
// value = static_cast<ACE_CDR::LongLong>(tov.ToLocalChecked()->Int64Value());
// return true;
// }
//}
#ifdef HAS_BIGINT
if (!mlvai.ToLocalChecked()->IsNumber()) {
Nan::MaybeLocal<v8::BigInt> tov = Nan::To<v8::BigInt>(mlvai.ToLocalChecked());
if (!tov.IsEmpty()) {
value = static_cast<ACE_CDR::LongLong>(tov.ToLocalChecked()->Int64Value());
return true;
}
}
#endif
{
Nan::MaybeLocal<v8::String> tov = Nan::To<v8::String>(mlvai.ToLocalChecked());
if (!tov.IsEmpty()) {
Expand All @@ -233,15 +244,15 @@ bool NodeValueReader::read_uint64(ACE_CDR::ULongLong& value)
{
Nan::MaybeLocal<v8::Value> mlvai = use_name_ ? Nan::Get(current_object_, current_property_name_) : Nan::Get(current_object_, current_index_);
if (!mlvai.IsEmpty()) {
// This doesn't work because NaN seems to lack conversion support to BigInt
// Not super surprising, since BigInt support wasn't introduced until Node.js 10.4
//{
// Nan::MaybeLocal<v8::BigInt> tov = Nan::To<v8::BigInt>(mlvai.ToLocalChecked());
// if (!tov.IsEmpty()) {
// value = static_cast<ACE_CDR::LongLong>(tov.ToLocalChecked()->UInt64Value());
// return true;
// }
//}
#ifdef HAS_BIGINT
if (!mlvai.ToLocalChecked()->IsNumber()) {
Nan::MaybeLocal<v8::BigInt> tov = Nan::To<v8::BigInt>(mlvai.ToLocalChecked());
if (!tov.IsEmpty()) {
value = static_cast<ACE_CDR::LongLong>(tov.ToLocalChecked()->Uint64Value());
return true;
}
}
#endif
{
Nan::MaybeLocal<v8::String> tov = Nan::To<v8::String>(mlvai.ToLocalChecked());
if (!tov.IsEmpty()) {
Expand Down Expand Up @@ -310,8 +321,8 @@ bool NodeValueReader::read_string(std::string& value)
if (!mlvai.IsEmpty()) {
Nan::MaybeLocal<v8::String> tov = Nan::To<v8::String>(mlvai.ToLocalChecked());
if (!tov.IsEmpty()) {
value.resize(tov.ToLocalChecked()->Utf8Length(v8::Isolate::GetCurrent()), 0);
tov.ToLocalChecked()->WriteUtf8(v8::Isolate::GetCurrent(), &value[0], -1, 0, v8::String::NO_NULL_TERMINATION);
Nan::Utf8String str(tov.ToLocalChecked());
value = *str;
return true;
}
}
Expand All @@ -324,12 +335,9 @@ bool NodeValueReader::read_wstring(std::wstring& value)
if (!mlvai.IsEmpty()) {
Nan::MaybeLocal<v8::String> tov = Nan::To<v8::String>(mlvai.ToLocalChecked());
if (!tov.IsEmpty()) {
std::vector<uint16_t> temp(tov.ToLocalChecked()->Length(), 0);
tov.ToLocalChecked()->Write(v8::Isolate::GetCurrent(), &temp[0], 0, -1, v8::String::NO_NULL_TERMINATION);
value.resize(temp.size(), 0);
for (size_t i = 0; i < temp.size(); ++i) {
value[i] = static_cast<wchar_t>(temp[i]);
}
Nan::Utf8String str(tov.ToLocalChecked());
std::wstring_convert<std::codecvt_utf8<wchar_t>, wchar_t> wconv;
value = wconv.from_bytes(*str);
return true;
}
}
Expand Down
12 changes: 4 additions & 8 deletions src/NodeValueReader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,8 @@ class NodeValueReader : public OpenDDS::DCPS::ValueReader {
}
}
if (lvai->IsString()) {
v8::Local<v8::String> ls = v8::Local<v8::String>::Cast(lvai);
std::string str(ls->Utf8Length(v8::Isolate::GetCurrent()), 0);
ls->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]);
safe_assign(value, (*str_conv)(str.c_str(), 0, str.find("0x") != std::string::npos ? 16 : 10));
Nan::Utf8String str(lvai);
safe_assign(value, (*str_conv)(*str, 0, std::string(*str).find("0x") != std::string::npos ? 16 : 10));
return true;
}
}
Expand All @@ -138,10 +136,8 @@ class NodeValueReader : public OpenDDS::DCPS::ValueReader {
}
}
if (lvai->IsString()) {
v8::Local<v8::String> ls = v8::Local<v8::String>::Cast(lvai);
std::string str(ls->Utf8Length(v8::Isolate::GetCurrent()), 0);
ls->WriteUtf8(v8::Isolate::GetCurrent(), &str[0]);
safe_assign(value, (*str_conv)(str.c_str(), 0));
Nan::Utf8String str(lvai);
safe_assign(value, (*str_conv)(*str, 0));
return true;
}
}
Expand Down
Loading

0 comments on commit ee4f24f

Please sign in to comment.