Merge pull request #656 from splitgraph/bugfix/osx-pyinstaller-fixes

Add new artifact `sgr-osx-x86_64.tgz` to releases, compiled with `pyinstaller --onedir`, and default to it in `install.sh` on Darwin

Author: milesrichardson

.ci/build_wheel.sh

@@ -5,20 +5,36 @@ DEFAULT_PYPI_URL="https://test.pypi.org/legacy/"
 CI_DIR=$(cd -P -- "$(dirname -- "$0")" && pwd -P)
 REPO_ROOT_DIR="${CI_DIR}/.."
 
-test -z "$PYPI_PASSWORD" && { echo "Fatal Error: No PYPI_PASSWORD set" ; exit 1 ; }
+# By default, will configure PyPi for publishing.
+# To skip publishing setup, set NO_PUBLISH=1 .ci/build_wheel.sh
+NO_PUBLISH_FLAG="${NO_PUBLISH}"
+
+test -n "$NO_PUBLISH_FLAG" && { echo "Skipping publish because \$NO_PUBLISH is set" ; }
+test -z "$PYPI_PASSWORD" && \
+    ! test -n "$NO_PUBLISH_FLAG" \
+    && { echo "Fatal Error: No PYPI_PASSWORD set. To skip, set NO_PUBLISH=1" ; exit 1 ; }
 test -z "$PYPI_URL" && { echo "No PYPI_URL set. Defaulting to ${DEFAULT_PYPI_URL}" ; }
 
 PYPI_URL=${PYPI_URL-"${DEFAULT_PYPI_URL}"}
 
 source "$HOME"/.poetry/env
 
 # Configure pypi for deployment
-pushd "$REPO_ROOT_DIR" \
-    && poetry config repositories.testpypi "$PYPI_URL" \
-    && poetry config http-basic.testpypi splitgraph "$PYPI_PASSWORD" \
-    && poetry config http-basic.pypi splitgraph "$PYPI_PASSWORD" \
-    && poetry build \
-    && popd \
-    && exit 0
-
-exit 1
+pushd "$REPO_ROOT_DIR"
+
+set -e
+if ! test -n "$NO_PUBLISH_FLAG" ; then
+    echo "Configuring poetry with password from \$PYPI_PASSWORD"
+    echo "To skip, try: NO_PUBLISH=1 $0 $*"
+    poetry config http-basic.testpypi splitgraph "$PYPI_PASSWORD"
+    poetry config http-basic.pypi splitgraph "$PYPI_PASSWORD"
+fi
+
+# Set the PyPi URL because it can't hurt (we skipped setting the credentials)
+poetry config repositories.testpypi "$PYPI_URL"
+
+poetry build
+popd
+
+set +e
+exit 0

.github/workflows/build_and_test_and_release.yml

@@ -8,8 +8,8 @@ jobs:
     runs-on: ubuntu-18.04
     if: "!contains(github.event.head_commit.message, '[skip ci]')"
     env:
-      COMPOSE_VERSION: '1.25.4'
-      POETRY_VERSION: '1.1.6'
+      COMPOSE_VERSION: "1.25.4"
+      POETRY_VERSION: "1.1.6"
       DOCKER_REPO: splitgraph
       DOCKER_ENGINE_IMAGE: engine
       DOCKER_TAG: development
@@ -23,7 +23,7 @@ jobs:
       - name: Setup Python 3.8
         uses: actions/setup-python@v2
         with:
-          python-version: '3.8'
+          python-version: "3.8"
       - uses: actions/cache@v1
         with:
           path: ~/.cache/pip
@@ -113,6 +113,15 @@ jobs:
           # TODO figure out if we want to do poetry upload here (can only do once, so will fail
           # if we're retrying an upload)
           # "$HOME"/.poetry/bin/poetry build
+      - name: "Build wheel only (do not configure publish)"
+        # The {windows,linux,osx}_binary stage will run if ref is tag, or msg contains "[artifacts]""
+        # If no tag, but [artifacts], we still need to build the wheel, but with NO_PUBLISH=1
+        # But if tag _and_ [artifacts], we want to skip this stage, to not build the wheel twice
+        if: "!startsWith(github.ref, 'refs/tags/') && contains(github.event.head_commit.message, '[artifacts]')"
+        env:
+          NO_PUBLISH: "1"
+        run: |
+          ./.ci/build_wheel.sh
       - name: "Upload release artifacts"
         uses: actions/upload-artifact@v2
         with:
@@ -121,7 +130,7 @@ jobs:
 
   windows_binary:
     runs-on: windows-latest
-    if: "startsWith(github.ref, 'refs/tags/')"
+    if: "startsWith(github.ref, 'refs/tags/') || contains(github.event.head_commit.message, '[artifacts]')"
     needs: build_and_test
     steps:
       - uses: actions/checkout@v1
@@ -150,7 +159,7 @@ jobs:
 
   linux_binary:
     runs-on: ubuntu-18.04
