You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/10/14 07:49:05 UTC

[GitHub] [pulsar-client-node] shibd opened a new pull request, #235: [feat] Napi support package c++ static binary.

shibd opened a new pull request, #235:
URL: https://github.com/apache/pulsar-client-node/pull/235

   ### Motivation
   
   Currently, users using the `node js client` must first install the `C++ client`. And when the user executes `npm install`, it also needs to be compiled. This is not user-friendly.
   
   This PR is to provide pre-built packages for all the supported environments, using [GYP]() and rebuild. The pre-built package is then uploaded to [`Github Release`](https://github.com/shibd/pulsar-client-node/releases/tag/v1.7.0-rc.10), and when the user installs the client, the system version is matched and the pre-built package is downloaded.
   
   Currently tested `linux_x86` and `mac_x86`. I've built it in my [fork repository](https://github.com/shibd/pulsar-client-node/releases/tag/v1.7.0-rc.10) and can install it directly using the following command (without first installing the C++ client).
   
   ```
   npm intall shibaodi-pulsar-client
   ```
   
   ### Modifications
   
   #### New Github workflows
   
   - When trigger release: 
      1. Build the binary C++ static library first(use new c++ client 3.0 source).
      2. And use [node-pre-gyp](https://github.com/mapbox/node-pre-gyp) pre-built `napi` packages.
      3. Use [node-pre-gyp-github](https://github.com/bchr02/node-pre-gyp-github) publish this package to `GitHub Release`.
   - When trigger unit test(use ubuntu GitHub runner):
      1. Install c++ lib(use new c++ client 3.0 version).
      2. Use this c++ lib build `napi`.
      3. Use pulsar image start broker(use pulsar latest).
      5. Run the unit test.
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
   - [x] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### TODO
   - [ ] Add pre-build windows binary package.
   - [ ] Change user docs.
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] merlimat commented on pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
merlimat commented on PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#issuecomment-1284711651

   > macOS_arm64: Not supported at the moment. Although the cpp client can be properly compiled on the x86_64 by cross-compilation, it is not successful when packaging NAPI. Maybe we need a macOS_m1 runner to run the compilation.
   
   We can try to find a way to cross-compile. Right now there are no M1 runners in GH actions. 
   Let this not be a blocker. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] merlimat commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1002504912


##########
build-support/stage-release.sh:
##########
@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+set -e -x
+
+if [ $# -neq 2 ]; then
+    echo "Usage: $0 \$DEST_PATH \$WORKFLOW_ID"
+    exit 1
+fi
+
+DEST_PATH=$(readlink -f $1)
+WORKFLOW_ID=$2
+
+pushd $(dirname "$0")
+PULSAR_NODE_PATH=$(git rev-parse --show-toplevel)
+popd
+
+mkdir -p $DEST_PATH
+
+cd $PULSAR_NODE_PATH
+
+build-support/download-release-artifacts.py $WORKFLOW_ID $DEST_PATH
+build-support/generate-source-archive.sh $DEST_PATH
+
+# Sign all files
+#cd $DEST_PATH
+#find . -type f | xargs $PULSAR_NODE_PATH/build-support/sign-files.sh

Review Comment:
   This should be uncommented



##########
build-support/pulsar-test-service-stop.sh:
##########
@@ -1,4 +1,6 @@
-#!/bin/bash
+
+
+#!/usr/bin/env bash

Review Comment:
   I think there should be no empty lines



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] shibd commented on pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
shibd commented on PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#issuecomment-1284249014

   Currently completed:
   - linux_glibc_x86_64
   - linux_musl_x86_64
   - linux_glibc_arm64
   - linux_musl_arm64
   - macOS_x86_64
   
   Please refer to my repo release: https://github.com/shibd/pulsar-client-node/releases/tag/v1.8.1-rc.3
   
   Other environments:
   - `macOS_arm64`: Not supported at the moment. Although the `cpp client` can be properly compiled on the `x86_64` by cross-compilation, it is not successful when packaging `NAPI`. Maybe we need a `macOS_m1` runner to run the compilation.
   - `windows_x86_64`: I'm doing compilation testing, which will be added later.
   
   @merlimat Do you feel there are other environments that need support?
   
   @BewareMyPower @merlimat This PR can be reviewed first when you have time. Thanks.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] merlimat commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1000019013


##########
.github/workflows/ci-build-release-napi.yml:
##########
@@ -0,0 +1,130 @@
+name: Build release napi
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - master
+    tags:
+      - '*'
+env:
+  FORCE_COLOR: 1
+concurrency:
+  group: ${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+jobs:
+  macos-napi:
+    name: Build NAPI macos - Node ${{matrix.nodejs}} - ${{matrix.arch}}
+    runs-on: macos-latest
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        arch:
+          - x86_64
+        nodejs:
+          - 16
+          - 18
+    steps:
+      - uses: actions/checkout@v3
+      - name: Use Node.js ${{ matrix.nodejs }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.nodejs }}
+          cache: 'npm'
+
+      - name: Cache Dependencies
+        id: cache-dependencies
+        uses: actions/cache@v3
+        with:
+          path: pkg/mac/build
+          key: ${{ runner.os }}-${{ matrix.arch }}-mac-${{ hashFiles('pkg/mac/build-cpp-deps-lib.sh') }}
+
+      - name: Build CPP dependencies lib
+        if: steps.cache-dependencies.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-deps-lib.sh
+
+      - name: Build CPP lib
+        if: steps.cache-pulsar.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-lib.sh
+
+      - name: Build Node binaries lib
+        run: |
+          npm install --ignore-scripts
+          npx node-pre-gyp configure
+          npx node-pre-gyp build
+
+      - name: Test loading Node binaries lib
+        run: |
+          node pkg/load_test.js
+
+      - name: Package Node binaries lib
+        run: |
+          npx node-pre-gyp package
+
+      - name: Upload Node binaries lib to GitHub Release
+        env:
+          NODE_PRE_GYP_GITHUB_TOKEN: ${{ secrets.GITHUBTOKEN }}
+        if: matrix.nodejs == 16 && startsWith(github.ref, 'refs/tags/')
+        run: |
+          echo $NODE_PRE_GYP_GITHUB_TOKEN
+          npx node-pre-gyp-github publish
+
+  linux-napi:
+    name: Build NAPI ${{matrix.image}} - Node ${{matrix.nodejs}} - ${{matrix.cpu.platform}}
+    runs-on: ubuntu-22.04
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        image:
+          - 'linux_glibc'
+          - 'linux_musl'
+        nodejs:
+          - 16
+          - 18
+        cpu:
+          - {arch: 'x86_64', platform: 'x86_64'}
+          - {arch: 'aarch64', platform: 'arm64'}
+
+    steps:
+      - name: checkout
+        uses: actions/checkout@v3
+
+      - name: Set up QEMU
+        uses: docker/setup-qemu-action@v2
+
+      - name: Setup Docker Buildx
+        uses: docker/setup-buildx-action@v2
+
+      - name: Build Linux Docker image
+        uses: docker/build-push-action@v3
+        with:
+          context: ./pkg/${{matrix.image}}
+          load: true
+          tags: build:latest
+          platforms: linux/${{matrix.cpu.arch}}
+          build-args: |
+            PLATFORM=${{matrix.cpu.platform}}
+            ARCH=${{matrix.cpu.arch}}
+            NODE_VERSION=${{matrix.nodejs}}
+          cache-from: type=gha
+          cache-to: type=gha,mode=max
+
+      - name: Build and Test NAPI file
+        run: |
+          docker run -i -v $PWD:/pulsar-client-node build:latest \
+              /pulsar-client-node/pkg/build-napi-inside-docker.sh
+
+      - name: Upload Node binaries lib to GitHub Release

Review Comment:
   We could try to upload these as github actions artifacts. Then, we can automate to fetch them at release time and add them to the ASF dist Subversion, similar to what we do for C++/Python.  See: 
   
   Then, we can just point GYP to the HTTP URL from ASF download.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] shibd commented on pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
shibd commented on PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#issuecomment-1278692145

   > Why do you rename the directory name from `certificate` to `test-conf`?
   
   Just for the unified, put all the test-related configurations together.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] merlimat merged pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
merlimat merged PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] shibd commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
shibd commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1002355295


