jpetrucciani / mypy-check

github action for python's mypy type checker tool
MIT License
33 stars 14 forks source link

[Feature Request] macOS Support #30

Open leycec opened 2 years ago

leycec commented 2 years ago

So... This action is really great under both Linux and Windows runners – but doesn't appear to support macOS. Sadness erupts like candy out a Halloween pumpkin.

This seems to be happening due to this action internally requiring Docker. Microsoft themselves were unable to convince Docker to extend licensing support for Docker Enterprise to macOS. Ergo, Docker is unavailable under macOS runners. If that isn't business madness, I don't know what is.

It's Been a Rough Saturday Night

I hit this issue just tonight for @beartype, a near-real-time runtime type-checker I maintain for fun and QA. Our GitHub Actions test workflow exercises @beartype against Linux, macOS, and Windows via the standard one-liner:

        platform: [ubuntu-latest, windows-latest, macos-latest]

So far, so good. This action mostly behaved as expected under Linux and Windows but then inexplicably failed under macOS with non-human-readable exceptions resembling:

/Users/runner/work/_temp/f61a2437-0d71-496a-ad50-79d503816400.sh:
    line 1: docker: command not found
Error: Process completed with exit code 127.

It took me what felt like a literal eternity to realize that macOS runners fail to ship Docker due to a petty licensing dispute on Docker's part. That same thread has what appears to be a non-working workaround using Homebrew as well as unsubstantiated advice to switch to the open-source Docker alternative podman. Frankly, I suspect nothing works.

At the moment, I've reluctantly disabled this action entirely in our workflow. That's clearly non-ideal. Surely something clever can be done! Sadly, I am not clever.

I'm really not as conversant in GitHub Actions as I'd like to be. Is there some easy way for macOS-friendly workflows like ours to just conditionally skip this action under macOS? Alternately, is there some easy way for this action to begin supporting macOS?

I assume not, because Docker gonna Docker. This is why I use sad cat emojis. :crying_cat_face:

But this Action Still Rocks!

@beartype is currently just using @jakebailey's excellent pyright-action action to statically type-check. That's fine for now, but we'd really love to also statically type-check against mypy at some point.

Until then, thanks so much for maintaining this extraordinary action on behalf of the whole typing community. You're amazing, @jpetrucciani.

jpetrucciani commented 2 years ago

Thank you for the kind words, and detailed issue! 😄

I would love to explore the possibility of turning this into a non-docker action! I usually default to this type of setup because docker does help make actions a lot less boilerplate-y, and easier to configure an environment for. I'd need to explore a bit more as to the current best practices of building js-based actions though. I'm not quite sure at the moment when I'll have some time to dig in unfortunately 😞

However, in the meantime, you should be able to selectively disable running this action only on the mac run of the matrix you have in place! Taking a look at your workflow, you should be able to add an if directive to the step:

- name: 'Type-checking package with "mypy"...'
  uses: 'jpetrucciani/mypy-check@master'
  if: matrix.platform != "macos-latest"
  with:
    python_version: "${{ matrix.python-version }}"
    path: 'beartype'

This should allow you to continue running with your matrix options intact, only skipping the run of mypy via macos (until one day when it does actually support it!)

leycec commented 1 year ago

Wunderbar. That's a wonderful workaround. My GitHub Actions skills are feeble compared to your own. I never knew that job steps accepted an optional if: clause. Today, I have finally levelled up.

That absolutely suffices for @beartype. That probably suffices for most other open-source projects, too. After all, if mypy passes under other platforms (e.g., Linux, Windows), then mypy probably passes under macOS too – even if we can't actually validate that just yet.

Refactoring from Docker to JavaScript seems like a mountain of a headache. I feel your volunteer pain. Perhaps we could simply add a FAQ entry to this project's README.md outlining what to do in the event of a docker: command not found error under macOS runners and then close this issue out?

Thanks again for the extraordinary fast turnaround time and the stunning workaround that actually works. You manually inspected our CI workflow and everything. Please take all the GitHub karma I have to give! :mage_man: :magic_wand: :star2:

jakebailey commented 1 year ago

Skimming through the repo, I'm noticing that it mainly consists of a shell script and a little python to pull info out of the mypy output.

You might be able to get away with converting this to a compostite action instead: https://docs.github.com/en/actions/creating-actions/creating-a-composite-action

This would let you run the shell script, and it's already the case that the runner will have python available to run the other script (or at the very least, the user will have already run setup-pyhon).

Writing it in TS or JS would actually be not all too bad either, but certainly more work than the above trick if you don't need to do any caching.