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.

Sign up for GitHub

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

Move main script to main.sh #70

Merged
merged 6 commits into from
Feb 12, 2022
Merged

Move main script to main.sh #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff04e81
Move main script to main.sh
connortann Feb 10, 2022
da8d736
Add environment variables mapping
connortann Feb 10, 2022
81ed015
Ensure bash variables are quoted
connortann Feb 10, 2022
194e7b3
Add .gitattributes to prevent cross-platform line endings errors
connortann Feb 10, 2022
336b9a7
Bumpy latest Poetry to 1.1.13
connortann Feb 10, 2022
46d7fcb
Explicitly expand tildes in virtualenvs-path
connortann Feb 10, 2022
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
7 changes: 7 additions & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

# Ensure git handles line endings correctly
# Set the default behavior, in case core.autocrlf is not set.
* text=auto

# Ensure shell scripts are strictly linux-style
*.sh text eol=lf
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ jobs:
# feel free to update it
- run: |
source .github/scripts/assert.sh
assert_in "1.1.12" "$(poetry --version)"
assert_in "1.1.13" "$(poetry --version)"

# Make sure scripts are not deleted.
# If we deleted the scripts folder (or subfolders) by accident,
Expand Down
65 changes: 8 additions & 57 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,61 +32,12 @@ runs:
using: "composite"
steps:
- name: Install and configure Poetry
run: |
installation_script="$(mktemp)"
curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/48339106eb0d403a3c66519317488c8185844b32/install-poetry.py --output "$installation_script"

if [ "${{ runner.os }}" == "Windows" ]; then
path="C:/Users/runneradmin/AppData/Roaming/Python/Scripts"
else
path="$HOME/.local/"
fi

echo -e "\n\033[33mSetting Poetry installation path as $path\033[0m\n"
echo -e "\033[33mInstalling Poetry 👷\033[0m\n"

if [ "${{ inputs.version }}" == "latest" ]; then
POETRY_HOME=$path python3 $installation_script --yes ${{ inputs.installation-arguments}}
else
POETRY_HOME=$path python3 $installation_script --yes --version=${{ inputs.version }} ${{ inputs.installation-arguments}}
fi

echo "$path/bin" >>$GITHUB_PATH
export PATH="$path/bin:$PATH"

if [ "${{ runner.os }}" == "Windows" ]; then
poetry_="$path/bin/poetry.exe"
else
poetry_=poetry
fi

$poetry_ config virtualenvs.create ${{ inputs.virtualenvs-create }}
$poetry_ config virtualenvs.in-project ${{ inputs.virtualenvs-in-project }}
$poetry_ config virtualenvs.path ${{ inputs.virtualenvs-path }}

config="$($poetry_ config --list)"

if echo "$config" | grep -q -c "installer.parallel"; then
$poetry_ config installer.parallel "${{ inputs.installer-parallel }}"
fi

if [ "${{ runner.os }}" == "Windows" ]; then
act="source .venv/scripts/activate"
echo "VENV=.venv/scripts/activate" >>"$GITHUB_ENV"
else
act="source .venv/bin/activate"
echo "VENV=.venv/bin/activate" >>"$GITHUB_ENV"
fi

echo -e "\n\033[33mInstallation completed. Configuring settings 🛠\033[0m"
echo -e "\n\033[33mDone ✅\033[0m"

if [ "${{ inputs.virtualenvs-create }}" == true ] || [ "${{ inputs.virtualenvs-create }}" == "true" ]; then
echo -e "\n\033[33mIf you are creating a venv in your project, you can activate it by running '$act'. If you're running this in an OS matrix, you can use 'source \$VENV' instead, as an OS agnostic option\033[0m"
fi
if [ "${{ runner.os }}" == "Windows" ]; then
echo -e "\n\033[33mMake sure to set your default shell to bash when on Windows.\033[0m"
echo -e "\n\033[33mSee the github action docs for more information and examples.\033[0m"
fi

run: /bin/bash $GITHUB_ACTION_PATH/main.sh
shell: bash
env:
VERSION: ${{ inputs.version }}
VIRTUALENVS_CREATE: ${{ inputs.virtualenvs-create }}
VIRTUALENVS_IN_PROJECT: ${{ inputs.virtualenvs-in-project }}
VIRTUALENVS_PATH: ${{ inputs.virtualenvs-path }}
INSTALLER_PARALLEL: ${{ inputs.installer-parallel }}
Comment on lines +38 to +42
Copy link
Member

@sondrelg sondrelg Feb 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Poetry supports environment variables for settings (docs). I wonder if we might be able to kill two birds with one stone here. Perhaps we can rename the env vars to POETRY_* and drop code like this from the shell script completely

$poetry_ config virtualenvs.create ${VIRTUALENVS_CREATE}
$poetry_ config virtualenvs.in-project ${VIRTUALENVS_IN_PROJECT}
$poetry_ config virtualenvs.path ${VIRTUALENVS_PATH}

Needs testing, but unless I'm mistaken this should be inferred by Poetry if we do this, so the extra specification will become redundant.

Copy link
Member

Choose a reason for hiding this comment

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

If this works out, I think it might also be worth checking if a user can overwrite our env vars using the env tag when specifying the action, like this:

- uses: snok/install-poetry@v1
  with:
    version: 1.1.10
  env:
    POETRY_VERSION: 1.1.13

