Port CI to GitHub Actions

[CAM-Gerlach]

As mentioned in PR #53 , I've ported this repo's CI setup to Github Actions following the recommend best practices, cleaning up some deprecated/obsolete syntax in the commands along the way.

In addition, I've refined the test runner call to check invalid bytes/string/int/etc comparisons (-bb); enable Python development mode to print warnings (e.g. forthcoming deprecations), provide more useful output on errors, and enable several additional runtime checks (-X dev); ignore the presumably expected invalid version warning (-W ignore::UserWarning:setuptools.dist), and enable more informative unittest output for debugging and verification (-v).

Finally, I've expanded the test matrix to test all three major platforms (Linux, Windows and macOS) to avoid regressions like #28 from being introduced in the future, while bookending the text matrix with the minor versions supported (given Python's 5-year deprecation policy, the probability of a failure on an intermediate version is vanishingly small, and furthermore 3.7 is still tested through PyPI).

From my experience, for security reasons newly-added actions won't actually run on the PR that adds them unless the PR feature branch is on this repo, not mine, so it is not possible to actually test this as-is. Options to deal with this:

• Temporarily add me as a collaborater on the repo so I can create a new branch here and create a PR for it against master, so the port can be tested and debugged immediately before merging, without any extra PR churn or other steps from you.
• Create a new branch add-github-actions on this repo and I can retarget this PR against that; the actions will only run after you merge this PR to that branch and any fixes would require more PRs, but at least we can treat that branch as a feature branch and you can do fixups/squashing/cleanup as necessary once everything's working before merging to master
• Just target master directly (as this currently does), and use followup PRs if needed to get everything working

[regebro]

I was just about to ask you if you wanted direct access to the repository, so that suits me fine.

[CAM-Gerlach]

That would be ideal, yeah, thanks. I wouldn't touch master, just make any changes needed to get this working in a feature branch, and submit it as another PR for your review and approval when ready. Would probably be best to have it in before PR #58 so those changes can be automatically tested across platforms and interpreter versions.

[CAM-Gerlach]

@regebro After fixing the tests to translate newlines properly, looks like I got them working on all platforms and interpreters, except for PyPy-3.7 on Windows. Seems like what's going on is because, as mentioned in PR #58 , the setup.py in the obsolete, unmaintained distribute package doesn't close files properly (which PyPI is particularly sensitive to), and exec is used to execute this arbitrary code in the primary pyroma process, the manual cleanup on the temporary directory obtained by the legacy tempfile.mkdtemp() function fails, which is a legitimate error that can happen on plenty of (poor-quality) real world code, not just an artifact of how the tests are run.

To address this, I will try replacing the low-level mkdtemp with the recommended high-level tempfile.TemporaryDirectory, and if using it as a local context manager doesn't work, ironically relying on the implicit destructor behavior might just get around the problem by deferring the removal until after the file objects opened by the exec call are destroyed, their file handles closed and thus the OS locks released, though relying on that kind of non-deterministic behavior is really quite an awful hack and might or might not work reliably.

The better long-term solution, I think, is to avoid using exec which is the root cause of these problems, and instead build a sdist in a subprocess and then read the written-out metadata in the standard format from there with e.g. importlib_metadata (you might have to get the built package on the PYTHONPATH first). This would also greatly help toward supporting PEP 517 isolated builds to decouple the implementation from setuptools as the recommended best practices in the packaging community move away from it, a likely necessary step in this package's future evolution to assure its continued relevancy for years to come. However, that's kinda out of scope here, so assuming unittests has something similar to Pytest's skip tests decorater (I'm not really familiar with unittests, since basically all the major projects I work with use pytest), if the above doesn't work we can just skip that test on PyPy AND Windows for now.

[CAM-Gerlach]

Relying on the implicit destructor behavior of tempfile.TemporaryDirectory does appear to work to get the tests to pass, but then an error is still raised when Python exits for PyPy3 on Windows. This even persists when I manually call cleanup in a try-except, because apparently since the removal fails, the finalizer is never executed and the weakref is never deallocated, thus it still runs again and produces an error on Python exit.

To fix this, I instead revert to the previous approach using tempfile.mkdtemp() and shutil.rmtree(tempdir), just with ignore_errors=True, and rely on the OS to clean up anything that's left (as it should eventually, since they are in an OS-level temporary directory). This also has the advantage of ensuring a deterministic state at block exit and that the pyroma run completes without erroring out. Optionally, we could use the onerrorhandler ofshutil.rmtree` to log a warning if an error occurs removing a file, if that's so desired.

This produces all green tests across the board, so this should be good to merge to master once you review and approve. It should still be fixed long-term by relying something other than exec hacks, but at least this gets things working reliably now without too much fuss.

I'll submit a BPO upstream to the Python bug tracker to propose implementing an ignore_error parameter to ``TemporaryDirectory` that will do this for us automatically while allowing using the nicer higher-level API and context manager interface...someday.

[CAM-Gerlach]

Proposed on BPO-29982

[regebro]

"The better long-term solution, I think, is to avoid using exec which is the root cause of these problems, and instead build a sdist in a subprocess and then read the written-out metadata in the standard format from there with e.g. importlib_metadata (you might have to get the built package on the PYTHONPATH first)."

Yeah, that could be a solution. I've been trying a lot of solutions, but making a parser for the built metadata was not something I felt like doing. Now when somebody else has done it, that would work. The exec() solution was always just a final resort because nothing else worked. :-)

[regebro]

Seems good to me.