Skip to content

Commit

Permalink
fix: restore externalized Node.js dep compatibility (#3421)
Browse files Browse the repository at this point in the history
* fix: restore externalized Node.js dep compatibility

Restore the ability to build Undici compatible with Node.js'
`configure --shared-builtin-undici/undici-path ...` build option.

Scopes the `hasApk` conditional to only cover the part that requires
`apk`.

Makes the WASM optimizer (binaryen) optional to allow building on
Linux distributions that do not package `binaryen` and must be able
to rebuild everything (including tooling) from source.

* ci: add workflow for externalized Node.js dep

Add a workflow to test building Undici in a way compatible with Node.js
built with `configure --shared-builtin-undici/undici-path ...`.

This configuration is used by downstream Node.js packagers (e.g. Fedora)
who require the ability to be able to build everything from source.
  • Loading branch information
richardlau authored and mcollina committed Jul 31, 2024
1 parent 62241c3 commit 93605ab
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 15 deletions.
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 @@ -157,7 +157,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`.

<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_CFLAGS = process.env.WASM_CFLAGS || '--sysroot=/usr/share/wasi-sysroot
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 @@ if (process.argv[2] === '--docker') {
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 @@ if (hasApk) {
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' })

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' })

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__
`)
}

0 comments on commit 93605ab

Please sign in to comment.