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

[Feature Request] macOS Support #30

Open
leycec opened this issue Oct 9, 2022 · 3 comments
Open

[Feature Request] macOS Support #30

leycec opened this issue Oct 9, 2022 · 3 comments

Comments

@leycec
Copy link

leycec commented Oct 9, 2022

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. 😿

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.

leycec added a commit to beartype/beartype that referenced this issue Oct 9, 2022
This commit is the next in a commit chain integrating our GitHub
Actions-based continuous integration (CI) workflow (i.e.,
`.github/workflows/python_test.yml`) with third-party GitHub Actions
statically type-checking beartype against both `mypy` and `pyright` at
CI time – including on every commit as well as pull request (PR). For
both robustness and efficiency, this commit prevents functional tests in
our test suite that perform these same static type-checks from running
under CI. In theory, doing so *should* resolve a recent spate of
spurious CI complaints. Specifically, this commit circumvents upstream
issue jpetrucciani/mypy-check#30 by temporarily disabling use of that
action (and thus mypy) entirely. Since this is *clearly* non-ideal,
revisit this shortly when something smarter arises.
(*Contemptible table in a tempestuous conniption!*)
@jpetrucciani
Copy link
Owner

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

leycec commented Oct 11, 2022

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! 🧙‍♂️ 🪄 🌟

@jakebailey
Copy link

jakebailey commented Oct 11, 2022

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.

leycec added a commit to beartype/beartype that referenced this issue Oct 11, 2022
This commit is (*hopefully*) the last in a commit chain integrating our
GitHub Actions-based continuous integration (CI) workflow (i.e.,
`.github/workflows/python_test.yml`) with third-party GitHub Actions
statically type-checking beartype against both `mypy` and `pyright` at
CI time – including on every commit as well as pull request (PR). For
both robustness and efficiency, this commit prevents functional tests in
our test suite that perform these same static type-checks from running
under CI. In theory, doing so *should* resolve a recent spate of
spurious CI complaints. Specifically, this commit:

* Conditionally restores usage of the excellent
  `jpetrucciani/mypy-check` action to statically type-check @beartype
  under both Linux and Windows but *not* macOS, circumventing upstream
  issue jpetrucciani/mypy-check#30 with a conditional `if:` clause
  miraculously authored by GitHub Actions superstar @jpetrucciani.
* Resolves a critical (albeit ultimately trivial) caching issue with
  respect to `beartype.BeartypeConf` singletons, in which singletons
  initialized with different parameters could conceivably have been
  erroneously cached to the same object. Hash collisions! Hash
  collisions everywhere!
* Resolves a local test-specific issue with `mypy`, in which our
  `test_pep561_mypy()` integration test previously attempted to
  erroneously configure `mypy` with a non-existent `.mypy.ini` file.
  *sigh*
* Publishes an external link to @beartype's discussion forums with our
  next PyPI release.

(*Fully artful dart!*)
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

No branches or pull requests

3 participants