Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
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
45 changes: 21 additions & 24 deletions .github/workflows/development-environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ jobs:
timeout-minutes: 40
strategy:
matrix:
# macosx-11.0 is Big Sur, however, it takes long for jobs to get started
# Using Ubuntu 18 until I figure out this error:
# -> ImportError: libffi.so.6: cannot open shared object file: No such file or directory
os: [macos-11.0, ubuntu-18.04]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes above are auto-formatting.

Expand All @@ -31,43 +30,41 @@ jobs:
- name: Checkout sentry
uses: actions/checkout@v2

- name: Set variables for caches
id: info
run: |
echo "::set-output name=brew-cache-dir::$(brew --cache)"
echo "::set-output name=yarn-cache-dir::$(yarn cache dir)"

- name: Cache (brew)
uses: actions/cache@v2
with:
path: ${{ steps.info.outputs.brew-cache-dir }}
key: devenv-${{ runner.os }}-brew-${{ hashFiles('Brewfile') }}
restore-keys: devenv-${{ runner.os }}-brew

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to take advantage of this PR and include brew caching.

- name: Install prerequisites
# Xcode CLI & brew are already installed, thus, no need to call xcode-select install
# Sometimes, brew needs to be updated before brew bundle would work
# After installing Docker (via homebrew) we need to make sure that it is properly initialized on Mac
run: |
brew update && brew bundle -q
# This code is mentioned in our dev docs. Only remove if you adjust the docs as well
SENTRY_NO_VENV_CHECK=1 ./scripts/do.sh init-docker
if [ $(uname) == "Darwin" ]; then
# If we have a sha mistmatch we can clean up and try again
brew update -q && brew install --cask docker || brew cleanup -v docker && brew install --cask docker
# This code is mentioned in our dev docs. Only remove if you adjust the docs as well
SENTRY_NO_VENV_CHECK=1 ./scripts/do.sh init-docker
fi
HOMEBREW_NO_AUTO_UPDATE=1 brew bundle --no-upgrade || brew bundle -q

# The next few steps are to set up the cache quickly
- name: Set environment variables & others
id: info
run: |
echo "::set-output name=python-version::$(SENTRY_NO_VENV_CHECK=1 ./scripts/do.sh get-pyenv-version)"
echo "::set-output name=pip-cache-dir::$(pip3 cache dir)"
echo "::set-output name=pip-version::$(pip -V | awk -F ' ' '{print $2}')"
echo "::set-output name=yarn-cache-dir::$(yarn cache dir)"

# In a sense, we set up Python two times (once here and once via pyenv). Setting
# it up here is instant and it helps us to get the cache primed sooner
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: ${{ steps.info.outputs.python-version }}
uses: ./.github/actions/setup-python

- name: Cache (pyenv)
uses: actions/cache@v2
with:
path: ~/.pyenv
key: devenv-${{ matrix.os }}-pyenv-${{ hashFiles('.python-version') }}

- name: Cache (pip)
uses: actions/cache@v2
with:
path: ${{ steps.info.outputs.pip-cache-dir }}
key: devenv-${{ matrix.os }}-py${{ steps.info.outputs.python-version }}-pip${{ steps.info.outputs.pip-version }}-${{ hashFiles('**/requirements.txt') }}

- name: Cache (yarn)
uses: actions/cache@v1 # We are explicitly using v1 due to perf reasons
with:
Expand Down
24 changes: 21 additions & 3 deletions .github/workflows/python-deps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,33 @@ jobs:
steps:
- uses: actions/checkout@v2

- name: Set brew cache path
id: brew-info
run: |
echo "::set-output name=brew-cache-dir::$(brew --cache)"

- name: Cache (brew)
uses: actions/cache@v2
with:
path: ${{ steps.brew-info.outputs.brew-cache-dir }}
key: devenv-${{ runner.os }}-brew-${{ hashFiles('Brewfile') }}
restore-keys: devenv-${{ runner.os }}-brew

- name: Install prerequisites
# Sometimes, brew needs to be updated before brew bundle would work
run: |
brew update && brew bundle -q
if [ $(uname) == "Darwin" ]; then
# If we have a sha mistmatch we can clean up and try again
brew update -q && brew install --cask docker || brew cleanup -v docker && brew install --cask docker
# This code is mentioned in our dev docs. Only remove if you adjust the docs as well
SENTRY_NO_VENV_CHECK=1 ./scripts/do.sh init-docker
fi
HOMEBREW_NO_AUTO_UPDATE=1 brew bundle --no-upgrade || brew bundle -q

- name: Setup python
id: setup-python
- name: Setup Python
uses: ./.github/actions/setup-python
with:
# TODO:
# XXX: We need to pass this python-deps-${{ matrix.os }}-py${{ matrix.python-version }}-${{ hashFiles('requirements-*.txt') }}
cache-files-hash: ${{ hashFiles('requirements-*.txt') }}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
#!/bin/bash
# This is a script to generate a dynamic plist for the Docker installation on Mac
# Major hack. See details: https://github.com/docker/for-mac/issues/2359#issuecomment-908628717
version=$(defaults read /Applications/Docker.app/Contents/Info.plist VmnetdVersion)

cat >/Library/LaunchDaemons/com.docker.vmnetd.plist <<EOF
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
Expand All @@ -23,6 +29,7 @@
</dict>
</dict>
<key>Version</key>
<string>59</string>
<string>${version}</string>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version can change depending on what Docker gets installed.
Unfortunately, I never figured out how to make brew pin a specific Docker version.

