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.

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

Windows Latest on Python 3.8 Constantly Fails #94

Closed
adam-grant-hendry opened this issue Oct 7, 2022 · 28 comments · Fixed by #98
Closed

Windows Latest on Python 3.8 Constantly Fails #94

adam-grant-hendry opened this issue Oct 7, 2022 · 28 comments · Fixed by #98

Comments

@adam-grant-hendry
Copy link

adam-grant-hendry commented Oct 7, 2022

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
Copy link
Member

sondrelg commented Oct 7, 2022

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

@adam-grant-hendry
Copy link
Author

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

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

@adam-grant-hendry
Copy link
Author

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
Copy link
Member

sondrelg commented Oct 7, 2022

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
Copy link
Author

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

@adam-grant-hendry
Copy link
Author

@sondrelg Feel free to look at my workflows

@miigotu
Copy link
Collaborator

miigotu commented Oct 7, 2022

Here it is: python-poetry/install.python-poetry.org#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
Copy link
Author

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
Copy link
Member

sondrelg commented Oct 7, 2022

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
Copy link
Member

sondrelg commented Oct 7, 2022

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

@miigotu
Copy link
Collaborator

miigotu commented Oct 7, 2022

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
Copy link
Member

sondrelg commented Oct 7, 2022

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
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

miigotu commented Oct 7, 2022

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
Copy link
Collaborator

miigotu commented Oct 7, 2022

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
Copy link
Collaborator

miigotu commented Oct 7, 2022

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
Copy link
Member

sondrelg commented Oct 7, 2022

So far I've tried:

  • Setting Python to 3.8.10 which is the version specifically used in the failing run above
  • Added SHELLOPTS to env
  • Tried setting windows-2022 instead of windows-latest
  • Switched to creating venvs in the basic test case

but so far nothing reproduces

@sondrelg
Copy link
Member

sondrelg commented Oct 7, 2022

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
Copy link

GuillaumeFalourd commented Oct 10, 2022

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
Copy link
Collaborator

miigotu commented Oct 10, 2022

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
Copy link

GuillaumeFalourd commented Oct 10, 2022

@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
Copy link
Member

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
Copy link

GuillaumeFalourd commented Oct 10, 2022

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
Copy link

GuillaumeFalourd commented Oct 10, 2022

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
Copy link
Member

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
Copy link
Collaborator

miigotu commented Oct 10, 2022

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

abey79 added a commit to abey79/vpype that referenced this issue Oct 11, 2022
abey79 added a commit to abey79/vpype that referenced this issue Oct 11, 2022
* Updated svgelements to 1.8.4 (fixes #537)
* Updated other deps
* Pinned snok/install-poetry to 1.3.1 for Windows test to address snok/install-poetry#94
* Cleaned up poetry version pinning in release.yml
@sondrelg
Copy link
Member

sondrelg commented Oct 11, 2022

Having had time to look at python-poetry/install.python-poetry.org#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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants