diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2b69b39..cb1ec2fb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,9 +21,13 @@ jobs: exclude: - os: windows-latest node_version: 14 + - os: macos-latest + node_version: 14 include: - os: windows-2019 node_version: 14 + - os: macos-13 + node_version: 14 name: Node ${{ matrix.node_version }} on ${{ matrix.os }} steps: @@ -52,14 +56,37 @@ jobs: with: python-version: '3.10' - - name: Install Python setuptools + - name: Install Python setuptools (Unix-likes) # This is needed for Python 3.12+, since many versions of node-gyp # are incompatible with Python 3.12+, which no-longer ships 'distutils' # out of the box. 'setuptools' package provides 'distutils'. - run: python3 -m pip install setuptools + if: ${{ runner.os != 'Windows' }} + run: | + python3 -m venv CI_venv + source CI_venv/bin/activate + python3 -m pip install setuptools - - name: Install dependencies - run: npm install + - name: Install Python setuptools (Windows) + # This is needed for Python 3.12+, since many versions of node-gyp + # are incompatible with Python 3.12+, which no-longer ships 'distutils' + # out of the box. 'setuptools' package provides 'distutils'. + if: ${{ runner.os == 'Windows' }} + run: | + python3 -m venv CI_venv + CI_venv\Scripts\activate.bat + python3 -m pip install setuptools + + - name: Install dependencies (Unix-likes) + if: ${{ runner.os != 'Windows' }} + run: | + source CI_venv/bin/activate + npm install + + - name: Install dependencies (Windows) + if: ${{ runner.os == 'Windows' }} + run: | + CI_venv\Scripts\activate.bat + npm install - name: Lint run: npm run standard diff --git a/.gitignore b/.gitignore index 51f142db..5592bbdf 100644 --- a/.gitignore +++ b/.gitignore @@ -2,7 +2,10 @@ node_modules build .DS_Store .clang_complete +ext /browser.js emsdk-portable package-lock.json + +vendor/libiconv diff --git a/.npmignore b/.npmignore index 97d77444..b57d7b9a 100644 --- a/.npmignore +++ b/.npmignore @@ -8,6 +8,8 @@ !src/bindings/*.h !src/bindings/*.cc +!script/fetch-libiconv-61.sh + !vendor/libcxx/* !vendor/pcre/pcre.gyp diff --git a/README.md b/README.md index c39c6ea5..a22948f8 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,10 @@ Native library at the core of Atom's text editor. +## Installation notes: + +On macOS 13 and greater, the OS no longer offers GNU `libiconv`. We handle this by downloading it from Apple’s OSS GitHub page and building it as a pre-compilation step. + ## Components: ### Patch diff --git a/binding.gyp b/binding.gyp index 249e9e82..32c9c751 100644 --- a/binding.gyp +++ b/binding.gyp @@ -21,12 +21,40 @@ "src/core", " { const callback = (error, result) => { if (error) { diff --git a/script/fetch-libiconv-61.sh b/script/fetch-libiconv-61.sh new file mode 100644 index 00000000..50bc244f --- /dev/null +++ b/script/fetch-libiconv-61.sh @@ -0,0 +1,105 @@ +#!/bin/bash + +# 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 +# `libiconv`, rather than GNU `libiconv`; the two are not API-compatible. +# +# For this reason, we download a known good version of `libiconv` from +# https://github.com/apple-oss-distributions/libiconv/tree/libiconv-61. +# +# We might eventually replace this approach with an explicit vendorization of +# the specific files needed, but that would require a universal build of +# `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; } + +create-if-missing() { + if [ -z "$1" ]; then + echoerr "Error: $1 is a file." + usage + exit 1 + fi + if [ ! -d "$1" ]; then + mkdir "$1" + fi +} + +usage() { + echoerr "superstring requires the GNU libiconv library, which macOS no longer bundles in recent versions. This package attempts to compile it from GitHub. If you're seeing this message, something has gone wrong; check the README for information and consider filing an issue." +} + +# Identify the directory of this script. +SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) + +ROOT="$SCRIPT_DIR/.." +SCRATCH="$ROOT/scratch" +EXT="$ROOT/ext" + +cleanup() { + if [ -d "$SCRATCH" ]; then + rm -rf "$SCRATCH" + fi +} +trap cleanup SIGINT EXIT + +create-if-missing "$EXT" +create-if-missing "$SCRATCH" + +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 + echo "Path $dylib_path is missing; fetching and installing libiconv." + 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` — + # at least until we can produce a "universal" version of the `.dylib` and put + # _that_ in source control. + git clone -b libiconv-61 "https://github.com/apple-oss-distributions/libiconv.git" + cd libiconv/libiconv + ./configure --prefix="$EXT" --libdir="$EXT/lib" + make + make install + + if [ ! -L "$dylib_path" ]; then + echoerr "Error: expected $dylib_path to be present, but it was not. Installation of libiconv failed. Cannot proceed." + usage + exit 1 + fi + + # Remove the directories we don't need. + rm -rf "$EXT/bin" + rm -rf "$EXT/share" + + # Copy over the license and README from the scratch directory. + cp "COPYING.LIB" "$EXT" + cp "README" "$EXT" +else + echo "Path $dylib_path is already present; skipping installation of libiconv." +fi + +cd $ROOT + +# We expect this path to exist and be a symbolic link that points to a file. +if [ ! -L "$dylib_path" ]; then + echoerr "Error: expected $dylib_path to be present, but it was not. Cannot proceed." + usage + exit 1 +fi + +# Set the install name of this library to something neutral and predictable to +# make a later step easier. +# +# NOTE: macOS complains about this action invalidating the library's code +# signature. This has not been observed to have any negative effects for +# Pulsar, possibly because we sign and notarize the entire app at a later stage +# of the build process. But if it _did_ have negative effects, we could switch +# to a different approach and skip this step. See the `binding.gyp` file for +# further details. + +install_name_tool -id "libiconv.2.dylib" "${dylib_path}" + +cleanup diff --git a/src/bindings/text-buffer-wrapper.cc b/src/bindings/text-buffer-wrapper.cc index 3c633269..52e7bded 100644 --- a/src/bindings/text-buffer-wrapper.cc +++ b/src/bindings/text-buffer-wrapper.cc @@ -387,11 +387,7 @@ static Local encode_ranges(const vector &ranges) { auto length = ranges.size() * 4; auto buffer = v8::ArrayBuffer::New(v8::Isolate::GetCurrent(), length * sizeof(uint32_t)); auto result = v8::Uint32Array::New(buffer, 0, length); - #if (V8_MAJOR_VERSION < 8) - auto data = buffer->GetContents().Data(); - #else - auto data = buffer->GetBackingStore()->Data(); - #endif + auto data = buffer->GetContents().Data(); memcpy(data, ranges.data(), length * sizeof(uint32_t)); return result; } @@ -615,11 +611,7 @@ void TextBufferWrapper::find_words_with_subsequence_in_range(const Nan::Function } auto positions_buffer = v8::ArrayBuffer::New(v8::Isolate::GetCurrent(), positions_buffer_size); - #if (V8_MAJOR_VERSION < 8) - uint32_t *positions_data = reinterpret_cast(positions_buffer->GetContents().Data()); - #else - uint32_t *positions_data = reinterpret_cast(positions_buffer->GetBackingStore()->Data()); - #endif + uint32_t *positions_data = reinterpret_cast(positions_buffer->GetContents().Data()); uint32_t positions_array_index = 0; for (size_t i = 0; i < result.size() && i < max_count; i++) { @@ -943,11 +935,7 @@ void TextBufferWrapper::load(const Nan::FunctionCallbackInfo &info) { if (!force && text_buffer.is_modified()) { Local argv[] = {Nan::Null(), Nan::Null()}; auto callback = info[0].As(); - #if (V8_MAJOR_VERSION > 9 || (V8_MAJOR_VERSION == 9 && V8_MINOR_VERION > 4)) - Nan::Call(callback, callback->GetCreationContext().ToLocalChecked()->Global(), 2, argv); - #else - Nan::Call(callback, callback->CreationContext()->Global(), 2, argv); - #endif + Nan::Call(callback, callback->CreationContext()->Global(), 2, argv); return; } @@ -1051,12 +1039,7 @@ void TextBufferWrapper::base_text_matches_file(const Nan::FunctionCallbackInfo argv[] = {Nan::Null(), Nan::New(result)}; auto callback = info[0].As(); - - #if (V8_MAJOR_VERSION > 9 || (V8_MAJOR_VERSION == 9 && V8_MINOR_VERION > 4)) - Nan::Call(callback, callback->GetCreationContext().ToLocalChecked()->Global(), 2, argv); - #else - Nan::Call(callback, callback->CreationContext()->Global(), 2, argv); - #endif + Nan::Call(callback, callback->CreationContext()->Global(), 2, argv); } }