Resolve windows 3.8 issue #98
Conversation
sondrelg
force-pushed
the
3.8-test
branch
4 times, most recently
from
b749a3f to
decda74
Compare
|
@miigotu looking at the poetry upstream issue, they recommend using powershell (default windows shell) instead of bash. Perhaps we should spend a little effort looking into whether we can create an ergonomic way of setting up an os-matrix with powershell as the shell for windows. We could even go as far as outputting specific errors for windows bash users, telling them to switch to powershell on python 3.7 and 3.8, though perhaps that should be considered a breaking change. Just thinking out loud 🤔 |
|
I think we should support both methods. I can look a bit at it later, tonight. I do have a huge project coming up (with a new developer joining) on sickchill. Rewriting the entire UI with jinja2, webpack, reactjs, and a flask backend on top of reorganizing the entire user flow for shows and settings. So I'll be on to get a wire frame up for him and take a look at poetry PowerShell. |
|
Yeah it now seems like the bash bug could be fixed, so yeah sounds better to support both. Maybe we should merge this in the meantime to fix v1 |
|
Yes I believe so. And maybe a refactor to support both bash, cmd, and PowerShell should be a major version bump, even though any shell they have set should work then. |
|
Does the fix in #95 not work with the newest poetry installer script? I'd much rather not pin an old installer, as there are a bunch of new configuration settings we can expose with it |
| @@ -3,7 +3,12 @@ | |||
| set -eo pipefail | |||
|
|
|||
| INSTALLATION_SCRIPT="$(mktemp)" | |||
| curl -sSL https://install.python-poetry.org/ --output "$INSTALLATION_SCRIPT" | |||
|
|
|||
| if [ "${RUNNER_OS}" == "Windows" ]; then | |||
There was a problem hiding this comment.
I dislike this much lol, can we fix this another way to keep using the latest installer?
There was a problem hiding this comment.
This is what we were doing prior to v1.3.2, one week ago, so in that sense it's not a huge change; but I agree I would never accept this if we had been on the official installation script from the beginning.
As far as I can tell there isn't another way to do this, until the upstream bug is fixed; but I would consider this a temporary band-aid until that happens. Maybe we could even assist in that. It would just be nice to have this working on the @v1 tag for windows users in the meantime.
There was a problem hiding this comment.
I agree. Just need to fix it somehow after the release. Can you link the upstream issue?
There was a problem hiding this comment.
There was a problem hiding this comment.
Opened an issue for powershell support, and one to remove the deprecated installation script. Will merge this now and release v1.3.3 👍
sondrelg
force-pushed
the
general-updates
branch
from
cecf7e2 to
e599f81
Compare
Ideally we would use the official source, but there's an open issue (looks like it might be a wontfix) here python-poetry/install.python-poetry.org#46 that's causing issues for Windows users
Adds the Python matrix to the non-default settings test. This would have uncovered the issue from #94. Then it resolves #94 by avoiding the official installation script for Windows targets.
A few things to consider:
setup-pythonafter this action works most of the time, if not all of the time.