Skip to content

Inconsistent python on mac depending on bash option #132

Closed

Description

@moble

Describe the bug
There's a really unfortunate and mostly silent bug that happens only on macos — not on ubuntu or windows. I'm not sure whether this issue could be solved more easily by setup-python or by virtual-environments, but this is where I think people will look to find this issue. If nothing else, I feel like it should be documented here.

Somewhere, I have a fairly complicated run section that (so that Windows doesn't go nuts) needs bash features — but also the login option to bash. Out of inertia, this became my way of running things:

    runs-on: macos-latest
    steps:
      - uses: actions/setup-python@v2
        with:
          python-version: 3.8

      - name: Some fancy thing
        shell: bash -l {0}
        run: |
          # Some bashy fanciness
          python --version

But the python that gets picked up on macos only is the wrong one — i.e., not 3.8. The reason is obvious if you're looking for it: using bash -l prepends /usr/local/bin:/usr/bin to PATH. The problem is that users probably aren't looking for it; their scripts are silently using the wrong version of python. Again, this is not the behavior on ubuntu or windows, both of which have their PATH variables ordered correctly to find the desired version of python. It's that difference in behavior that I'm calling a bug, and I think it's clear that ubuntu and windows have the preferable behavior.

Which version of the action are you using?

  • v1
  • v2
  • Some other tag (such as v2.0.1 or master)

Environment

  • self-hosted
  • Linux
  • Windows
  • Mac

If applicable, please specify if you're using a container

Python Versions
All versions on Mac

To Reproduce
Run a python command with bash -l as the shell on any mac instance with any python version. A simple workflow is here

Run/Repo Url

https://github.com/moble/test_github_actions_python/actions/runs/222351275

Activity

  1. jspaaks commented on Sep 7, 2020

    @jspaaks

    Thanks @moble, this report helped me solve my error.

  2. maxim-lobanov commented on Jan 11, 2021

    @maxim-lobanov
    Contributor

    Hello @moble , I have explained the same behavior in #179 (comment).
    Unfortunately, we don't see the way to resolve it from actions/setup-python or actions/virtual-environments side. It is by design on macOS that bash with login option prepends default /usr/local/bin:/usr/bin to PATH and these directories already contain system python.

    The single workaround that we can provide is adding the following line to your bash step so it will modify PATH after login.

    export PATH="$pythonLocation:$PATH"
    
  3. torressa commented on Jan 18, 2021

    @torressa

    Hey just tried the workaround in a workflow and it doesn't seem to work (as seen here).
    Not quite sure what you mean by running after login

  4. maxim-lobanov commented on Jan 18, 2021

    @maxim-lobanov
    Contributor

    Your issue is different, the initial issue was about using custom shell with -l option (login mode)

    shell: bash -l {0}
    
  5. moble commented on Jan 18, 2021

    @moble
    Author

    It is by design on macOS that bash with login option prepends default /usr/local/bin:/usr/bin to PATH and these directories already contain system python.

    @maxim-lobanov I still think this is a broken design. I understand what the -l option means, and I understand why it would prepend the system directories to PATH. But I'm suggesting that the setup-python action should append something to /etc/profile or ~/.bash_profile or whatever is relevant, so that the correct python bin gets prepended in front of those system directories. This is how conda works, for example.

    But at the very least, if it cannot be changed, this should be documented prominently somewhere. I lost most of a day trying to track down a bizarre silent bug caused by this, and I bet I'm not the only one.

  6. torressa commented on Jan 18, 2021

    @torressa

    You're probably right. pythonLocation seems consistent throughout but seems strange that setting -DPython3_ROOT_DIR=${pythonLocation} on cmake configure works only for python3.9:

    -- Found Python3: /Users/runner/hostedtoolcache/Python/3.9.1/x64/bin/python3.9 (found version "3.9.1") found components: Interpreter Development Development.Module Development.Embed 
    

    (see here for original log)

    While the rest (3.6-3.8 in my case) seem to be ignored and default to 3.9 anyway

    -- Found Python3: /usr/local/bin/python3.9 (found version "3.9.1") found components: Interpreter Development Development.Module Development.Embed 
    

    (e.g. python3.7 here)

  7. torressa commented on Jan 19, 2021

    @torressa

    Fixed it with some CMake magic

    Not sure what did it but here's what I've done:

    1. Fix the version in the find_package command using the environment variable pythonVersion (i.e.find_package(Python3 ${pythonVersion}))
    2. Set Python3_ROOT_DIR to the environment variable pythonLocation
    3. Set the strategy Python3_FIND_STRATEGY=LOCATION(after setting the CMP0094 policy to NEW)

    For more info see https://cmake.org/cmake/help/latest/module/FindPython3.html#hints

  8. moble commented on Jan 19, 2021

    @moble
    Author

    Just for anyone who skipped to the bottom of this thread: again, @torressa's problem is unrelated. The workaround for the problem mentioned at the beginning of this issue is to use bash without -l if possible, or to run export PATH="$pythonLocation:$PATH" inside your list of commands in each bash -l section.

  9. konradpabjan commented on Jan 19, 2021

    @konradpabjan
    Contributor

    Personally I think that modifying /etc/profile or ~/.bash_profile for this to work could be problematic because we have to take into account users that have self-hosted runners who use setup-python. It could mess with their machines in unexpected ways.

    We could add some post step for setup-python that does some cleanup or reverts any of these modifications that but I'm not a fan of that.

    I think a good compromise would be to

    • Add a section somewhere in the README with info about this along with possible solutions (I personally don't think it's important enough to warrant being near the top above some of the other content
    • Add some extra logs if the runtime if macOS is being used
    if (os == darwin)
    core.info("Using Python on MacOS with extra bash options can cause unexpected behavior. See ....") 
    

    If someone does hit this issue. I think they'll first check the logs for setup-python so they should see the message with linked documentation and not have to spend too much time debugging what is going on. Thoughts?

  10. moble commented on Jan 20, 2021

    @moble
    Author

    @konradpabjan I don't understand self-hosted runners, so if that's the best that can be done, I think both of your points would be quite helpful. Thanks!

  11. Chrismarsh commented on Feb 4, 2021

    @Chrismarsh

    I note that using @moble 's suggestion of
    export PATH="$pythonLocation:$PATH"
    doesn't fix pip and that I needed to use python -m pip to correctly install dependencies.

  12. moble commented on Feb 6, 2021

    @moble
    Author

    @Chrismarsh Good point. I'll note further that there seems to be some consensus among the python and pip core developers that everyone should always use python -m pip.

  13. tsibley commented on Feb 15, 2021

    @tsibley

    This workaround as a job step after setup-python seems to work ok for me. Using $pythonLocation/bin in PATH ensures that more than just python is available, e.g. python3, pip, pip3, etc.

          - if: runner.os == 'macOS'
            name: Fix Python PATH on macOS
            run: |
              tee -a ~/.profile <<<'export PATH="$pythonLocation/bin:$PATH"'
    

    I flailed for a bit at a suitable workaround because the iteration loop is so slow with GH actions and it's hard to introspect the virtual-environment.

    If you're also using setup-miniconda, note that it blows away ~/.bash_profile and ~/.bashrc, so you'll need to put this step after it. You also need to use ~/.profile as that's what setup-miniconda installs its own init into, and creating ~/.bash_profile will mask it.

    In my case, I'm using bash -l because I'm using setup-miniconda and want the Conda env activated by default. However, I'm not using Conda to provide Python, so I still want to use the one provided by setup-python.

    Edit: Argh, but Conda's /usr/local/miniconda/bin includes a Python version, which is also interfering.

  14. removed their assignment
    on Nov 8, 2021
  15. dsame commented on Aug 1, 2022

    @dsame
    Contributor

    Hello @moble,

    Because of -l options intend to reset the user's environment and the workaround is provided i close this issue, but please feel free to reopen this issue or create new one in case if the problem still exists.

  16. self-assigned this
    on Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions