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.

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

switch everything to node and dont use O_NONBLOCK #25

Merged
merged 44 commits into from
Jun 8, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
7f35457
switch everything to node
jackyzha0 Jun 5, 2024
cb1ef5f
oops forgot packagelock
jackyzha0 Jun 5, 2024
0904a82
3.0.0
jackyzha0 Jun 5, 2024
605b315
test fixes
jackyzha0 Jun 5, 2024
e5413e7
only
jackyzha0 Jun 5, 2024
fb3e5da
use nix instead of rustix, convert to blocking mode
jackyzha0 Jun 6, 2024
cf94ce8
try to fix ci?
jackyzha0 Jun 6, 2024
1e5762c
strip w -x
jackyzha0 Jun 6, 2024
0f9c8c6
language agnostic build
jackyzha0 Jun 6, 2024
d49fdb0
change recommended start strat
jackyzha0 Jun 6, 2024
998c8c3
update package json, try not stripping
jackyzha0 Jun 6, 2024
b4286f2
remove concurrent sad
jackyzha0 Jun 6, 2024
28270d1
bad assert
jackyzha0 Jun 6, 2024
fa4d05d
try specifying target again
jackyzha0 Jun 6, 2024
3391137
dont ci
jackyzha0 Jun 6, 2024
c47260a
fix linux tests
jackyzha0 Jun 6, 2024
ced5a0e
try not using docker??
jackyzha0 Jun 6, 2024
a3df0bb
combine build and test
jackyzha0 Jun 6, 2024
9cd45d2
simplify even more
jackyzha0 Jun 6, 2024
7adb3cd
fix upload path
jackyzha0 Jun 6, 2024
9243065
try not repeating?
jackyzha0 Jun 6, 2024
859f26c
dont repeat, abstract rejectOnNonEIO
jackyzha0 Jun 6, 2024
add0b3c
?????
jackyzha0 Jun 6, 2024
8edfc51
only run publish on main
jackyzha0 Jun 6, 2024
728fb16
try 50 again?
jackyzha0 Jun 6, 2024
82d0861
lower repeat?? idk
jackyzha0 Jun 6, 2024
26e702e
add before/aftereach
jackyzha0 Jun 6, 2024
6720477
try without cloning
jackyzha0 Jun 6, 2024
e91e2c8
cleanup/teardown improvement
jackyzha0 Jun 6, 2024
ff9016b
now try bumping retry
jackyzha0 Jun 6, 2024
6c16e58
50???? john pls
jackyzha0 Jun 6, 2024
42f3552
drop controller fd
jackyzha0 Jun 6, 2024
23b6009
use unsafe close
jackyzha0 Jun 6, 2024
627c699
wait for the right things...
jackyzha0 Jun 6, 2024
38e7a0b
bump retries
jackyzha0 Jun 6, 2024
b1d19d2
add poll
jackyzha0 Jun 7, 2024
f2255b0
use end instead of close
jackyzha0 Jun 7, 2024
546eae7
wrapper time
jackyzha0 Jun 7, 2024
3d82fdf
update docs, remove extra ports from .replit
jackyzha0 Jun 7, 2024
2365f06
run 1000??
jackyzha0 Jun 7, 2024
25b6d59
stty is bad
jackyzha0 Jun 7, 2024
4f0fd09
add ordering test
jackyzha0 Jun 7, 2024
9895696
some test cleanup
jackyzha0 Jun 8, 2024
99fdc02
remove only modifier
jackyzha0 Jun 8, 2024
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
128 changes: 15 additions & 113 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,42 +22,27 @@ permissions:
pull_request: null