##########
.github/workflows/ci-build-release-napi.yml:
##########
@@ -0,0 +1,130 @@
+name: Build release napi
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - master
+    tags:
+      - '*'
+env:
+  FORCE_COLOR: 1
+concurrency:
+  group: ${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+jobs:
+  macos-napi:
+    name: Build NAPI macos - Node ${{matrix.nodejs}} - ${{matrix.arch}}
+    runs-on: macos-latest
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        arch:
+          - x86_64
+        nodejs:
+          - 16
+          - 18
+    steps:
+      - uses: actions/checkout@v3
+      - name: Use Node.js ${{ matrix.nodejs }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.nodejs }}
+          cache: 'npm'
+
+      - name: Cache Dependencies
+        id: cache-dependencies
+        uses: actions/cache@v3
+        with:
+          path: pkg/mac/build
+          key: ${{ runner.os }}-${{ matrix.arch }}-mac-${{ hashFiles('pkg/mac/build-cpp-deps-lib.sh') }}
+
+      - name: Build CPP dependencies lib
+        if: steps.cache-dependencies.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-deps-lib.sh
+
+      - name: Build CPP lib
+        if: steps.cache-pulsar.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-lib.sh
+
+      - name: Build Node binaries lib
+        run: |
+          npm install --ignore-scripts
+          npx node-pre-gyp configure
+          npx node-pre-gyp build
+
+      - name: Test loading Node binaries lib
+        run: |
+          node pkg/load_test.js
+
+      - name: Package Node binaries lib
+        run: |
+          npx node-pre-gyp package
+
+      - name: Upload Node binaries lib to GitHub Release
+        env:
+          NODE_PRE_GYP_GITHUB_TOKEN: ${{ secrets.GITHUBTOKEN }}
+        if: matrix.nodejs == 16 && startsWith(github.ref, 'refs/tags/')
+        run: |
+          echo $NODE_PRE_GYP_GITHUB_TOKEN
+          npx node-pre-gyp-github publish
+
+  linux-napi:
+    name: Build NAPI ${{matrix.image}} - Node ${{matrix.nodejs}} - ${{matrix.cpu.platform}}
+    runs-on: ubuntu-22.04
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        image:
+          - 'linux_glibc'
+          - 'linux_musl'
+        nodejs:
+          - 16
+          - 18
+        cpu:
+          - {arch: 'x86_64', platform: 'x86_64'}
+          - {arch: 'aarch64', platform: 'arm64'}
+
+    steps:
+      - name: checkout
+        uses: actions/checkout@v3
+
+      - name: Set up QEMU
+        uses: docker/setup-qemu-action@v2
+
+      - name: Setup Docker Buildx
+        uses: docker/setup-buildx-action@v2
+
+      - name: Build Linux Docker image
+        uses: docker/build-push-action@v3
+        with:
+          context: ./pkg/${{matrix.image}}
+          load: true
+          tags: build:latest
+          platforms: linux/${{matrix.cpu.arch}}
+          build-args: |
+            PLATFORM=${{matrix.cpu.platform}}
+            ARCH=${{matrix.cpu.arch}}
+            NODE_VERSION=${{matrix.nodejs}}
+          cache-from: type=gha
+          cache-to: type=gha,mode=max
+
+      - name: Build and Test NAPI file
+        run: |
+          docker run -i -v $PWD:/pulsar-client-node build:latest \
+              /pulsar-client-node/pkg/build-napi-inside-docker.sh
+
+      - name: Upload Node binaries lib to GitHub Release

Review Comment:
   I modified it, but I didn't test the whole process. I don't have permission to commit to svn. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] merlimat commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1002546779


##########
pulsar-client-cpp-version.txt:
##########
@@ -0,0 +1 @@
+3.0.0

Review Comment:
   👍 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] BewareMyPower commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r995500769


##########
examples/encryption-consumer.js:
##########
@@ -32,7 +32,7 @@ const Pulsar = require('pulsar-client');
     subscription: 'sub1',
     subscriptionType: 'Shared',
     ackTimeoutMs: 10000,
-    privateKeyPath: "./certificate/private-key.client-rsa.pem"
+    privateKeyPath: "./test-test-conf/private-key.client-rsa.pem"