It would be great if the POETRY_VERSION here would take precedence, but I'm guessing it wouldn't.

INSTALLATION_ARGUMENTS: ${{ inputs.installation-arguments }}
60 changes: 60 additions & 0 deletions main.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env bash

installation_script="$(mktemp)"
curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/48339106eb0d403a3c66519317488c8185844b32/install-poetry.py --output "$installation_script"

if [ "${RUNNER_OS}" == "Windows" ]; then
path="C:/Users/runneradmin/AppData/Roaming/Python/Scripts"
else
path="$HOME/.local/"
fi

echo -e "\n\033[33mSetting Poetry installation path as $path\033[0m\n"
echo -e "\033[33mInstalling Poetry 👷\033[0m\n"

if [ "${VERSION}" == "latest" ]; then
POETRY_HOME=$path python3 $installation_script --yes ${INSTALLATION_ARGUMENTS}
else
POETRY_HOME=$path python3 $installation_script --yes --version=${VERSION} ${INSTALLATION_ARGUMENTS}
fi
Comment on lines +15 to +19
Copy link
Member

Choose a reason for hiding this comment

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

Assuming the condition becomes redundant if we use POETRY_ env vars

Suggested change
if [ "${VERSION}" == "latest" ]; then
POETRY_HOME=$path python3 $installation_script --yes ${INSTALLATION_ARGUMENTS}
else
POETRY_HOME=$path python3 $installation_script --yes --version=${VERSION} ${INSTALLATION_ARGUMENTS}
fi
POETRY_HOME=$path python3 $installation_script --yes ${INSTALLATION_ARGUMENTS}

Copy link
Member

Choose a reason for hiding this comment

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

On second thought, I'm not so sure the version will be inferred by the installer. I can help test tomorrow.

Copy link
Member

Choose a reason for hiding this comment

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

POETRY_VERSION=1.1.10 python3 $installation_script --yes prompts for a 1.1.10 installation, so that seems like it's working 👍


echo "$path/bin" >>"$GITHUB_PATH"
export PATH="$path/bin:$PATH"

if [ "${RUNNER_OS}" == "Windows" ]; then
poetry_="$path/bin/poetry.exe"
else
poetry_=poetry
fi

# Expand any "~" in VIRTUALENVS_PATH
VIRTUALENVS_PATH="${VIRTUALENVS_PATH/#\~/$HOME}"

$poetry_ config virtualenvs.create ${VIRTUALENVS_CREATE}
$poetry_ config virtualenvs.in-project ${VIRTUALENVS_IN_PROJECT}
$poetry_ config virtualenvs.path ${VIRTUALENVS_PATH}

config="$($poetry_ config --list)"

if echo "$config" | grep -q -c "installer.parallel"; then
$poetry_ config installer.parallel "${INSTALLER_PARALLEL}"
fi

Comment on lines +33 to +42
Copy link
Member

Choose a reason for hiding this comment

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

Assuming this all becomes redundant if we use POETRY_ env vars

Suggested change
$poetry_ config virtualenvs.create ${VIRTUALENVS_CREATE}
$poetry_ config virtualenvs.in-project ${VIRTUALENVS_IN_PROJECT}
$poetry_ config virtualenvs.path ${VIRTUALENVS_PATH}
config="$($poetry_ config --list)"
if echo "$config" | grep -q -c "installer.parallel"; then
$poetry_ config installer.parallel "${INSTALLER_PARALLEL}"
fi

Copy link
Member

@sondrelg sondrelg Feb 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

While using POETRY_VERSION with the installation script works, doing POETRY_VIRTUALENVS_CREATE=false python3 $installation_script --yes does not results in the correct configuration:

❯ /Users/sondrelg/.local/bin/poetry config --list
cache-dir = "/Users/sondrelg/Library/Caches/pypoetry"
experimental.new-installer = true
installer.parallel = true
virtualenvs.create = true  # <-- should be false 
virtualenvs.in-project = null
virtualenvs.path = "{cache-dir}/virtualenvs"  # /Users/sondrelg/Library/Caches/pypoetry/virtualenvs

I suppose these values are checked when running poetry install, in which case I guess we would still need to explicitly set the configuration here, or export the environment variables so they're available in subsequent steps.

if [ "${RUNNER_OS}" == "Windows" ]; then
act="source .venv/scripts/activate"
echo "VENV=.venv/scripts/activate" >>"$GITHUB_ENV"
else
act="source .venv/bin/activate"
echo "VENV=.venv/bin/activate" >>"$GITHUB_ENV"
fi

echo -e "\n\033[33mInstallation completed. Configuring settings 🛠\033[0m"
echo -e "\n\033[33mDone ✅\033[0m"

if [ "${VIRTUALENVS_CREATE}" == true ] || [ "${VIRTUALENVS_CREATE}" == "true" ]; then
echo -e "\n\033[33mIf you are creating a venv in your project, you can activate it by running '$act'. If you're running this in an OS matrix, you can use 'source \$VENV' instead, as an OS agnostic option\033[0m"
fi
if [ "${RUNNER_OS}" == "Windows" ]; then
echo -e "\n\033[33mMake sure to set your default shell to bash when on Windows.\033[0m"
echo -e "\n\033[33mSee the github action docs for more information and examples.\033[0m"
fi