-    if: "startsWith(github.ref, 'refs/tags/')"
+    if: "startsWith(github.ref, 'refs/tags/') || contains(github.event.head_commit.message, '[artifacts]')"
     needs: build_and_test
     steps:
       - uses: actions/checkout@v1
@@ -188,7 +197,7 @@ jobs:
 
   osx_binary:
     runs-on: macOS-latest
-    if: "startsWith(github.ref, 'refs/tags/')"
+    if: "startsWith(github.ref, 'refs/tags/') || contains(github.event.head_commit.message, '[artifacts]')"
     needs: build_and_test
     steps:
       - uses: actions/checkout@v1
@@ -201,18 +210,27 @@ jobs:
         with:
           name: dist
           path: dist
-      - name: Build the binary
+      - name: Build the single-file binary
         run: |
           pip install dist/splitgraph-*-py3-none-any.whl
           pip install pyinstaller
           pyinstaller -F splitgraph.spec
           dist/sgr --version
-      - name: Upload binary as artifact
+      - name: Upload single-file binary as artifact
         uses: actions/upload-artifact@v2
         with:
           name: sgr-osx
           path: dist/sgr
-
+      - name: Build the multi-file binary.gz
+        run: |
+          pyinstaller --clean --noconfirm --onedir splitgraph.spec
+          dist/sgr-pkg/sgr --version
+          cd dist/sgr-pkg && tar zcvf ../sgr.tgz .
+      - name: Upload multi-file binary.gz as artifact
+        uses: actions/upload-artifact@v2
+        with:
+          name: sgr-osx.tgz
+          path: dist/sgr.tgz
 
   upload_release:
     runs-on: ubuntu-18.04
@@ -231,13 +249,15 @@ jobs:
           mv artifacts/sgr-windows/sgr.exe artifacts/sgr-windows-x86_64.exe
           mv artifacts/sgr-linux/sgr artifacts/sgr-linux-x86_64
           mv artifacts/sgr-osx/sgr artifacts/sgr-osx-x86_64
+          mv artifacts/sgr-osx/sgr.tgz artifacts/sgr-osx-x86_64.tgz
       - name: Release artifacts
         uses: softprops/action-gh-release@v1
         with:
           files: |
             artifacts/sgr-windows-x86_64.exe
             artifacts/sgr-linux-x86_64
             artifacts/sgr-osx-x86_64
+            artifacts/sgr-osx-x86_64.tgz
             artifacts/dist/sgr-docs-bin.tar.gz
             artifacts/dist/install.sh
           draft: true

install.sh