Review Comment:
   This path is appears to be wrong. It should be `./tests/test-conf`.
   
   BTW, you should not change JS code to adapt your test script. Instead, you should change your test script to adapt the existing code, which is a little different from Python client.



##########
examples/encryption-consumer.js:
##########
@@ -32,7 +32,7 @@ const Pulsar = require('pulsar-client');
     subscription: 'sub1',
     subscriptionType: 'Shared',
     ackTimeoutMs: 10000,
-    privateKeyPath: "./certificate/private-key.client-rsa.pem"
+    privateKeyPath: "./test-test-conf/private-key.client-rsa.pem"

Review Comment:
   This path appears to be wrong. It should be `./tests/test-conf`.
   
   BTW, you should not change JS code to adapt your test script. Instead, you should change your test script to adapt the existing code, which is a little different from Python client.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] merlimat commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1000020826


##########
.github/workflows/ci-pr-validation.yml:
##########
@@ -0,0 +1,40 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+name: PR validation
+on:
+  pull_request:
+    branches: ['master']
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  unit-tests:
+    name: Run unit tests
+    runs-on: ubuntu-22.04
+    timeout-minutes: 120
+
+    steps:
+      - name: checkout
+        uses: actions/checkout@v3
+
+      - name: Test
+        run: |
+          ./run-unit-tests.sh

Review Comment:
   We should also run the pre-gyp build as part of PR validation (to ensure that it keeps working and we don't find the issues only when we're attempting to do a release). 
   
   We should run it for a subset of targets though: eg. linux-glibc-x64_86 & mac_x86_64



##########
pulsar-client-cpp-version.txt:
##########
@@ -0,0 +1 @@
+3.0.0

Review Comment:
   Where are we going to define the version of the wrapper?



##########
src/pulsar-binding.js:
##########
@@ -0,0 +1,8 @@
+const path = require('path');
+const binary = require('@mapbox/node-pre-gyp');
+
+const bindingPath = binary.find(path.resolve(path.join(__dirname, '../package.json')));

Review Comment:
   Would this work on Windows?



##########
.github/workflows/ci-build-release-napi.yml:
##########
@@ -0,0 +1,130 @@
+name: Build release napi
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - master

Review Comment:
   We should probably not running this for `master`



##########
pkg/mac/build-cpp-lib.sh:
##########
@@ -0,0 +1,51 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+source $(dirname $0)/common.sh
+
+PULSAR_DIR=${MAC_BUILD_DIR}/build-pulsar
+PULSAR_PREFIX=${PULSAR_DIR}/install
+mkdir -p $PULSAR_PREFIX
+cd $PULSAR_DIR
+
+# Pulsar
+curl -O -L https://dist.apache.org/repos/dist/dev/pulsar/pulsar-client-cpp-${PULSAR_CPP_VERSION}-candidate-2/apache-pulsar-client-cpp-${PULSAR_CPP_VERSION}.tar.gz

Review Comment:
   Add TODO comment to switch to official releases URL



##########
pkg/mac/build-cpp-deps-lib.sh:
##########
@@ -0,0 +1,167 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+source $(dirname $0)/common.sh
+
+ZLIB_VERSION=1.2.13
+OPENSSL_VERSION=1_1_1q
+BOOST_VERSION=1.79.0
+PROTOBUF_VERSION=3.20.0
+ZSTD_VERSION=1.5.2
+SNAPPY_VERSION=1.1.3
+CURL_VERSION=7.61.0

Review Comment:
   we can try to use the same format as for cpp/python: https://github.com/apache/pulsar-client-cpp/blob/main/dependencies.yaml and sync the versions as of now.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] shibd commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
shibd commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1002324782


##########
pulsar-client-cpp-version.txt:
##########
@@ -0,0 +1 @@
+3.0.0

Review Comment:
   In pakcage.json -> version



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] merlimat commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1002540159


