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

fix: restore externalized Node.js dep compatibility #3421

Merged
merged 2 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
102 changes: 102 additions & 0 deletions .github/workflows/nodejs-shared.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
name: Node.js compiled --shared-builtin-undici/undici-path CI

on:
push:
branches:
- main
- current
- next
- 'v*'
pull_request:

permissions:
contents: read

jobs:
test-shared-builtin:
name: Test with Node.js ${{ matrix.version }} compiled --shared-builtin-undici/undici-path
strategy:
fail-fast: false
max-parallel: 0
matrix:
version: [20, 22]
runs-on: ubuntu-latest
timeout-minutes: 120
steps:
# Checkout into a subdirectory otherwise Node.js tests will break due to finding Undici's package.json in a parent directory.
- name: Checkout
uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
with:
path: ./undici
persist-credentials: false

# Setup node, install deps, and build undici prior to building node with `--shared-builtin-undici/undici-path` and testing
- name: Setup Node.js@${{ inputs.version }}
uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
with:
node-version: ${{ inputs.version }}

- name: Install dependencies
working-directory: ./undici
run: npm install

- name: Install wasi-libc
run: sudo apt-get install -y wasi-libc

- name: Build WASM
working-directory: ./undici
run: |
export EXTERNAL_PATH=${{ github.workspace }}/undici
export WASM_CC=clang
export WASM_CFLAGS='--target=wasm32-wasi --sysroot=/usr'
export WASM_LDFLAGS='-nodefaultlibs'
export WASM_LDLIBS='-lc'
node build/wasm.js

- name: Determine latest release
id: release
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
with:
result-encoding: string
script: |
const req = await fetch('https://nodejs.org/download/release/index.json')
const releases = await req.json()

const latest = releases.find((r) => r.version.startsWith('v${{ matrix.version }}'))
return latest.version

- name: Download and extract source
run: curl https://nodejs.org/download/release/${{ steps.release.outputs.result }}/node-${{ steps.release.outputs.result }}.tar.xz | tar xfJ -

- name: Install ninja
run: sudo apt-get install ninja-build

- name: ccache
uses: hendrikmuhs/ccache-action@c92f40bee50034e84c763e33b317c77adaa81c92 #v1.2.13
with:
key: node(external_undici)${{ matrix.version }}

- name: Build node
working-directory: ./node-${{ steps.release.outputs.result }}
run: |
export CC="ccache gcc"
export CXX="ccache g++"
rm -rf deps/undici
./configure --shared-builtin-undici/undici-path ${{ github.workspace }}/undici/loader.js --ninja --prefix=./final
make
make install
echo "$(pwd)/final/bin" >> $GITHUB_PATH

- name: Print version information
run: |
echo OS: $(node -p "os.version()")
echo Node.js: $(node --version)
echo npm: $(npm --version)
echo git: $(git --version)
echo external config: $(node -e "console.log(process.config)" | grep NODE_SHARED_BUILTIN_UNDICI_UNDICI_PATH)
echo Node.js built-in undici version: $(node -p "process.versions.undici") # undefined for external Undici

- name: Run tests
working-directory: ./node-${{ steps.release.outputs.result }}
run: tools/test.py -p dots --flaky-tests=dontcare

3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ If you are packaging `undici` for a distro, this might help if you would like to
an unbundled version instead of bundling one in `libnode.so`.

To enable this, pass `EXTERNAL_PATH=/path/to/global/node_modules/undici` to `build/wasm.js`.
You shall also pass this path to `--shared-builtin-undici/undici-path` in Node.js's `configure.py`.
Pass this path with `loader.js` appended to `--shared-builtin-undici/undici-path` in Node.js's `configure.py`.
If building on a non-Alpine Linux distribution, you may need to also set the `WASM_CC`, `WASM_CFLAGS`, `WASM_LDFLAGS` and `WASM_LDLIBS` environment variables before running `build/wasm.js`.
Copy link
Member Author

Choose a reason for hiding this comment

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

The exact value of what to set the environment variables to differ by Linux distribution.

See the workflow file for the Ubuntu 22.04 settings, and https://src.fedoraproject.org/rpms/nodejs-undici/blob/rawhide/f/nodejs-undici.spec for Fedora.