jobs:
build:
build-test:
strategy:
fail-fast: false
matrix:
settings:
- host: macos-14-large
target: x86_64-apple-darwin
architecture: x64
build: |-
set -e &&
bun run build --target x86_64-apple-darwin &&
strip -x *.node
- host: macos-latest
target: aarch64-apple-darwin
- host: ubuntu-latest
target: x86_64-unknown-linux-gnu
docker: ghcr.io/napi-rs/napi-rs/nodejs-rust:lts-debian
build: |-
set -e &&
npm install -g bun &&
bun run build --target x86_64-unknown-linux-gnu &&
strip *.node
name: Build on ${{ matrix.settings.target }}
name: Build and test on ${{ matrix.settings.target }}
runs-on: ${{ matrix.settings.host }}
steps:
- uses: actions/checkout@v4
- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: 1.1.6
- name: Setup node
uses: actions/setup-node@v4
if: ${{ !matrix.settings.docker }}
with:
node-version: 20
- name: Install
uses: dtolnay/rust-toolchain@stable
if: ${{ !matrix.settings.docker }}
with:
toolchain: stable
targets: ${{ matrix.settings.target }}
Expand All @@ -71,126 +56,43 @@ jobs:
.cargo-cache
target/
key: ${{ matrix.settings.target }}-cargo-${{ matrix.settings.host }}
- uses: goto-bus-stop/setup-zig@v2
if: ${{ matrix.settings.target == 'armv7-unknown-linux-gnueabihf' }}
with:
version: 0.11.0
- name: Setup toolchain
run: ${{ matrix.settings.setup }}
if: ${{ matrix.settings.setup }}
shell: bash
- name: Install dependencies
run: bun install
- name: Build in docker
uses: addnab/docker-run-action@v3
if: ${{ matrix.settings.docker }}
with:
image: ${{ matrix.settings.docker }}
options: '--user 0:0 -v ${{ github.workspace }}/.cargo-cache/git/db:/usr/local/cargo/git/db -v ${{ github.workspace }}/.cargo/registry/cache:/usr/local/cargo/registry/cache -v ${{ github.workspace }}/.cargo/registry/index:/usr/local/cargo/registry/index -v ${{ github.workspace }}:/build -w /build'
run: ${{ matrix.settings.build }}
run: npm ci
- name: Build
run: ${{ matrix.settings.build }}
if: ${{ !matrix.settings.docker }}
run: |-
set -e &&
npm run build &&
strip -x *.node
shell: bash
- name: Test bindings
run: npm run test
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: bindings-${{ matrix.settings.target }}
path: ${{ env.APP_NAME }}.*.node
if-no-files-found: error

test-macos-binding:
name: Test on ${{ matrix.settings.target }}
needs:
- build
strategy:
matrix:
settings:
- host: macos-14-large
target: x86_64-apple-darwin
architecture: x64
runs-on: ${{ matrix.settings.host }}
steps:
- uses: actions/checkout@v4
- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: 1.1.6
- name: Setup node
uses: actions/setup-node@v4
with:
node-version: 20
architecture: ${{ matrix.settings.architecture }}
- name: Install dependencies
run: bun install
- name: Download artifacts
uses: actions/download-artifact@v4
with:
name: bindings-${{ matrix.settings.target }}
path: .
- name: List packages
run: ls -R .
shell: bash
- name: Test bindings
run: npm run test:darwin

test-linux-binding:
name: Test on ${{ matrix.settings.target }}
needs:
- build
strategy:
matrix:
settings:
- host: ubuntu-latest
target: x86_64-unknown-linux-gnu
runs-on: ${{ matrix.settings.host }}
steps:
- uses: actions/checkout@v4
- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: 1.1.6
- name: Setup node
uses: actions/setup-node@v4
with:
node-version: 20
- name: Install dependencies
run: bun install
- name: Download artifacts
uses: actions/download-artifact@v4
with:
name: bindings-${{ matrix.settings.target }}
path: .
- name: List packages
run: ls -R .
shell: bash
- name: Test bindings
run: docker run --rm -v $(pwd):/build -w /build oven/bun:1.1.6 bun test:linux

publish:
name: Publish
runs-on: ubuntu-latest
if: github.ref == 'refs/heads/main'
needs:
- test-macos-binding
- test-linux-binding
- build-test
steps:
- uses: actions/checkout@v4
- name: Setup Bun
uses: oven-sh/setup-bun@v1
with:
bun-version: 1.1.6
- name: Setup node
uses: actions/setup-node@v4
with:
node-version: 20
- name: Install dependencies
run: bun install
run: npm ci
- name: Download all artifacts
uses: actions/download-artifact@v4
with:
path: artifacts
- name: Move artifacts
run: bun run artifacts
run: npm run artifacts
- name: List packages
run: ls -R ./npm
shell: bash
Expand Down
8 changes: 6 additions & 2 deletions .replit
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
run = "bun run build:debug && bun test"
modules = ["bun-1.0:v17-20240117-0bd73cd", "nodejs-20:v24-20240117-0bd73cd", "rust-stable:v4-20240117-0bd73cd"]
run = "npm run build && npm test"
modules = ["rust-stable", "nodejs-20", "nix"]