##########
.github/workflows/ci-build-release-napi.yml:
##########
@@ -0,0 +1,136 @@
+name: Build release napi
+on:
+  workflow_dispatch:
+  push:
+    tags:
+      - '*'
+env:
+  FORCE_COLOR: 1
+concurrency:
+  group: ${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+jobs:
+  macos-napi:
+    name: Build NAPI macos - Node ${{matrix.nodejs}} - ${{matrix.arch}}
+    runs-on: macos-latest
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        arch:
+          - x86_64
+#          - arm64
+        nodejs:
+          - 16
+          - 18
+    steps:
+      - uses: actions/checkout@v3
+      - name: Use Node.js ${{ matrix.nodejs }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.nodejs }}
+          cache: 'npm'
+
+      - name: Cache Dependencies
+        id: cache-dependencies
+        uses: actions/cache@v3
+        with:
+          path: pkg/mac/build
+          key: ${{ runner.os }}-${{ matrix.arch }}-mac-${{ hashFiles('pkg/mac/build-cpp-deps-lib.sh') }}
+
+      - name: Add env vars
+        shell: bash
+        run: |
+          if [ "${{ matrix.target }}" = "x86_64" ]; then

Review Comment:
   Variable name `matrix.target` does not exists so it will resolve to empty. 



##########
.github/workflows/ci-build-release-napi.yml:
##########
@@ -0,0 +1,136 @@
+name: Build release napi
+on:
+  workflow_dispatch:
+  push:
+    tags:
+      - '*'
+env:
+  FORCE_COLOR: 1
+concurrency:
+  group: ${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+jobs:
+  macos-napi:
+    name: Build NAPI macos - Node ${{matrix.nodejs}} - ${{matrix.arch}}
+    runs-on: macos-latest
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        arch:
+          - x86_64
+#          - arm64
+        nodejs:
+          - 16
+          - 18
+    steps:
+      - uses: actions/checkout@v3
+      - name: Use Node.js ${{ matrix.nodejs }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.nodejs }}
+          cache: 'npm'
+
+      - name: Cache Dependencies
+        id: cache-dependencies
+        uses: actions/cache@v3
+        with:
+          path: pkg/mac/build
+          key: ${{ runner.os }}-${{ matrix.arch }}-mac-${{ hashFiles('pkg/mac/build-cpp-deps-lib.sh') }}
+
+      - name: Add env vars
+        shell: bash
+        run: |
+          if [ "${{ matrix.target }}" = "x86_64" ]; then
+            echo "TARGET=x64" >> $GITHUB_ENV
+          else
+            echo "TARGET=${{ matrix.target }}" >> $GITHUB_ENV
+          fi
+
+      - name: Build CPP dependencies lib
+        if: steps.cache-dependencies.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-deps-lib.sh
+
+      - name: Build CPP lib
+        if: steps.cache-pulsar.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-lib.sh
+
+      - name: Build Node binaries lib
+        run: |
+          npm install --ignore-scripts
+          npx node-pre-gyp configure --target_arch=${{ env.TARGET }}

Review Comment:
   instead of using `evn.TARGET`, we can use `matrix.arch` directly here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] merlimat commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1000019013


##########
.github/workflows/ci-build-release-napi.yml:
##########
@@ -0,0 +1,130 @@
+name: Build release napi
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - master
+    tags:
+      - '*'
+env:
+  FORCE_COLOR: 1
+concurrency:
+  group: ${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+jobs:
+  macos-napi:
+    name: Build NAPI macos - Node ${{matrix.nodejs}} - ${{matrix.arch}}
+    runs-on: macos-latest
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        arch:
+          - x86_64
+        nodejs:
+          - 16
+          - 18
+    steps:
+      - uses: actions/checkout@v3
+      - name: Use Node.js ${{ matrix.nodejs }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.nodejs }}
+          cache: 'npm'
+
+      - name: Cache Dependencies
+        id: cache-dependencies
+        uses: actions/cache@v3
+        with:
+          path: pkg/mac/build
+          key: ${{ runner.os }}-${{ matrix.arch }}-mac-${{ hashFiles('pkg/mac/build-cpp-deps-lib.sh') }}
+
+      - name: Build CPP dependencies lib
+        if: steps.cache-dependencies.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-deps-lib.sh
+
+      - name: Build CPP lib
+        if: steps.cache-pulsar.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-lib.sh
+
+      - name: Build Node binaries lib
+        run: |
+          npm install --ignore-scripts
+          npx node-pre-gyp configure
+          npx node-pre-gyp build
+
+      - name: Test loading Node binaries lib
+        run: |
+          node pkg/load_test.js
+
+      - name: Package Node binaries lib
+        run: |
+          npx node-pre-gyp package
+
+      - name: Upload Node binaries lib to GitHub Release
+        env:
+          NODE_PRE_GYP_GITHUB_TOKEN: ${{ secrets.GITHUBTOKEN }}
+        if: matrix.nodejs == 16 && startsWith(github.ref, 'refs/tags/')
+        run: |
+          echo $NODE_PRE_GYP_GITHUB_TOKEN
+          npx node-pre-gyp-github publish
+
+  linux-napi:
+    name: Build NAPI ${{matrix.image}} - Node ${{matrix.nodejs}} - ${{matrix.cpu.platform}}
+    runs-on: ubuntu-22.04
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        image:
+          - 'linux_glibc'
+          - 'linux_musl'
+        nodejs:
+          - 16
+          - 18
+        cpu:
+          - {arch: 'x86_64', platform: 'x86_64'}
+          - {arch: 'aarch64', platform: 'arm64'}
+
+    steps:
+      - name: checkout
+        uses: actions/checkout@v3
+
+      - name: Set up QEMU
+        uses: docker/setup-qemu-action@v2
+
+      - name: Setup Docker Buildx
+        uses: docker/setup-buildx-action@v2
+
+      - name: Build Linux Docker image
+        uses: docker/build-push-action@v3
+        with:
+          context: ./pkg/${{matrix.image}}
+          load: true
+          tags: build:latest
+          platforms: linux/${{matrix.cpu.arch}}
+          build-args: |
+            PLATFORM=${{matrix.cpu.platform}}
+            ARCH=${{matrix.cpu.arch}}
+            NODE_VERSION=${{matrix.nodejs}}
+          cache-from: type=gha
+          cache-to: type=gha,mode=max
+
+      - name: Build and Test NAPI file
+        run: |
+          docker run -i -v $PWD:/pulsar-client-node build:latest \
+              /pulsar-client-node/pkg/build-napi-inside-docker.sh
+
+      - name: Upload Node binaries lib to GitHub Release

Review Comment:
   We could try to upload these as github actions artifacts. Then, we can automate to fetch them at release time and add them to the ASF dist Subversion, similar to what we do for C++/Python.  See: 
    * https://github.com/apache/pulsar-client-cpp/blob/main/.github/workflows/ci-build-binary-artifacts.yaml#L75 
    * https://github.com/apache/pulsar-client-cpp/blob/main/build-support/stage-release.sh
    * https://github.com/apache/pulsar-client-cpp/blob/main/build-support/download-release-artifacts.py
   
   Then, we can just point GYP to the HTTP URL from ASF download.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] BewareMyPower commented on pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#issuecomment-1278700978

   > Just for the unified
   
   There is no need to make two separate repositories consistent. What if the Python client repo renamed the directory or adjust the hierarchy later? **In this case, do you also want to sync the changes here?** People who are interested in Node.js client might not be interested in Python client. That's why do we separate C++/Python repos.
   
   From my perspective, if you have a huge PR, please remove unnecessary changes as much as possible.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] shibd commented on pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
shibd commented on PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#issuecomment-1278797419

   > From my perspective, if you have a huge PR, please remove unnecessary changes as much as possible.
   
   Make sense. I removed unnecessary changes. PTAL.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] shibd commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
shibd commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r1002324814


##########
src/pulsar-binding.js:
##########
@@ -0,0 +1,8 @@
+const path = require('path');
+const binary = require('@mapbox/node-pre-gyp');
+
+const bindingPath = binary.find(path.resolve(path.join(__dirname, '../package.json')));

Review Comment:
   I'm starting to package Windows and I'll test it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] shibd commented on a diff in pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
shibd commented on code in PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#discussion_r999681658


