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

(Test) Fix install name _and_ encoding name conversion issue #12

Closed
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ node_modules
build
.DS_Store
.clang_complete
ext

/browser.js
emsdk-portable
package-lock.json

vendor/libiconv
4 changes: 4 additions & 0 deletions .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
75 changes: 73 additions & 2 deletions binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,40 @@
"src/core",
"<!(node -e \"require('nan')\")"
],
"conditions": [
['OS=="mac"', {
"postbuilds": [
{
'postbuild_name': 'Adjust vendored libiconv install name',
'action': [
'install_name_tool',
"-change",
"libiconv.2.dylib",
"@loader_path/../../ext/lib/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)'
# ]

}
]
}]
]
},
{
"target_name": "superstring_core",
"type": "static_library",
"dependencies": [
"./vendor/pcre/pcre.gyp:pcre",
"./vendor/pcre/pcre.gyp:pcre"
],
"sources": [
"src/core/encoding-conversion.cc",
Expand All @@ -46,8 +74,14 @@
],
"conditions": [
['OS=="mac"', {
'dependencies': [
'build_libiconv'
],
'include_dirs': [
'<(module_root_dir)/ext/include'
],
'link_settings': {
'libraries': ['libiconv.dylib'],
'libraries': ['<(module_root_dir)/ext/lib/libiconv.2.dylib']
}
}],
['OS=="win"', {
Expand All @@ -71,6 +105,43 @@
},

"conditions": [
['OS=="mac"', {
'targets+': [
{
"target_name": "build_libiconv",
"target_type": "none",
"actions": [
{
"action_name": "Run script",
"message": "Building GNU libiconv...",
"inputs": [],
"outputs": ["ext"],
"action": [
"bash",
"script/fetch-libiconv-61.sh"
]
}
]
}
# {
# "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"
# ]
# }
# ]
# }
]
}],

# If --tests is passed to node-gyp configure, we'll build a standalone
# executable that runs tests on the patch.
['tests != 0', {
Expand Down
2 changes: 2 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ if (process.env.SUPERSTRING_USE_BROWSER_VERSION) {
}

TextBuffer.prototype.baseTextMatchesFile = function (source, encoding = 'UTF8') {
encoding = normalizeEncoding(encoding)

return new Promise((resolve, reject) => {
const callback = (error, result) => {
if (error) {
Expand Down
23 changes: 23 additions & 0 deletions script/adjust-install-name.sh
Original file line number Diff line number Diff line change
@@ -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"
86 changes: 86 additions & 0 deletions script/fetch-libiconv-61.sh
Original file line number Diff line number Diff line change
@@ -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
90 changes: 90 additions & 0 deletions script/find-gnu-libiconv.sh
Original file line number Diff line number Diff line change
@@ -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}"
Loading