</dict>
</plist>
EOF
71 changes: 41 additions & 30 deletions scripts/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -75,48 +75,59 @@ sudo-askpass() {
init-docker() {
# Need to start docker if it was freshly installed or updated
# You will know that Docker is ready for devservices when the icon on the menu bar stops flashing
if query-mac && ! require docker && [ -d "/Applications/Docker.app" ]; then
echo "Making some changes to complete Docker initialization"
# allow the app to run without confirmation
xattr -d -r com.apple.quarantine /Applications/Docker.app

# preemptively do docker.app's setup to avoid any gui prompts
# This path is not available for brand new MacBooks
sudo-askpass /bin/mkdir -p /Library/PrivilegedHelperTools
sudo-askpass /bin/chmod 754 /Library/PrivilegedHelperTools
sudo-askpass /bin/cp /Applications/Docker.app/Contents/Library/LaunchServices/com.docker.vmnetd /Library/PrivilegedHelperTools/
sudo-askpass /bin/chmod 544 /Library/PrivilegedHelperTools/com.docker.vmnetd

# This file used to be generated as part of brew's installation
if [ -f /Applications/Docker.app/Contents/Resources/com.docker.vmnetd.plist ]; then
sudo-askpass /bin/cp /Applications/Docker.app/Contents/Resources/com.docker.vmnetd.plist /Library/LaunchDaemons/
else
sudo-askpass /bin/cp .github/workflows/files/com.docker.vmnetd.plist /Library/LaunchDaemons/
if query-mac; then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the code of this function only applies to Mac machines, thus, adding the check here instead on each three involved functions.

if ! require docker && [ -d "/Applications/Docker.app" ]; then
setup-docker
fi
sudo-askpass /bin/chmod 644 /Library/LaunchDaemons/com.docker.vmnetd.plist
sudo-askpass /bin/launchctl load /Library/LaunchDaemons/com.docker.vmnetd.plist
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm moving this code block into the function setup-docker to make the if/condition more clear to read.

start-docker
ensure-docker-server
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm willing now to wait that Docker starts up rather than finding out many minutes later when we run make bootstrap.

fi
start-docker
}

setup-docker() {
echo "Making some changes to complete Docker initialization"
# To disable the UI prompt Docker is an app downloaded from the internet
# allow the app to run without confirmation
xattr -d -r com.apple.quarantine /Applications/Docker.app

# preemptively do docker.app's setup to avoid any gui prompts
# This path is not available for brand new MacBooks
sudo-askpass /bin/mkdir -p /Library/PrivilegedHelperTools
sudo-askpass /bin/chmod 754 /Library/PrivilegedHelperTools
sudo-askpass /bin/cp /Applications/Docker.app/Contents/Library/LaunchServices/com.docker.vmnetd /Library/PrivilegedHelperTools/
sudo-askpass /bin/chmod 544 /Library/PrivilegedHelperTools/com.docker.vmnetd

set -x
# Generate dynamic plist to allow starting Docker up without GUI prompt
sudo-askpass ./.github/workflows/scripts/write-docker-plist.sh
sudo-askpass /bin/chmod 644 /Library/LaunchDaemons/com.docker.vmnetd.plist
sudo-askpass /bin/launchctl load /Library/LaunchDaemons/com.docker.vmnetd.plist
}

# This is mainly to be used by CI
# We need this for Mac since the executable docker won't work properly
# until the app is opened once
start-docker() {
if query-mac && ! docker system info &>/dev/null; then
if ! docker system info &>/dev/null; then
echo "About to open Docker.app"
# At a later stage in the script, we're going to execute
# ensure_docker_server which waits for it to be ready
if ! open -g -a Docker.app; then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is useless since open does not wait for the execution to complete.

# If the step above fails, at least we can get some debugging information to determine why
sudo-askpass ls -l /Library/PrivilegedHelperTools/com.docker.vmnetd
ls -l /Library/LaunchDaemons/
cat /Library/LaunchDaemons/com.docker.vmnetd.plist
ls -l /Applications/Docker.app
fi
open -g -a Docker.app
fi
}

ensure-docker-server() {
# Taken from https://github.com/docker/for-mac/issues/2359#issuecomment-607154849
# Wait for the server to start up
echo "Waiting for server to start up"
i=0
while ! docker system info &>/dev/null; do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should abort within 30 tries, otherwise, we might end up running until the workflow times out.

((i++ == 0)) && printf %s '-- Waiting for Docker to finish starting up...' || printf '.'
sleep 1
done
((i)) && printf '\n'

echo "Docker is ready."
}

upgrade-pip() {
pip install --upgrade "pip==21.1.2" "wheel==0.36.2"
}
Expand Down
5 changes: 3 additions & 2 deletions scripts/pyenv_setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ _append_to_startup_script() {
;;
*/zsh)
# shellcheck disable=SC2016
echo -e '# It is assumed that pyenv is installed via Brew, so this is all we need to do.\n' \
'eval "$(pyenv init --path)"' >>"${1}"
echo -e '# It is assumed that pyenv is installed via Brew, so this is all we need to do.' \
'\neval "$(pyenv init --path)"' >>"${1}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a small nit. It would add a white space before the word eval.

;;
*/fish)
# shellcheck disable=SC2016
Expand All @@ -68,6 +68,7 @@ _append_to_startup_script() {
}

append_to_config() {
[[ ! -f "$1" ]] && touch "$1"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen EvanP be distracted by the file not being present. I might need a better fix. I will move this work to another PR.

if [[ -n "$1" ]]; then
if grep -qF "(pyenv init -)" "${1}"; then
echo >&2 "!!! Please remove the old-style pyenv initialization and try again:"
Expand Down