##########
.github/workflows/ci-build-release-napi.yml:
##########
@@ -0,0 +1,130 @@
+name: Build release napi
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - master
+    tags:
+      - '*'
+env:
+  FORCE_COLOR: 1
+concurrency:
+  group: ${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+jobs:
+  macos-napi:
+    name: Build NAPI macos - Node ${{matrix.nodejs}} - ${{matrix.arch}}
+    runs-on: macos-latest
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        arch:
+          - x86_64
+        nodejs:
+          - 16
+          - 18
+    steps:
+      - uses: actions/checkout@v3
+      - name: Use Node.js ${{ matrix.nodejs }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.nodejs }}
+          cache: 'npm'
+
+      - name: Cache Dependencies
+        id: cache-dependencies
+        uses: actions/cache@v3
+        with:
+          path: pkg/mac/build
+          key: ${{ runner.os }}-${{ matrix.arch }}-mac-${{ hashFiles('pkg/mac/build-cpp-deps-lib.sh') }}
+
+      - name: Build CPP dependencies lib
+        if: steps.cache-dependencies.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-deps-lib.sh
+
+      - name: Build CPP lib
+        if: steps.cache-pulsar.outputs.cache-hit != 'true'
+        run: |
+          export ARCH=${{ matrix.arch }}
+          pkg/mac/build-cpp-lib.sh
+
+      - name: Build Node binaries lib
+        run: |
+          npm install --ignore-scripts
+          npx node-pre-gyp configure
+          npx node-pre-gyp build
+
+      - name: Test loading Node binaries lib
+        run: |
+          node pkg/load_test.js
+
+      - name: Package Node binaries lib
+        run: |
+          npx node-pre-gyp package
+
+      - name: Upload Node binaries lib to GitHub Release
+        env:
+          NODE_PRE_GYP_GITHUB_TOKEN: ${{ secrets.GITHUBTOKEN }}
+        if: matrix.nodejs == 16 && startsWith(github.ref, 'refs/tags/')
+        run: |
+          echo $NODE_PRE_GYP_GITHUB_TOKEN
+          npx node-pre-gyp-github publish
+
+  linux-napi:
+    name: Build NAPI ${{matrix.image}} - Node ${{matrix.nodejs}} - ${{matrix.cpu.platform}}
+    runs-on: ubuntu-22.04
+    timeout-minutes: 3000
+
+    strategy:
+      fail-fast: false
+      matrix:
+        image:
+          - 'linux_glibc'
+          - 'linux_musl'
+        nodejs:
+          - 16
+          - 18
+        cpu:
+          - {arch: 'x86_64', platform: 'x86_64'}
+          - {arch: 'aarch64', platform: 'arm64'}
+
+    steps:
+      - name: checkout
+        uses: actions/checkout@v3
+
+      - name: Set up QEMU
+        uses: docker/setup-qemu-action@v2
+
+      - name: Setup Docker Buildx
+        uses: docker/setup-buildx-action@v2
+
+      - name: Build Linux Docker image
+        uses: docker/build-push-action@v3
+        with:
+          context: ./pkg/${{matrix.image}}
+          load: true
+          tags: build:latest
+          platforms: linux/${{matrix.cpu.arch}}
+          build-args: |
+            PLATFORM=${{matrix.cpu.platform}}
+            ARCH=${{matrix.cpu.arch}}
+            NODE_VERSION=${{matrix.nodejs}}
+          cache-from: type=gha
+          cache-to: type=gha,mode=max
+
+      - name: Build and Test NAPI file
+        run: |
+          docker run -i -v $PWD:/pulsar-client-node build:latest \
+              /pulsar-client-node/pkg/build-napi-inside-docker.sh
+
+      - name: Upload Node binaries lib to GitHub Release

Review Comment:
   Need to add a secrets to this repo.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-client-node] shibd commented on pull request #235: [feat] Napi support package c++ static binary.

Posted by GitBox <gi...@apache.org>.
shibd commented on PR #235:
URL: https://github.com/apache/pulsar-client-node/pull/235#issuecomment-1287636247

   > We can try to find a way to cross-compile. Right now there are no M1 runners in GH actions.
   Let this not be a blocker.
   
   Hi, @merlimat I can cross-compile `libpulsarwithdeps.a`, but I can't cross-compile `napi`. I tried using the command '--target_arch=arm64', but it doesn't work. You can refer to failed job: https://github.com/shibd/pulsar-client-node/actions/runs/3294556450/jobs/5432190252 
   
   I raised an issue [here](https://github.com/mapbox/node-pre-gyp/issues/669) and haven't waited for a response yet.
   
   I'll finish compiling Windows first and then continue working on it here.
   
   If you have a good idea, please recommend it to me. Thanks.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org