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

Cant run bash from GitHub CI #2416

Closed
madhavajay opened this issue May 10, 2022 · 12 comments
Closed

Cant run bash from GitHub CI #2416

madhavajay opened this issue May 10, 2022 · 12 comments
Labels
bug:normal affects many people or has quite an impact

Comments

@madhavajay
Copy link

I have this really weird issue where if I run tox from github CI it wont let me use bash inside tox. If I run this locally myself in windows or by hand on the CI machine everything is fine.

So here is an example where I run something in bash which then uses || true if it fails.

I also tried escaping with ^|^| but that didnt work.

commands =
    bash -c "docker volume rm test_domain_1_app-db-data --force || true"

Screen Shot 2022-05-10 at 4 57 16 pm

To run the tox tasks I do:

- name: Run integration tests
        if: steps.changes.outputs.stack == 'true'
        timeout-minutes: 30
        env:
          HAGRID_ART: false
        run: |
          tox -e stack.test.integration.windows
        shell: cmd

It says its shell: cmd, but I have also tried running `powershell.exe -command "tox -e stack.test.integration.windows" to no avail.

I can't figure it out. Its like the quotes are changing to single quotes somehow and then its failing, but why? Is there some way to control what shell gets used for each line under command when running in windows?

Why does this only happen with the github ci runner and not when I run the command by hand?

@madhavajay madhavajay added the bug:normal affects many people or has quite an impact label May 10, 2022
@jugmac00
Copy link
Member

Please share your tox.ini.

Also, as suggested by the bug report template, please run tox -rvv and paste the complete output as text, not as a screenshot.

Meanwhile, in order to limit the possible issues:
Could you please run bash on gha/windows and confirm it works?
Could you then please run bash inside of tox on gha/windows but with a very simple command (not using docker)?
e.g. bash --version and then bash -c "echo test"

@asottile
Copy link
Contributor

3221225477 in hex is 0xc0000005 -- access violation on windows

something is segfaulting or some such -- maybe try passenv = * as well to see if that's why

@madhavajay
Copy link
Author

Hrm, okay so I got a chance to test this, seems like the issue is still there:

toxfile

[testenv:stack.test.integration.windows]
description = Integration Tests for Core Stack
changedir = {toxinidir}
allowlist_externals =
    docker
    bash
    timeout
    chcp
setenv =
    HAGRID_ART = false
    PYTHONIOENCODING = utf-8
    GIT_PYTHON_REFRESH = quiet
passenv = *
commands =
    chcp 65001
    pip install jaxlib===0.3.7 -f https://whls.blob.core.windows.net/unstable/index.html
    pip install -e packages/syft[dev]

    pip install -e packages/hagrid
    docker --version
    hagrid launch test_network_1 network to docker:9081 --tag=local --tail=false --headless=true --test
    hagrid launch test_domain_1 domain to docker:9082 --tag=local --tail=false --headless=true --test
    hagrid launch test_domain_2 domain to docker:9083 --tag=local --tail=false --build=false --headless=true --test --vpn=false
    bash -c "(docker logs test_domain_1-frontend-1 -f &) | grep -q 'event - compiled .* successfully\|nginx' || true"
    bash -c "(docker logs test_domain_1-backend_stream-1 -f &) | grep -q 'Application startup complete' || true"
    bash -c "(docker logs test_domain_2-backend_stream-1 -f &) | grep -q 'Application startup complete' || true"
    bash -c "(docker logs test_network_1-backend_stream-1 -f &) | grep -q 'Application startup complete' || true"

    ; pytest tests/integration -m frontend -p no:randomly --co
    ; pytest tests/integration -m frontend -vvvv -p no:randomly -p no:benchmark -o log_cli=True --capture=no
    ; bash -c "docker stop test_domain_1-frontend-1 || true"

    pytest tests/integration -m network -p no:randomly --co
    pytest tests/integration -m network -vvvv -p no:randomly -p no:benchmark -o log_cli=True --capture=no

    pytest tests/integration -m e2e -p no:randomly --co
    pytest tests/integration -m e2e -vvvv -p no:randomly -p no:benchmark -o log_cli=True --capture=no

    ; pytest tests/integration -m security -p no:randomly --co
    ; pytest tests/integration -m security -vvvv -p no:randomly -p no:benchmark -o log_cli=True --capture=no

    ; hagrid land test_network_1
    ; hagrid land test_domain_1
    ; hagrid land test_domain_2

https://github.com/OpenMined/PySyft/runs/7296035966?check_suite_focus=true

I don't understand what passenv = * is supposed to do in helping debug the issue.
You can see that the quotes are being changed by something.... is that tox?

My tox file:

    bash -c "(docker logs test_domain_1-frontend-1 -f &) | grep -q 'event - compiled .* successfully\|nginx' || true"

GitHub CI log:

stack.test.integration.windows run-test: commands[8] | bash -c '(docker logs test_domain_1-frontend-1 -f &) | grep -q '"'"'event - compiled .* successfully\|nginx'"'"' || true'

@madhavajay
Copy link
Author

Okay so I tried this again and have gone to a much simpler test case which shows that this is simply broken.

[testenv:stack.test.integration.windows]
description = Integration Tests for Core Stack
changedir = {toxinidir}
allowlist_externals =
    docker
    bash
    timeout
    chcp
setenv =
    HAGRID_ART = false
    PYTHONIOENCODING = utf-8
    GIT_PYTHON_REFRESH = quiet
passenv = *
commands =
    bash -c "echo a"

And the error:
https://github.com/OpenMined/PySyft/runs/7297605083?check_suite_focus=true

.tox create: C:\Users\azureuser\actions-runner\_work\PySyft\PySyft\.tox\.tox
.tox installdeps: tox-run-command, pip >= 22.0.4, tox >= 3.25.1
stack.test.integration.windows create: C:\Users\azureuser\actions-runner\_work\PySyft\PySyft\.tox\stack.test.integration.windows
stack.test.integration.windows installdeps: pip
stack.test.integration.windows run-test-pre: PYTHONHASHSEED='90[8](https://github.com/OpenMined/PySyft/runs/7297605083?check_suite_focus=true#step:18:9)'
stack.test.integration.windows run-test: commands[0] | bash -c 'echo a'
ERROR: InvocationError for command 'C:\Windows\system32\bash.EXE' -c 'echo a' (exited with code 322[12](https://github.com/OpenMined/PySyft/runs/7297605083?check_suite_focus=true#step:18:13)25477)

I guess the question is, if its literally using those ' quotes around the binary path thats probably why its failing, if its not then I have no clue.

Screen Shot 2022-07-12 at 6 08 30 pm

@jugmac00
Copy link
Member

@madhavajay The passenv = * was a suggestion to rule out one common source of issues. tox does isolate the build, and does not permit external env variables - there are quite some applications out there which rely on those, but do not make it clear. That is what debugging is about - ruling out issues until you find a solution.

When there is an issue with tox, I usually recommend to try whether this issue has been fixed with the upcoming rewrite - tox4 (pip install --pre tox), though in this case there are still too many moving parts to make a good guess - at least for me.

I also would suggest to run a much simplified command on your local machine. Unfortunately I have no access to a Windows box, so I can't help here.

@asottile / @gaborbernat What's your take on this?

@asottile
Copy link
Contributor

3221225477 is the windows equivalent to a segfault

it works fine for me on my windows host so I suspect some sort of misconfiguration on the host you're on

@madhavajay
Copy link
Author

@jugmac00 and @asottile nothing simpler than:

bash -c "echo a"

This is on our self hosted Windows runners and on GitHub CI, so I don't think this is with my setup, I think this is an issue with tox and windows. Can either of you confirm that tox does indeed run correctly bash on windows?

@asottile
Copy link
Contributor

yes I can confirm that -- I did so here: #2416 (comment)

@madhavajay
Copy link
Author

Okay, I did some debugging with my local Windows VM and figured out that for some reason calling the WSL bash at C:\Windows\System32\bash.exe causes the problem.

If I manually execute it, there is no issue, so perhaps its a weird security thing, since its failing on the most simple bash command bash -c "echo a".

So switching to another bash like the one with git works fine.

Screen Shot 2022-07-13 at 1 00 40 pm

Screen Shot 2022-07-13 at 1 35 34 pm

Screen Shot 2022-07-13 at 1 48 36 pm

But since its Windows im neither surprised nor... can I even... 🙄

Thanks for the 🐥 help.

@madhavajay
Copy link
Author

Also, is there any way this can be fixed with tox. Could I for instance somehow tell tox to use a specific path for bash without having to explicitly put path\to\bash.exe in the tox commands so that it works both on linux and windows?

Maybe like a:

allowlist_externals =
    docker
    grep
    sleep
    bash:{env:BASH_PATH:bash}

@asottile
Copy link
Contributor

you can set PATH, either in your actions config or in tox

@madhavajay
Copy link
Author

I tried to do that on windows (following this actions/runner#497) and got a warning that you need to do some special work around for changing path on windows in GitHub Actions. I guess if theres a way its out of scope of this ticket anyway. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:normal affects many people or has quite an impact
Projects
None yet
Development

No branches or pull requests

4 participants