diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b6ae41b8..973f7476 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,7 +1,12 @@ name: ci on: - - pull_request - - push + push: + branches: [master] + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true jobs: Test: @@ -13,26 +18,26 @@ jobs: os: - ubuntu-latest - macos-latest - - windows-latest + - windows-2022 node_version: - - 16 - 18 - 20 + - 22 name: Node ${{ matrix.node_version }} on ${{ matrix.os }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: submodules: true - name: Cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: | node_modules key: ${{ runner.os }}-${{ matrix.node_version }}-${{ hashFiles('package.json') }} - name: Setup node - uses: actions/setup-node@v4 + uses: actions/setup-node@v5 with: node-version: ${{ matrix.node_version }} diff --git a/binding.gyp b/binding.gyp index 23943893..f4b8fd80 100644 --- a/binding.gyp +++ b/binding.gyp @@ -28,27 +28,24 @@ "conditions": [ ['OS=="mac"', { "postbuilds": [ + { + 'postbuild_name': 'Copy vendored libiconv next to the binding', + 'action': [ + 'bash', + '<(module_root_dir)/script/copy-libiconv.sh', + '<(module_root_dir)/ext/lib/libiconv.2.dylib', + '<(PRODUCT_DIR)/libiconv.2.dylib' + ] + }, { 'postbuild_name': 'Adjust vendored libiconv install name', 'action': [ 'install_name_tool', "-change", "libiconv.2.dylib", - "@loader_path/../../ext/lib/libiconv.2.dylib", + "@loader_path/libiconv.2.dylib", "<(PRODUCT_DIR)/superstring.node" ] - - # NOTE: This version of the post-build action - # should be used if we find it necessary to avoid - # changing the `dylib`’s install name in an earlier - # step. - # - # 'action': [ - # 'bash', - # '<(module_root_dir)/script/adjust-install-name.sh', - # '<(PRODUCT_DIR)' - # ] - } ] }] @@ -120,7 +117,7 @@ "action_name": "Run script", "message": "Building GNU libiconv...", "inputs": [], - "outputs": ["ext"], + "outputs": ["<(module_root_dir)/ext/lib/libiconv.2.dylib"], "action": [ "bash", "script/fetch-libiconv-61.sh" @@ -128,22 +125,6 @@ } ] } - # { - # "target_name": "find_libiconv", - # "target_type": "none", - # "actions": [ - # { - # "action_name": "Run script", - # "message": "Locating GNU libiconv...", - # "inputs": [], - # "outputs": ["vendor/libiconv/lib/libiconv.2.dylib"], - # "action": [ - # "bash", - # "script/find-gnu-libiconv.sh" - # ] - # } - # ] - # } ] }], @@ -194,28 +175,26 @@ 'MACOSX_DEPLOYMENT_TARGET': '10.12', }, "postbuilds": [ + { + 'postbuild_name': 'Copy vendored libiconv next to the binding', + 'action': [ + 'bash', + '<(module_root_dir)/script/copy-libiconv.sh', + '<(module_root_dir)/ext/lib/libiconv.2.dylib', + '<(PRODUCT_DIR)/libiconv.2.dylib' + ] + }, { 'postbuild_name': 'Adjust vendored libiconv install name', 'action': [ - 'install_name_tool', - "-change", - "libiconv.2.dylib", - "@executable_path/../../ext/lib/libiconv.2.dylib", - "<(PRODUCT_DIR)/tests" + 'install_name_tool', + "-change", + "libiconv.2.dylib", + "@executable_path/libiconv.2.dylib", + "<(PRODUCT_DIR)/tests" ] - - # NOTE: This version of the post-build action - # should be used if we find it necessary to avoid - # changing the `dylib`’s install name in an earlier - # step. - # - # 'action': [ - # 'bash', - # '<(module_root_dir)/script/adjust-install-name.sh', - # '<(PRODUCT_DIR)' - # ] } - ] + ] }] ] }] diff --git a/script/copy-libiconv.sh b/script/copy-libiconv.sh new file mode 100755 index 00000000..19deeb9e --- /dev/null +++ b/script/copy-libiconv.sh @@ -0,0 +1,24 @@ +#!/bin/bash +set -euo pipefail + +# Copy the vendored libiconv next to the built product. Resolves the symlink +# (`ext/lib/libiconv.2.dylib` points at the versioned dylib) and installs the +# copy atomically, so two targets postbuilding in parallel can't observe a +# half-written file. + +src="$1" +dest="$2" + +tmp="$(mktemp "${dest}.XXXXXX")" +trap 'rm -f "$tmp"' EXIT + +# `-L` because `ext/lib/libiconv.2.dylib` is a symlink to the versioned dylib. +# We want the real file here — a link would still point outside of `build/`, +# which is what we're getting away from. +cp -L "$src" "$tmp" + +# We _must_ get the permissions right here; a `.dylib` with 0600 would work for +# the user who built it but fail for anyone else. +chmod 755 "$tmp" + +mv -f "$tmp" "$dest" diff --git a/script/fetch-libiconv-61.sh b/script/fetch-libiconv-61.sh index 74c98904..6cee8b5c 100644 --- a/script/fetch-libiconv-61.sh +++ b/script/fetch-libiconv-61.sh @@ -1,4 +1,5 @@ #!/bin/bash +set -euo pipefail # When compiling `superstring` on macOS, we used to be able to rely on the # builtin version of `libiconv`. But newer versions of macOS include FreeBSD @@ -12,16 +13,16 @@ # `libiconv.2.dylib`. For now, letting the user compile their own `libiconv` # has the advantage of very likely matching the system's architecture. -echoerr() { echo "$@\n" >&2; } +echoerr() { printf '%s\n\n' "$*" >&2; } create-if-missing() { - if [ -z "$1" ]; then + if [ -f "$1" ]; then echoerr "Error: $1 is a file." usage exit 1 fi if [ ! -d "$1" ]; then - mkdir "$1" + mkdir -p "$1" fi } @@ -50,9 +51,9 @@ dylib_path="$EXT/lib/libiconv.2.dylib" # If this path already exists, we'll assume libiconv has already been fetched # and compiled. Otherwise we'll do it now. -if [ ! -L "$dylib_path" ]; then +if [ ! -e "$dylib_path" ]; then echo "Path $dylib_path is missing; fetching and installing libiconv." - cd $SCRATCH + cd "$SCRATCH" # TODO: Instead of downloading this each time, we can check this into source # control via git subtree. That would allow someone to build this without # needing internet connectivity. But we'd still need to do a `make install` — @@ -64,7 +65,7 @@ if [ ! -L "$dylib_path" ]; then make make install - if [ ! -L "$dylib_path" ]; then + if [ ! -e "$dylib_path" ]; then echoerr "Error: expected $dylib_path to be present, but it was not. Installation of libiconv failed. Cannot proceed." usage exit 1 @@ -81,10 +82,9 @@ else echo "Path $dylib_path is already present; skipping installation of libiconv." fi -cd $ROOT +cd "$ROOT" -# We expect this path to exist and be a symbolic link that points to a file. -if [ ! -L "$dylib_path" ]; then +if [ ! -e "$dylib_path" ]; then echoerr "Error: expected $dylib_path to be present, but it was not. Cannot proceed." usage exit 1