@@ -62,7 +62,15 @@ _get_binary_name() {
   if [ "$os" == Linux ]; then
     BINARY="sgr-linux-x86_64"
   elif [ "$os" == Darwin ]; then
-    BINARY="sgr-osx-x86_64"
+    if [ -n "$FORCE_ONEFILE" ] ; then
+      echo "Forcing --onefile installation on OS X because \$FORCE_ONEFILE is set."
+      BINARY="sgr-osx-x86_64"
+    else
+      # OS X has bad single-file executable support (pyinstaller --onefile), so we default to --onedir variant
+      echo "Installing optimized package for OS X (built with pyinstaller --onedir instead of --onefile)"
+      echo "To force install single-file executable (not recommended), set FORCE_ONEFILE=1"
+      BINARY="sgr-osx-x86_64.tgz"
+    fi
   else
     _die "This installation method only supported on Linux/OSX. Please see https://www.splitgraph.com/docs/installation/ for other installation methods."
   fi
@@ -77,14 +85,34 @@ _install_binary () {
   _check_sgr_exists
 
   URL="https://github.com/splitgraph/splitgraph/releases/download/v${SGR_VERSION}"/$BINARY
-  echo "Installing the sgr binary from $URL into $INSTALL_DIR"
-  mkdir -p "$INSTALL_DIR"
-  curl -fsL "$URL" > "$INSTALL_DIR/sgr"
-  chmod +x "$INSTALL_DIR/sgr"
-  "$INSTALL_DIR/sgr" --version
-  echo "sgr binary installed."
-  echo
+  # on OS X, splitgraph.spec is called with --onedir to output .tgz of exe and shlibs
+  if [ "$BINARY" == "sgr-osx-x86_64.tgz" ] ; then
+    echo "Installing the compressed sgr binary and deps from $URL into $INSTALL_DIR"
+    echo "Installing sgr binary and deps into $INSTALL_DIR/pkg"
+
+    if [ -d "$INSTALL_DIR/pkg/sgr" ] ; then
+      echo "Removing existing $INSTALL_DIR/pkg/sgr"
+      rm -rf "$INSTALL_DIR/pkg/sgr"
+    fi
+
+    mkdir -p "$INSTALL_DIR/pkg/sgr"
+
+    curl -fsL "$URL" > "$INSTALL_DIR/pkg/sgr/sgr.tgz"
+
+    echo "Extract sgr binary and deps into $INSTALL_DIR/pkg/sgr (necessary on MacOS)"
+    (cd "$INSTALL_DIR"/pkg/sgr && tar xfz sgr.tgz && rm sgr.tgz)
+    echo "Main sgr binary is at $INSTALL_DIR/pkg/sgr/sgr"
+    echo "Link $INSTALL_DIR/sgr -> $INSTALL_DIR/pkg/sgr/sgr"
+    ln -fs "$INSTALL_DIR"/pkg/sgr/sgr "$INSTALL_DIR"/sgr
+  else
+    echo "Installing the sgr binary from $URL into $INSTALL_DIR"
+    mkdir -p "$INSTALL_DIR"
+    curl -fsL "$URL" > "$INSTALL_DIR/sgr"
+    chmod +x "$INSTALL_DIR/sgr"
+  fi
 
+  "$INSTALL_DIR/sgr" --version && echo "sgr binary installed." && echo && return 0
+  _die "Installation apparently failed. got non-zero exit code from: $INSTALL_DIR/sgr --version"
 }
 
 _setup_engine() {

splitgraph.spec

@@ -3,10 +3,23 @@
 # * LD_LIBRARY_PATH=`echo $(python3-config --prefix)/lib` pyinstaller -F splitgraph.spec produces a single sgr binary in the dist/ folder
 #   with libc being the only dynamic dependency (python interpreter included)
 # * can also do poetry install && poetry run pyinstaller -F splitgraph.spec to build the binary inside of the poetry's venv.
+# * specifying `--onedir` instead of `-F` will compile a multi-file executable with COLLECT()
+# * e.g. : pyinstaller --clean --noconfirm --onedir splitgraph.spec
 
+import sys
 import os
 import importlib
 
+# Pass --onedir or -D to build a multi-file executable (dir with shlibs + exe)
+# Note: This is the same flag syntax as pyinstaller uses, but when using a
+#       .spec file with pyinstaller, the flag is normally ignored, so we
+#       explicitly check for it here. This way we can pass different arguments
+#       to EXE() and conditionally call COLLECTION() without duplicating code.
+MAKE_EXE_COLLECTION = False
+if "--onedir" in sys.argv or "-D" in sys.argv:
+    print("splitgraph.spec : --onedir was specified. Will build a multi-file executable...")
+    MAKE_EXE_COLLECTION = True
+
 block_cipher = None
 
 datas = []
@@ -43,18 +56,36 @@ a = Analysis(
 a.datas += Tree("./splitgraph/resources", "splitgraph/resources")
 
 pyz = PYZ(a.pure, a.zipped_data, cipher=block_cipher)
-exe = EXE(
-    pyz,
-    a.scripts,
-    a.binaries,
-    a.zipfiles,
-    a.datas,
-    [],
-    name="sgr",
-    debug=False,
-    bootloader_ignore_signals=False,
-    strip=False,
-    upx=True,
-    runtime_tmpdir=None,
-    console=True,
-)
+
+# Note: `exe` global is injected by pyinstaller. Can see the code for EXE at:
+# https://github.com/pyinstaller/pyinstaller/blob/15f23a8a89be5453b3520df8fc3e667346e103a6/PyInstaller/building/api.py
+
+# TOCS to include in EXE() when in single-file mode (default, i.e. no --onedir flag)
+all_tocs = [a.scripts, a.binaries, a.zipfiles, a.datas]
+# TOCS to include in EXE() when in multi-file mode (i.e., --onedir flag)
+exe_tocs = [a.scripts]
+# TOCS to include in COLL() when in multi-file mode (i.e., --onedir flag)
+coll_tocs = [a.binaries, a.zipfiles, a.datas]
+
+# When compiling single-file executable, we include every TOC in the EXE
+# When compiling multi-file executable, include some TOC in EXE, and rest in COLL
+exe_args = [pyz, *exe_tocs, []] if MAKE_EXE_COLLECTION else [pyz, *all_tocs, []]
+
+exe_kwargs_base = {
+    "name": "sgr",
+    "debug": False,
+    "bootloader_ignore_signals": False,
+    "strip_binaries": False,
+    "runtime_tmpdir": None,
+    "console": True,
+}
+# In multi-file mode, we exclude_binaries from EXE since they will be in COLL
+exe_kwargs_onedir = {**exe_kwargs_base, "upx": False, "exclude_binaries": True}
+# In single-file mode, we set upx: true because it works. (It might actually work in multi-file mode too)
+exe_kwargs_onefile = {**exe_kwargs_base, "upx": True, "exclude_binaries": False}
+exe_kwargs = exe_kwargs_onedir if MAKE_EXE_COLLECTION else exe_kwargs_onefile
+
+exe = EXE(*exe_args, **exe_kwargs)
+
+if MAKE_EXE_COLLECTION:
+    coll = COLLECT(exe, *coll_tocs, name="sgr-pkg", strip=False, upx=False)