From d5c9006cf013e46df743ba814597ad564e53b20c Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Wed, 3 Jul 2024 14:39:10 -0500 Subject: [PATCH 1/8] Amassed changes --- .gitignore | 3 ++ .npmignore | 4 ++ README.md | 4 ++ binding.gyp | 75 ++++++++++++++++++++++++++++- index.js | 2 + script/adjust-install-name.sh | 23 +++++++++ script/fetch-libiconv-61.sh | 86 +++++++++++++++++++++++++++++++++ script/find-gnu-libiconv.sh | 90 +++++++++++++++++++++++++++++++++++ 8 files changed, 285 insertions(+), 2 deletions(-) create mode 100755 script/adjust-install-name.sh create mode 100644 script/fetch-libiconv-61.sh create mode 100755 script/find-gnu-libiconv.sh 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..edb606e7 100644 --- a/.npmignore +++ b/.npmignore @@ -8,6 +8,10 @@ !src/bindings/*.h !src/bindings/*.cc +!script/fetch-libiconv-61.sh +!script/find-gnu-libiconv.sh +!script/adjust-install-name.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/adjust-install-name.sh b/script/adjust-install-name.sh new file mode 100755 index 00000000..e68226c7 --- /dev/null +++ b/script/adjust-install-name.sh @@ -0,0 +1,23 @@ +#!/bin/bash + +# This script can be used if we find it necessary, for code signing reasons, +# not to alter the install name of `libiconv.2.dylib` during compilation. macOS +# complains when we do it, saying that the code signature has been invalidated, +# but we haven't noticed any ill effects… yet. +# +# But this script would allow us to point `superstring.node` at the correct +# library by figuring out `libicov.2.dylib`’s existing install name, rather +# than setting it to a known value in an earlier step. + +product_dir=$1 + +# Ask for the current install name expected by `superstring.node`. We need to +# know this in order to change it in the next step. +current_install_name=$(otool -L "$product_dir/superstring.node" | awk 'BEGIN{FS=OFS=" "};NR==2{print $1}') + +# Now use `install_name_tool` to tell `superstring.node` to instead look for +# `libiconv.2.dylib` at a path relative to itself. +install_name_tool -change \ + "$current_install_name" \ + "@loader_path/../../vendor/libiconv/lib/libiconv.2.dylib" \ + "$product_dir/superstring.node" diff --git a/script/fetch-libiconv-61.sh b/script/fetch-libiconv-61.sh new file mode 100644 index 00000000..2385bc3d --- /dev/null +++ b/script/fetch-libiconv-61.sh @@ -0,0 +1,86 @@ + +#!/bin/bash + +# The purpose of this script is to find a copy of GNU `libiconv` on this macOS +# machine. Since newer versions of macOS include a FreeBSD `libiconv`, we no +# longer assume it's safe to use any ambient `libiconv.dylib` we find. +# +# For this reason, we try to detect a Homebrew installation of `libiconv`; we +# also allow the user to install GNU `libiconv` manually and specify the path +# via an environment variable. +# +# 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 provide their `libiconv` has +# the advantage of very likely matching the system's architecture. + +echoerr() { echo "$@\n" >&2; } + + +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() { + if [ -z "$1" ]; then + echoerr "Error: $1 is a file." + usage + exit 1 + fi + if [ ! -d "$1" ]; then + mkdir "$1" + fi +} + +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 + cd $SCRATCH + 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 +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/script/find-gnu-libiconv.sh b/script/find-gnu-libiconv.sh new file mode 100755 index 00000000..4565db5d --- /dev/null +++ b/script/find-gnu-libiconv.sh @@ -0,0 +1,90 @@ +#!/bin/bash + +# The purpose of this script is to find a copy of GNU `libiconv` on this macOS +# machine. Since newer versions of macOS include a FreeBSD `libiconv`, we no +# longer assume it's safe to use any ambient `libiconv.dylib` we find. +# +# For this reason, we try to detect a Homebrew installation of `libiconv`; we +# also allow the user to install GNU `libiconv` manually and specify the path +# via an environment variable. +# +# 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 provide their `libiconv` has +# the advantage of very likely matching the system's architecture. + +echoerr() { echo "$@\n" >&2; } + +usage() { + echoerr "superstring requires the GNU libiconv library. You can install it with Homebrew (\`brew install libiconv\`) and we'll be able to detect its presence. You may also define a SUPERSTRING_LIBICONV_PATH variable set to the absolute path of your libiconv installation. (This path should have \`lib\` and \`include\` as child directories.)" +} + +# Identify the directory of this script. +SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) + +# Find this package's `vendor` directory; make sure it exists. +VENDOR="$SCRIPT_DIR/../vendor" +if [ ! -d "$VENDOR" ]; then + echoerr "Aborting; expected $VENDOR to be a directory, but it was not." + exit 1 +fi + +TARGET="$VENDOR/libiconv" + +# Make a `libiconv` directory for us to vendorize into. +if [ ! -d "$TARGET" ]; then + mkdir "$TARGET" +fi + +if [[ ! -z "${SUPERSTRING_LIBICONV_PATH}" ]]; then + # First, we allow the user to specify a path and override our heuristics. + # This should propagate even if the user ran `yarn install` from a project + # that has `superstring` as a dependency. + source="${SUPERSTRING_LIBICONV_PATH}" +elif command -v brew &> /dev/null; then + # If that variable isn't set, then we check if this machine has Homebrew + # installed. If so, we'll opt into Homebrew's version of `libiconv`. This is + # the safest option because we can reasonably conclude that this `libiconv` + # is the right flavor and matches the system's architecture. + source="$(brew --prefix)/opt/libiconv" +else + # If neither of these things is true, we won't try to add an entry to + # `library_dirs`. + usage + exit 1 +fi + +if [ ! -d "$source" ]; then + echoerr "Expected $source to be the path to GNU libiconv, but it is not a directory. " + usage + exit 1 +fi + +# We expect the `dylib` we need to be at this exact path. +dylib_path="${source}/lib/libiconv.2.dylib" + +if [ ! -f "$dylib_path" ]; then + echoerr "Invalid location for libiconv. Expected to find: ${dylib_path} but it was not present." + usage + exit 1 +fi + +# We need the `include` directory for compilation, plus the `libiconv.2.dylib` +# file. We'll also copy over the README and license files for compliance. +cp -R "${source}/include" "$TARGET/" +cp "${dylib_path}" "$TARGET/lib/" +cp "${source}/COPYING.LIB" "$TARGET/" +cp "${source}/README" "$TARGET/" + + +# 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}" From 26a41ec90a6ad84526e3a83885ce4c8b9ef081cb Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Fri, 5 Jul 2024 14:22:06 -0500 Subject: [PATCH 2/8] Delete stuff we don't need from `ext` --- script/fetch-libiconv-61.sh | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/script/fetch-libiconv-61.sh b/script/fetch-libiconv-61.sh index 2385bc3d..b9a6bce5 100644 --- a/script/fetch-libiconv-61.sh +++ b/script/fetch-libiconv-61.sh @@ -5,9 +5,8 @@ # machine. Since newer versions of macOS include a FreeBSD `libiconv`, we no # longer assume it's safe to use any ambient `libiconv.dylib` we find. # -# For this reason, we try to detect a Homebrew installation of `libiconv`; we -# also allow the user to install GNU `libiconv` manually and specify the path -# via an environment variable. +# 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 @@ -54,12 +53,25 @@ 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 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. Cannot proceed." + usage + exit 1 + else + # Remove the directories we don't need. + rm -rf "$EXT/bin" + rm -rf "$EXT/share" + fi +else + echo "Path $dylib_path is already present; skipping installation of libiconv." fi cd $ROOT From 266230e442e858154b2723232293739875d28466 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Fri, 5 Jul 2024 19:59:34 -0500 Subject: [PATCH 3/8] =?UTF-8?q?Clean=20up=20scripts=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove scripts we don't need. * Report progress better on fetching/compiling `libiconv`. * Preserve the license and README from `libiconv`. --- .npmignore | 2 - script/adjust-install-name.sh | 23 --------- script/fetch-libiconv-61.sh | 51 +++++++++++--------- script/find-gnu-libiconv.sh | 90 ----------------------------------- 4 files changed, 29 insertions(+), 137 deletions(-) delete mode 100755 script/adjust-install-name.sh delete mode 100755 script/find-gnu-libiconv.sh diff --git a/.npmignore b/.npmignore index edb606e7..b57d7b9a 100644 --- a/.npmignore +++ b/.npmignore @@ -9,8 +9,6 @@ !src/bindings/*.cc !script/fetch-libiconv-61.sh -!script/find-gnu-libiconv.sh -!script/adjust-install-name.sh !vendor/libcxx/* diff --git a/script/adjust-install-name.sh b/script/adjust-install-name.sh deleted file mode 100755 index e68226c7..00000000 --- a/script/adjust-install-name.sh +++ /dev/null @@ -1,23 +0,0 @@ -#!/bin/bash - -# This script can be used if we find it necessary, for code signing reasons, -# not to alter the install name of `libiconv.2.dylib` during compilation. macOS -# complains when we do it, saying that the code signature has been invalidated, -# but we haven't noticed any ill effects… yet. -# -# But this script would allow us to point `superstring.node` at the correct -# library by figuring out `libicov.2.dylib`’s existing install name, rather -# than setting it to a known value in an earlier step. - -product_dir=$1 - -# Ask for the current install name expected by `superstring.node`. We need to -# know this in order to change it in the next step. -current_install_name=$(otool -L "$product_dir/superstring.node" | awk 'BEGIN{FS=OFS=" "};NR==2{print $1}') - -# Now use `install_name_tool` to tell `superstring.node` to instead look for -# `libiconv.2.dylib` at a path relative to itself. -install_name_tool -change \ - "$current_install_name" \ - "@loader_path/../../vendor/libiconv/lib/libiconv.2.dylib" \ - "$product_dir/superstring.node" diff --git a/script/fetch-libiconv-61.sh b/script/fetch-libiconv-61.sh index b9a6bce5..50bc244f 100644 --- a/script/fetch-libiconv-61.sh +++ b/script/fetch-libiconv-61.sh @@ -1,20 +1,29 @@ - #!/bin/bash -# The purpose of this script is to find a copy of GNU `libiconv` on this macOS -# machine. Since newer versions of macOS include a FreeBSD `libiconv`, we no -# longer assume it's safe to use any ambient `libiconv.dylib` we find. +# 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 provide their `libiconv` has -# the advantage of very likely matching the system's architecture. +# `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." @@ -34,17 +43,6 @@ cleanup() { } trap cleanup SIGINT EXIT -create-if-missing() { - if [ -z "$1" ]; then - echoerr "Error: $1 is a file." - usage - exit 1 - fi - if [ ! -d "$1" ]; then - mkdir "$1" - fi -} - create-if-missing "$EXT" create-if-missing "$SCRATCH" @@ -55,6 +53,11 @@ dylib_path="$EXT/lib/libiconv.2.dylib" 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" @@ -62,14 +65,18 @@ if [ ! -L "$dylib_path" ]; then make install if [ ! -L "$dylib_path" ]; then - echoerr "Error: expected $dylib_path to be present, but it was not. Cannot proceed." + echoerr "Error: expected $dylib_path to be present, but it was not. Installation of libiconv failed. Cannot proceed." usage exit 1 - else - # Remove the directories we don't need. - rm -rf "$EXT/bin" - rm -rf "$EXT/share" 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 diff --git a/script/find-gnu-libiconv.sh b/script/find-gnu-libiconv.sh deleted file mode 100755 index 4565db5d..00000000 --- a/script/find-gnu-libiconv.sh +++ /dev/null @@ -1,90 +0,0 @@ -#!/bin/bash - -# The purpose of this script is to find a copy of GNU `libiconv` on this macOS -# machine. Since newer versions of macOS include a FreeBSD `libiconv`, we no -# longer assume it's safe to use any ambient `libiconv.dylib` we find. -# -# For this reason, we try to detect a Homebrew installation of `libiconv`; we -# also allow the user to install GNU `libiconv` manually and specify the path -# via an environment variable. -# -# 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 provide their `libiconv` has -# the advantage of very likely matching the system's architecture. - -echoerr() { echo "$@\n" >&2; } - -usage() { - echoerr "superstring requires the GNU libiconv library. You can install it with Homebrew (\`brew install libiconv\`) and we'll be able to detect its presence. You may also define a SUPERSTRING_LIBICONV_PATH variable set to the absolute path of your libiconv installation. (This path should have \`lib\` and \`include\` as child directories.)" -} - -# Identify the directory of this script. -SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) - -# Find this package's `vendor` directory; make sure it exists. -VENDOR="$SCRIPT_DIR/../vendor" -if [ ! -d "$VENDOR" ]; then - echoerr "Aborting; expected $VENDOR to be a directory, but it was not." - exit 1 -fi - -TARGET="$VENDOR/libiconv" - -# Make a `libiconv` directory for us to vendorize into. -if [ ! -d "$TARGET" ]; then - mkdir "$TARGET" -fi - -if [[ ! -z "${SUPERSTRING_LIBICONV_PATH}" ]]; then - # First, we allow the user to specify a path and override our heuristics. - # This should propagate even if the user ran `yarn install` from a project - # that has `superstring` as a dependency. - source="${SUPERSTRING_LIBICONV_PATH}" -elif command -v brew &> /dev/null; then - # If that variable isn't set, then we check if this machine has Homebrew - # installed. If so, we'll opt into Homebrew's version of `libiconv`. This is - # the safest option because we can reasonably conclude that this `libiconv` - # is the right flavor and matches the system's architecture. - source="$(brew --prefix)/opt/libiconv" -else - # If neither of these things is true, we won't try to add an entry to - # `library_dirs`. - usage - exit 1 -fi - -if [ ! -d "$source" ]; then - echoerr "Expected $source to be the path to GNU libiconv, but it is not a directory. " - usage - exit 1 -fi - -# We expect the `dylib` we need to be at this exact path. -dylib_path="${source}/lib/libiconv.2.dylib" - -if [ ! -f "$dylib_path" ]; then - echoerr "Invalid location for libiconv. Expected to find: ${dylib_path} but it was not present." - usage - exit 1 -fi - -# We need the `include` directory for compilation, plus the `libiconv.2.dylib` -# file. We'll also copy over the README and license files for compliance. -cp -R "${source}/include" "$TARGET/" -cp "${dylib_path}" "$TARGET/lib/" -cp "${source}/COPYING.LIB" "$TARGET/" -cp "${source}/README" "$TARGET/" - - -# 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}" From e09c09652eb0b04a10917c0f0e91979bf095aa2f Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Fri, 5 Jul 2024 20:09:28 -0500 Subject: [PATCH 4/8] Revert code changes from #1 --- src/bindings/text-buffer-wrapper.cc | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) 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); } } From 8b9511725e06458dfbc500f28031e584ba00de5d Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Mon, 27 May 2024 21:56:02 -0400 Subject: [PATCH 5/8] CI: Use a venv to install setuptools The latest macOS GitHub Actions CI runners ship with a Python that is configured as "This environment is externally managed". This is a somewhat recent thing in the Python world, see: - https://packaging.python.org/en/latest/specifications/externally-managed-environments/ - https://peps.python.org/pep-0668/ We can try using virtual environments ("venvs") to manage manual package installs as the error message suggests. --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2b69b39..ef04d0f8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,7 +56,10 @@ jobs: # 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 + run: | + python3 -m venv CI_venv + source CI_venv/bin/activate + python3 -m pip install setuptools - name: Install dependencies run: npm install From fe84f963d27b45d38ec7bf1e23d5f80fe915c37e Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Mon, 27 May 2024 22:03:10 -0400 Subject: [PATCH 6/8] CI: Activate python venv before running npm install We need setuptools for node-gyp to be able to do its thing --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef04d0f8..f100ff27 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,7 +62,9 @@ jobs: python3 -m pip install setuptools - name: Install dependencies - run: npm install + run: | + source CI_venv/bin/activate + npm install - name: Lint run: npm run standard From 5d1f2afae2f79ad3e5d805d0e226a31e4c5358f6 Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Wed, 3 Jul 2024 16:43:18 -0400 Subject: [PATCH 7/8] CI: Test Node 14 with macos-13 image macOS 14 (`macos-latest`, `macos-14`) runners are implicitly Apple Silicon runners, at least for runners we have access to on the free tier. Node 14 isn't built for Apple Silicon. So, we can test Node 14 on older macOS, like... macOS 13. That will get us an x64 macOS runner, an arch for which Node 14 should indeed be available. --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f100ff27..65429fd1 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: From 49bf7309d4a5b37281e007c8dba94908fbc93025 Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Wed, 3 Jul 2024 17:02:14 -0400 Subject: [PATCH 8/8] Fix venv usage on Windows --- .github/workflows/ci.yml | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65429fd1..cb1ec2fb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,20 +56,38 @@ 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'. + if: ${{ runner.os != 'Windows' }} run: | python3 -m venv CI_venv source CI_venv/bin/activate python3 -m pip install setuptools - - name: Install dependencies + - 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