<a id="benchmarks"></a>
### Benchmarks
Expand Down
48 changes: 34 additions & 14 deletions build/wasm.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
let WASM_LDFLAGS = process.env.WASM_LDFLAGS || ''
const WASM_LDLIBS = process.env.WASM_LDLIBS || ''

// For compatibility with Node.js' `configure --shared-builtin-undici/undici-path ...`
const EXTERNAL_PATH = process.env.EXTERNAL_PATH

// These are relevant for undici and should not be overridden
WASM_CFLAGS += ' -Ofast -fno-exceptions -fvisibility=hidden -mexec-model=reactor'
WASM_LDFLAGS += ' -Wl,-error-limit=0 -Wl,-O3 -Wl,--lto-O3 -Wl,--strip-all'
Expand Down Expand Up @@ -73,6 +76,9 @@
const hasApk = (function () {
try { execSync('command -v apk'); return true } catch (error) { return false }
})()
const hasOptimizer = (function () {
try { execSync('./wasm-opt --version'); return true } catch (error) { return false }
})()
if (hasApk) {
// Gather information about the tools used for the build
const buildInfo = execSync('apk info -v').toString()
Expand All @@ -81,24 +87,38 @@
process.exit(-1)
}
console.log(buildInfo)
}

// Build wasm binary
execSync(`${WASM_CC} ${WASM_CFLAGS} ${WASM_LDFLAGS} \
${join(WASM_SRC, 'src')}/*.c \
-I${join(WASM_SRC, 'include')} \
-o ${join(WASM_OUT, 'llhttp.wasm')} \
${WASM_LDLIBS}`, { stdio: 'inherit' })
// Build wasm binary
execSync(`${WASM_CC} ${WASM_CFLAGS} ${WASM_LDFLAGS} \
${join(WASM_SRC, 'src')}/*.c \
-I${join(WASM_SRC, 'include')} \
-o ${join(WASM_OUT, 'llhttp.wasm')} \
${WASM_LDLIBS}`, { stdio: 'inherit' })
github-advanced-security[bot] marked this conversation as resolved.
Dismissed
Show resolved Hide resolved

if (hasOptimizer) {
execSync(`./wasm-opt ${WASM_OPT_FLAGS} -o ${join(WASM_OUT, 'llhttp.wasm')} ${join(WASM_OUT, 'llhttp.wasm')}`, { stdio: 'inherit' })
writeWasmChunk('llhttp.wasm', 'llhttp-wasm.js')
}
writeWasmChunk('llhttp.wasm', 'llhttp-wasm.js')

// Build wasm simd binary
execSync(`${WASM_CC} ${WASM_CFLAGS} -msimd128 ${WASM_LDFLAGS} \
${join(WASM_SRC, 'src')}/*.c \
-I${join(WASM_SRC, 'include')} \
-o ${join(WASM_OUT, 'llhttp_simd.wasm')} \
${WASM_LDLIBS}`, { stdio: 'inherit' })
// Build wasm simd binary
execSync(`${WASM_CC} ${WASM_CFLAGS} -msimd128 ${WASM_LDFLAGS} \
${join(WASM_SRC, 'src')}/*.c \
-I${join(WASM_SRC, 'include')} \
-o ${join(WASM_OUT, 'llhttp_simd.wasm')} \
${WASM_LDLIBS}`, { stdio: 'inherit' })
github-advanced-security[bot] marked this conversation as resolved.
Dismissed
Show resolved Hide resolved

if (hasOptimizer) {
execSync(`./wasm-opt ${WASM_OPT_FLAGS} --enable-simd -o ${join(WASM_OUT, 'llhttp_simd.wasm')} ${join(WASM_OUT, 'llhttp_simd.wasm')}`, { stdio: 'inherit' })
writeWasmChunk('llhttp_simd.wasm', 'llhttp_simd-wasm.js')
}
writeWasmChunk('llhttp_simd.wasm', 'llhttp_simd-wasm.js')

// For compatibility with Node.js' `configure --shared-builtin-undici/undici-path ...`
if (EXTERNAL_PATH) {
writeFileSync(join(ROOT, 'loader.js'), `
'use strict'
globalThis.__UNDICI_IS_NODE__ = true
module.exports = require('node:module').createRequire('${EXTERNAL_PATH}/loader.js')('./index-fetch.js')
delete globalThis.__UNDICI_IS_NODE__
`)
}
Loading