disableGuessImports = true
disableInstallBeforeRun = true
Expand All @@ -15,3 +15,7 @@ channel = "stable-23_11"

[rules.formatter.fileExtensions.".ts"]
id = "module:nodejs-20:v24-20240117-0bd73cd/formatter:prettier"

[[ports]]
localPort = 24678
externalPort = 80
jackyzha0 marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ libc = "0.2.152"
# Default enable napi4 feature, see https://nodejs.org/api/n-api.html#node-api-version-matrix
napi = { version = "2.12.2", default-features = false, features = ["napi4"] }
napi-derive = "2.12.2"
rustix = { version = "0.38.30", features = ["event"] }
rustix-openpty = "0.1.1"
nix = { version = "0.29.0", features = ["fs", "term"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

image


[build-dependencies]
napi-build = "2.0.1"
Expand Down
13 changes: 5 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
# `@replit/ruspty` - PTY for Bun (and Node) through Rust FFI
# `@replit/ruspty` - PTY for JavaScript through Rust FFI

Running:

- `bun install`
- `bun run build` / `bun run build:debug`
- `bun test:linux` on Linux
- `npm run test:darwin` on macOS
- `npm install`
- `npm run build` / `npm run build:debug`
- `npm run test`

The code mainly targets Bun on Linux.
The code mainly targets Node on Linux.
jackyzha0 marked this conversation as resolved.
Show resolved Hide resolved

The biggest difference from existing PTY libraries is that this one works with Bun, and doesn't cross the FFI bridge for every input/output instead requiring the consumer to deal with the `fd` of the PTY.

**WARNING**: as of 2024-05-06 there's a [bug in Bun](https://github.com/oven-sh/bun/issues/9907) which prevents us from using `fd` with Bun, and a temporary workaround with `onData` handler was introduced in `v2.0.1`. Check out Linux tests for usage.

## Publishing

Following ["Publish It" section from `napi-rs` docs](https://napi.rs/docs/introduction/simple-package#publish-it):
Expand Down
Binary file removed bun.lockb
Binary file not shown.
8 changes: 3 additions & 5 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export interface PtyOptions {
dir?: string
size?: Size
onExit: (err: null | Error, exitCode: number) => void
onData?: (err: null | Error, data: Buffer) => void
}
/** A size struct to pass to resize. */
export interface Size {
Expand All @@ -26,7 +25,7 @@ export interface Size {
* This is the recommended usage:
*
* ```
* const { Pty } = require('@replit/ruspy');
* const { Pty } = require('@replit/ruspty');
* const fs = require('fs');
*
* const pty = new Pty({
Expand All @@ -43,13 +42,12 @@ export interface Size {
*
* const read = new fs.createReadStream('', {
* fd: pty.fd(),
* start: 0,
* highWaterMark: 16 * 1024,
* autoClose: true,
* autoClose: false,
jackyzha0 marked this conversation as resolved.
Show resolved Hide resolved
* });
* const write = new fs.createWriteStream('', {
* fd: pty.fd(),
* autoClose: true,
* autoClose: false,
* });
*
* read.on('data', (chunk) => {
Expand Down
3 changes: 3 additions & 0 deletions npm/darwin-arm64/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# `@replit/ruspty-darwin-arm64`

This is the **aarch64-apple-darwin** binary for `@replit/ruspty`
18 changes: 18 additions & 0 deletions npm/darwin-arm64/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "@replit/ruspty-darwin-arm64",
"version": "3.0.0",
"os": [
"darwin"
],
"cpu": [
"arm64"
],
"main": "ruspty.darwin-arm64.node",
"files": [
"ruspty.darwin-arm64.node"
],
"license": "MIT",
"engines": {
"node": ">= 10"
}
}
2 changes: 1 addition & 1 deletion npm/darwin-x64/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@replit/ruspty-darwin-x64",
"version": "2.2.0",
"version": "3.0.0",
"os": [
"darwin"
],
Expand Down
2 changes: 1 addition & 1 deletion npm/linux-x64-gnu/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@replit/ruspty-linux-x64-gnu",
"version": "2.2.0",
"version": "3.0.0",
"os": [
"linux"
],
Expand Down
Loading