snok / install-poetry

Github action for installing and configuring Poetry
MIT License
573 stars 53 forks source link

Windows Latest on Python 3.8 Constantly Fails #94

Closed adam-grant-hendry closed 1 year ago

adam-grant-hendry commented 1 year ago

For some reason, Windows-2022 is now failing for me on Python 3.8 only:

Run snok/install-poetry@v[1](https://github.com/adam-grant-hendry/qtpygraph/actions/runs/3202205327/jobs/5230946943#step:6:1).3.2
Run $GITHUB_ACTION_PATH/main.sh

Setting Poetry installation path as C:/Users/runneradmin/AppData/Roaming/Python/Scripts

Installing Poetry 👷

Retrieving Poetry metadata

# Welcome to Poetry!

This will download and install the latest version of Poetry,
a dependency and package manager for Python.

It will add the `poetry` command to Poetry's bin directory, located at:

C:\Users\runneradmin\AppData\Roaming\Python\Scripts\bin

You can uninstall at any time by executing this script with the --uninstall option,
and these changes will be reverted.

Installing Poetry (1.2.1)
Installing Poetry (1.2.1): Creating environment
Installing Poetry (1.2.1): An error occurred. Removing partial environment.
Traceback (most recent call last):
  File "C:/Users/RUNNER~1/AppData/Local/Temp/tmp.0IGyZ1qQC7", line 9[40](https://github.com/adam-grant-hendry/qtpygraph/actions/runs/3202205327/jobs/5230946943#step:6:42), in <module>
    sys.exit(main())
  File "C:/Users/RUNNER~1/AppData/Local/Temp/tmp.0IGyZ1qQC7", line 919, in main
    return installer.run()
  File "C:/Users/RUNNER~1/AppData/Local/Temp/tmp.0IGyZ1qQC7", line 550, in run
    self.install(version)
  File "C:/Users/RUNNER~1/AppData/Local/Temp/tmp.0IGyZ1qQC7", line 571, in install
    with self.make_env(version) as env:
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "C:/Users/RUNNER~1/AppData/Local/Temp/tmp.0IGyZ1qQC7", line 643, in make_env
    raise e
  File "C:/Users/RUNNER~1/AppData/Local/Temp/tmp.0IGyZ1qQC7", line 629, in make_env
    yield VirtualEnvironment.make(env_path)
  File "C:/Users/RUNNER~1/AppData/Local/Temp/tmp.0IGyZ1qQC7", line 319, in make
    builder.create(target)
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\venv\__init__.py", line 68, in create
    self._setup_pip(context)
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\venv\__init__.py", line 289, in _setup_pip
    subprocess.check_output(cmd, stderr=subprocess.STDOUT)
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\subprocess.py", line [41](https://github.com/adam-grant-hendry/qtpygraph/actions/runs/3202205327/jobs/5230946943#step:6:43)5, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\subprocess.py", line [49](https://github.com/adam-grant-hendry/qtpygraph/actions/runs/3202205327/jobs/5230946943#step:6:51)3, in run
    with Popen(*popenargs, **kwargs) as process:
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\subprocess.py", line 8[58](https://github.com/adam-grant-hendry/qtpygraph/actions/runs/3202205327/jobs/5230946943#step:6:60), in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\hostedtoolcache\windows\Python\3.8.10\x[64](https://github.com/adam-grant-hendry/qtpygraph/actions/runs/3202205327/jobs/5230946943#step:6:66)\lib\subprocess.py", line 1311, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
FileNotFoundError: [WinError 2] The system cannot find the file specified
Error: Process completed with exit code 1.
sondrelg commented 1 year ago

Thanks for the report. Does changing v1 to v1.3.1 (the previous release) fix the issue for you?

adam-grant-hendry commented 1 year ago

@sondrelg How strange. Yes! That completely fixed it!

...What's happened from v1.3.1 to v1.3.2?

adam-grant-hendry commented 1 year ago

Specifically, the error only happened for Windows and version 3.8. MacOS and Linux on 3.8 worked and 3.9 and 3.10 on all OS's worked.

sondrelg commented 1 year ago

We mostly just quoted env vars in the latest release. I'll add a test case for Windows later to see if I can reproduce.

Would you be able to share the relevant parts of your workflow to make that easier?

adam-grant-hendry commented 1 year ago

@sondrelg It's a public repo, which makes it easier. Here's the link: https://github.com/adam-grant-hendry/qtpygraph

adam-grant-hendry commented 1 year ago

@sondrelg Feel free to look at my workflows

miigotu commented 1 year ago

Here it is: https://github.com/python-poetry/install.python-poetry.org/issues/46

@adam-grant-hendry If you remove the shell default it will work: https://github.com/adam-grant-hendry/qtpygraph/actions/runs/3202205327/workflow#L41

adam-grant-hendry commented 1 year ago

If you remove the shell default it will work

Interesting...that's been part of what was recommended previously for this action (i.e. to use bash and the default shell). @sondrelg isn't that true? Does this need to change going forward?

sondrelg commented 1 year ago

Of course there's a regression specifically for 3.8 just as we remove it from the matrix 😅 It's a bit of a pain to have 20+ permutations of the basic install test, but I suppose this is a prime example of why it's worth having.

Yes @adam-grant-hendry, IIRC, removing the bash shell default on windows causes other problems. Not sure if they've gone away yet, but would be good to know.

Either way, I suppose we could just switch to --ssl-no-revoke if that fixes it for all platforms. Don't think certificate revocation checks are relevant when curling the hardcoded official installation script, like we're doing here. Do you agree?

sondrelg commented 1 year ago

Surprisingly, CI doesn't fail when including windows 3.8 (here) 🤔

miigotu commented 1 year ago

Surprisingly, CI doesn't fail when including windows 3.8 (here) thinking

If you add this to the test.yml it will fail https://github.com/SickChill/sickchill/blob/test-deploy/.github/workflows/pythonpackage.yml#L27L29 Or if you add shell: bash to this step: https://github.com/snok/install-poetry/actions/runs/3202445864/workflow#L135

sondrelg commented 1 year ago

There is this already

https://github.com/snok/install-poetry/blob/main/.github/workflows/test.yml#L58:L60

Are you saying the location of the default specification matters?

adam-grant-hendry commented 1 year ago

Either way, I suppose we could just switch to --ssl-no-revoke if that fixes it for all platforms. Don't think certificate revocation checks are relevant when curling the hardcoded official installation script, like we're doing here. Do you agree?

I'm perfectly fine sticking with v1.3.1 right now. I don't have an immediate need to upgrade. I personally used default: bash only because it was easier to stick to one scripting language instead of having to switch back-and-forth between that and powershell.

Not sure I can say I know much about the --ssl-no-revoke option or how it works, but if it works I'm fine with it! 😆

adam-grant-hendry commented 1 year ago

Are you saying the location of the default specification matters?

Looks like yours is specific to a job, whereas mine and @miigotu have ours for the entire workflow. Is that correct?

miigotu commented 1 year ago

There is this already

https://github.com/snok/install-poetry/blob/main/.github/workflows/test.yml#L58:L60

Are you saying the location of the default specification matters?

~I'm guessing because that job isnt creating a venv (https://github.com/snok/install-poetry/blob/main/.github/workflows/test.yml#L117) it skips the bug~

EDIT: nvm I'm really tired. Saw that line totally wrong. The linked issue is exact same though, pretty sure its the same bug.

miigotu commented 1 year ago

~In that issue they said its only with msys bash on windows, I have to check OP logs but here it is git bash https://github.com/snok/install-poetry/actions/runs/3202507531/jobs/5231559465#step:3:104~

NVM his is git bash also: https://github.com/adam-grant-hendry/qtpygraph/actions/runs/3202205327/jobs/5230946897#step:6:19

miigotu commented 1 year ago

I wonder if it is him setting: SHELLOPTS: errexit:pipefail and we do that also, causing some weirdness? I dont see anything else really different with the environment.

sondrelg commented 1 year ago

So far I've tried:

but so far nothing reproduces

sondrelg commented 1 year ago

In the latest push to #95 I've added the --ssl-no-revoke command to main.sh.

@adam-grant-hendry, would you mind testing with @94 instead of v1.3.1 to see if that works for you?

GuillaumeFalourd commented 1 year ago

I was having the same issue than @adam-grant-hendry in my workflow (windows-latest) using:

      - name: Install and configure Poetry
        uses: snok/install-poetry@v1
        with:
          version: 1.1.10
          virtualenvs-create: false

and (after trying to update to the latest version) with:

      - name: Install and configure Poetry
        uses: snok/install-poetry@v1.3.2
        with:
          version: 1.2.1
          virtualenvs-create: false

PS: This 2nd setup worked on Ubuntu and MacOS runners

After reading this thread and seeing the tests workflows @sondrelg shared, I used instead this config on windows:

      - name: Install and configure Poetry
        uses: snok/install-poetry@v1.3.1
        with:
          version: 1.1.14
          virtualenvs-create: false

And the error didn't occur anymore 🤷‍♂️

miigotu commented 1 year ago

We need to find the cause though. Can you try updating just one or the other of those? Aka is the problem here, or in poetry itself?

GuillaumeFalourd commented 1 year ago

@miigotu: It worked on the windows-latest runner with the action version 1.3.1, but not with the version 1.3.2, using poetry version 1.1.14 in both scenarios.

EDIT: Idem with poetry version 1.2.1 (therefore the problem seems related to the latest action tag: 1.3.2).

sondrelg commented 1 year ago

Then perhaps the changes made to https://github.com/snok/install-poetry/blob/main/main.sh#L38:L40 is the reason.

The thing we need to get to the bottom of this is a reproducible example of a failing workflow for v1.3.2. So far we haven't been able to reproduce the issue. Any ideas as to why not @GuillaumeFalourd?

GuillaumeFalourd commented 1 year ago

I've got a workflow reproducing the error here using this workflow implementation.

EDIT: Error occurred with both Python versions 3.8 and 3.10

GuillaumeFalourd commented 1 year ago

After making some tests in a fork, it seems the problem is coming from this commit, by adding the set -eo pipefail line to the main.sh file.

Just removing this line from the main.sh made the previous workflow which was failing working again: evidence

EDIT: I also tested it on windows with more python and poetry versions here successfully.

sondrelg commented 1 year ago

Great, thanks for that! I'll do a little bit more testing, then see if we can release a fix in a few hours 👍

miigotu commented 1 year ago

https://askubuntu.com/questions/1044280/set-eo-pipefail-not-working-in-windows-subsystem-for-linux-ubuntu-16-04

sondrelg commented 1 year ago

Having had time to look at https://github.com/python-poetry/install.python-poetry.org/issues/46 @miigotu, I think they're saying git bash on windows runners are affected, so I think this is an upstream bug. The script target for this github action was changed a while back, but I hadn't released a new version until v1.3.2, so that's why this is happening.

Perhaps we should do a simple

if windows on python 3.8 or less:
    install using old script
else:
    install using new script

What do you think?