Skip to content

CI: Add a Cygwin run to GHA CI. #22999

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

Merged
merged 12 commits into from
Jan 11, 2023
Merged

CI: Add a Cygwin run to GHA CI. #22999

merged 12 commits into from
Jan 11, 2023

Conversation

DWesl
Copy link
Contributor

@DWesl DWesl commented May 7, 2022

PR Summary

Add a Cygwin run to GHA, as suggested by #22997 (comment)

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

@oscargus
Copy link
Member

oscargus commented May 7, 2022

I think that libjpeg-devel is also required (as you probably have figured out as well).

@oscargus oscargus added the Build label May 7, 2022
@DWesl
Copy link
Contributor Author

DWesl commented May 7, 2022

I can try that and see if it works (I shouldn't run into python-pillow/Pillow#6216)

@DWesl
Copy link
Contributor Author

DWesl commented May 7, 2022

The build is now failing in the expected place, so let's see if the suggestion in #22997 (comment) allows the build to finish.

@DWesl
Copy link
Contributor Author

DWesl commented May 7, 2022

The build is succeeding now, but the tests fail with ImageComparisonErrors due to using the wrong version of FreeType. Let's see if that compiles now.

@DWesl
Copy link
Contributor Author

DWesl commented May 8, 2022

The freetype compile still fails during configure; the closest Cygwin build script I can find is here. The first patch doesn't look like the kind of thing to fix a configure crash (probably the same as this one), and the name of the second patch doesn't look relevant.

The only other thing I can think of is running autoconf/autoreconf.

@DWesl
Copy link
Contributor Author

DWesl commented May 14, 2022

The Cygwin CI is still failing in FreeType configure:
https://github.com/matplotlib/matplotlib/runs/6436739391?check_suite_focus=true#step:11:756

I can get the compilation done using system FreeType and QHull, but building the FreeType version needed for the tests to pass fails, either during the libtoolize step of autoreconf or during configure without that step.

Is there a way to skip all the tests that require an exact image match? It wouldn't provide a thorough test of Matplotlib on Cygwin, but most of the differences from Windows and Linux are in compiling and importing the extensions, so that might be enough.

@DWesl
Copy link
Contributor Author

DWesl commented Jun 8, 2022

The bundled FreeType build fails even if I push through after the libtoolize failure. I can try again without the bundled FreeType, and see if I can eliminate the fork() failures, even if the ImageComparisonFailures stick around.

@DWesl
Copy link
Contributor Author

DWesl commented Jun 8, 2022

I don't know why upload-artifact fails; the previous step should give everyone read and write permissions to everything in that directory and also execute permissions to every sub-directory.

@DWesl
Copy link
Contributor Author

DWesl commented Jun 27, 2022

I haven't seen that error message on a fork() failure before:

      1 [main] python3.8 (5008) C:\cygwin\bin\python3.8.exe: *** fatal error in forked process - MEM_COMMIT failed, Win32 error 1455
  13465 [main] python3.8 (5008) cygwin_exception::open_stackdumpfile: Dumping stack trace to python3.8.exe.stackdump
2003466 [main] python 2011 dofork: child -1 - forked process 5008 died unexpectedly, retry 0, exit code 0x100, errno 11

The upload-artifact step is also still failing, and I have no idea why.

@DWesl
Copy link
Contributor Author

DWesl commented Jun 27, 2022

Do Windows ReadAndExecute permissions not imply Read or ReadPermissions? I wouldn't think upload-artifact would need Write, ExecuteFile, or delete permissions.

@DWesl
Copy link
Contributor Author

DWesl commented Jul 4, 2022

And now the FreeType build is failing with no apparent reason given.

@QuLogic
Copy link
Member

QuLogic commented Jul 26, 2022

I do not understand all the extra work that needs to be done here. On Cygwin x86_64, I installed Python 3.9, gcc-c++, make, and I think python39-matplotlib, and was able to build FreeType just fine (with #23066). I did have trouble with downloading qhull, which I have not yet investigated.

@DWesl
Copy link
Contributor Author

DWesl commented Jul 26, 2022

I was also able to build FreeType locally, but it still fails in CI, for reasons the error message does not seem to explain. Trying to narrow down the cause of that failure is the bulk of the commits.

@DWesl
Copy link
Contributor Author

DWesl commented Dec 1, 2022

fatal error in forked process - MEM_COMMIT failed, Win32 error 1455

Well that's a new kind of fork() failure.

@DWesl
Copy link
Contributor Author

DWesl commented Dec 3, 2022

The most recent commit should have stopped trying to install wxPython, so I'm not sure what to do about the six failing MPLBACKEND: wxagg tests

@tacaswell
Copy link
Member

From this cryptic (truncated!) message:

stderr = 'Traceback (most recent call last):\n  File "<string>", line 1, in <module>\n  File "/cygdrive/d/a/matplotlib/matplotl....8/site-packages/wx/core.py", line 12, in <module>\n    from ._core import *\nImportError: No such file or directory\n'

I think that wx is still installed enough to pass our "can we test this" filter (it looks like there is a wx/core.py is site-packages?!) but not enough to actually run (the _core (I assume) c-extensions are mssing).

def _get_testable_interactive_backends():
envs = []
for deps, env in [
*[([qt_api],
{"MPLBACKEND": "qtagg", "QT_API": qt_api})
for qt_api in ["PyQt6", "PySide6", "PyQt5", "PySide2"]],
*[([qt_api, "cairocffi"],
{"MPLBACKEND": "qtcairo", "QT_API": qt_api})
for qt_api in ["PyQt6", "PySide6", "PyQt5", "PySide2"]],
*[(["cairo", "gi"], {"MPLBACKEND": f"gtk{version}{renderer}"})
for version in [3, 4] for renderer in ["agg", "cairo"]],
(["tkinter"], {"MPLBACKEND": "tkagg"}),
(["wx"], {"MPLBACKEND": "wx"}),
(["wx"], {"MPLBACKEND": "wxagg"}),
(["matplotlib.backends._macosx"], {"MPLBACKEND": "macosx"}),
]:
reason = None
missing = [dep for dep in deps if not importlib.util.find_spec(dep)]
if (sys.platform == "linux" and
not _c_internal_utils.display_is_valid()):
reason = "$DISPLAY and $WAYLAND_DISPLAY are unset"
elif missing:
reason = "{} cannot be imported".format(", ".join(missing))
elif env["MPLBACKEND"] == 'macosx' and os.environ.get('TF_BUILD'):
reason = "macosx backend fails on Azure"
elif env["MPLBACKEND"].startswith('gtk'):
import gi
version = env["MPLBACKEND"][3]
repo = gi.Repository.get_default()
if f'{version}.0' not in repo.enumerate_versions('Gtk'):
reason = "no usable GTK bindings"
marks = []
if reason:
marks.append(pytest.mark.skip(
reason=f"Skipping {env} because {reason}"))
elif env["MPLBACKEND"].startswith('wx') and sys.platform == 'darwin':
# ignore on OSX because that's currently broken (github #16849)
marks.append(pytest.mark.xfail(reason='github #16849'))
envs.append(
pytest.param(
{**env, 'BACKEND_DEPS': ','.join(deps)},
marks=marks, id=str(env)
)
)
return envs

This logic can not actually import the GUI toolkits because importing more than one can lead to all manner of trouble so we check with find_spec and then run the actual tests is a sub-process (sorry if I am over explaining obvious things).

@tacaswell
Copy link
Member

It looks like it is coming from the system packages?

  15 install python3-wx-common                      4.0.7.post2-4               
...
 401 install python38-wx                            4.0.7.post2-4           

https://github.com/matplotlib/matplotlib/actions/runs/3606263992/jobs/6077436300#step:4:104
https://github.com/matplotlib/matplotlib/actions/runs/3606263992/jobs/6077436300#step:4:490

@tacaswell
Copy link
Member

FAILED lib/matplotlib/tests/test_matplotlib.py::test_tmpconfigdir_warning - AssertionError: assert 'set the MPLCONFIGDIR' in ''
634
 +  where '' = CompletedProcess(args=['/usr/bin/python', '-c', 'import matplotlib'], returncode=0, stderr='').stderr
635
FAILED lib/matplotlib/tests/test_pyplot.py::test_pylab_integration - BlockingIOError: [Errno 11] Resource temporarily unavailable
636
FAILED lib/matplotlib/tests/test_simplification.py::test_throw_rendering_complexity_exceeded

The tempdir one is already skiped on nt, I assume cygwin has the same limitations and should be skipped?

The rendering complexity one may be an actual bug that is testing that c code bails on too-big things. #21789 seems related and the most recent PR to touch this code was #19343.

I have no good theory on the pylab one.

@DWesl
Copy link
Contributor Author

DWesl commented Dec 5, 2022

FAILED lib/matplotlib/tests/test_matplotlib.py::test_tmpconfigdir_warning - AssertionError: assert 'set the MPLCONFIGDIR' in ''
634
 +  where '' = CompletedProcess(args=['/usr/bin/python', '-c', 'import matplotlib'], returncode=0, stderr='').stderr
635
FAILED lib/matplotlib/tests/test_pyplot.py::test_pylab_integration - BlockingIOError: [Errno 11] Resource temporarily unavailable
636
FAILED lib/matplotlib/tests/test_simplification.py::test_throw_rendering_complexity_exceeded

The tempdir one is already skipped on nt, I assume cygwin has the same limitations and should be skipped?

Cygwin implements its own permissions: to check I just ran the code from the test locally and got the expected warning.
Extrapolating from this FAQ entry, I think whether the test could work would depend on whether the underlying filesystem is FAT/FAT32 (fails) or NTFS/NFS (can pass). There's a possibility there's default ACLs on one of the parent directories (implied by a different FAQ entry).

The other possibility is that the test for root isn't working since the user ID for "Administrator" even running as administrator is not 0 by default (see this user guide description). I could try changing that using mkpasswd, or try to look up how to tell if the current session has the relevant "Access files regardless of permissions" token.

The rendering complexity one may be an actual bug that is testing that c code bails on too-big things. #21789 seems related and the most recent PR to touch this code was #19343.

I have no good theory on the pylab one.

That's a fork() failure, reasonably well-known on Cygwin. The three main recommendations are:

  • stop antivirus software (probably not relevant on CI, also not something we can control on CI)
  • rebase all DLLs so they have distinct memory regions to load to so they don't collide while Cygwin tries to recreate the parent memory layout in the child (done here in this PR; I can try adding /bin to be absolutely sure all files have an address and that address is found
  • restart the computer and hope it goes away (WIndows makes fork even harder sometimes; probably not relevant on CI)

@DWesl
Copy link
Contributor Author

DWesl commented Dec 5, 2022

It looks like all the rest are fork failures. There was only one before I tried to fix it, so let's see if undoing that fix gets things passing.

In case that doesn't work, is there some way to tell pytest that all instances of BlockingIOError: [Errno 11] Resource temporarily unavailable should be treated as a non-strict xfail of the relevant test on Cygwin?

@tacaswell
Copy link
Member

That logic should probably go around the subprocess calls in the helper used in the sphinx tests + subprocess_run_helper?

Might also be worth putting wrapped versions of the subproccess functions we use an matplotlib.testing and then making sure we never directly use the stdlib functions?

@DWesl
Copy link
Contributor Author

DWesl commented Dec 6, 2022

The new CI run passes! Now let's see if it can do that consistently.

It looks like the remaining failures are mostly docs-related, then a compilation failure on Windows.

@DWesl
Copy link
Contributor Author

DWesl commented Dec 7, 2022

Should I have moved the stdint include after the Python.h include rather than moving Python.h before stdint.h?

@@ -9,6 +9,8 @@
import subprocess
import sys

import pytest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

burying this import in a function should fix the docs (pytest is not a build dependency, but we want to document things from this sub-package in the docs.

Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mild preference that the codecov uploader goes in, but OK merging with out it.

@QuLogic
Copy link
Member

QuLogic commented Dec 28, 2022

Note, we do prefer rebases over merges, if you can.

@efiring
Copy link
Member

efiring commented Dec 29, 2022

It's not clear to me that this is worth the added CI load and complexity.

@QuLogic
Copy link
Member

QuLogic commented Dec 30, 2022

We discussed this a bit on call; there was some worry about how fragile Cygwin might be given none(?) of the core developers use it. I suggest making it less required by a) moving it to a separate file cygwin.yml (AFAIK, there's no real dependence on anything else in tests.yml); b) triggering on a schedule and label instead of pull request as is done in cibuildwheel.yml. I think we are unlikely to break Cygwin builds often unless editing the C++ files, so a bonus would be to get the build to automatically trigger if files in src/ were changed, but that's just a nice-to-have.

DWesl added 4 commits January 4, 2023 09:36
Python.h defines several visibility macros, which only work if defined before any stdlib header is included.
This is needed for things like pybind11's strdup to get declared.
Not a pass, but not the fault of the test.

TST,BUG: Move pytest import to just before its use.

Hopefully this fixes the doc builds.

TST: Cygwin: Mark more fork failures as xfail.

Not a pass, but not matplotlib's fault.
This involved a good bit of debugging of the build process, especially
for the bundled FreeType.  Eventually removing the debugging flags let
it just work.

There are still fork failures, but I've done everything I know to do.
CYGWIN=detect_bloda may reveal more, but may not.

parent 48e5934
author DWesl <22566757+DWesl@users.noreply.github.com> 1651882104 -0400
committer DWesl <22566757+DWesl@users.noreply.github.com> 1670678538 -0500

parent 48e5934
author DWesl <22566757+DWesl@users.noreply.github.com> 1651882104 -0400
committer DWesl <22566757+DWesl@users.noreply.github.com> 1670678484 -0500

CI: Add a Cygwin run to GHA CI.

Let's see if I translated the workflow even close to correctly.

CI: Pin Cygwin setuptools<60 and NumPy !=1.21

setuptools>60 breaks Pillow install, and may cause problems for NumPy.

NumPy 1.21 has intermittent segfaults on GHA runners, so let's just skip that.

CI: Pin Cygwin setuptools<60 and avoid problem upgrades

setuptools>=60 runs into python-pillow/Pillow#6216 and pypa/setuptools#3304.

NumPy==1.21.* has intermittent segfaults due to SIMD attempts on unaligned memory.

kiwisolver, numpy, and pillow have problems installing, so just verify that they are present, don't try to upgrade them.

CI: Upgrade Cygwin pip

CI: Install g++ and print info on Python.h

Hopefully one of these will convince NumPy to compile.

CI: Install libjpeg-devel for Pillow install

Let's see if it keeps setuptools old enough for this to work.

CI: BLD: Use system freetype and qhull

Given that I'm checking a build problem at the moment, this should allow that to show up faster.

This should probably be reverted before merging.

CI: Specify shell for Cygwin pytest step.

CI: Rebase Matplotlib DLLs before running pytest

Try to avoid "BlockingIOError: Resource Unavailable"
with a fork() error on stderr.

CI: Compile against bundled freetype.

See if this fixes the various test errors.

It will take a few tries to get freetype to actually compile, I think.

CI: Install make for libfreetype build

Let's see if this lets the build run to completion.

BLD: Run autoconf before installing bundled freetype

Let's see if this fixes the install problems.

CI: Try to get Cygwin build of FreeType working.

It doesn't fail the same way locally.

CI: Revert to actually-configured python alternatives.

BLD: Fix autoconf call on Cygwin.

BLD: Fix directory for Cygwin autoreconf.

CI: Fix directory for Cygwin autoconf in FreeType.

BLD: Fix autoreconf command line.

BLD: Run only autoconf on Cygwin, not autoreconf.

libtoolize still fails; let's see if this at least gets through configure now.

BLD: Run verbose libtoolize on bundled FreeType

Let's see if this produces useful information.

BLD: Copy the libtool files and ensure write permissions

Copy instead of symlinking, and make sure the current user has write permissions for the target files.

BLD: Add print statements to update on Autotools progress

Let's see if I this narrows down where the error happens.

BLD: Add permissions for everybody to fix libtoolize

It's the only thing I can think of that would make the copy fail.

CI: Try to clean up error reporting.

Errors importing matplotlib are distinct from test failures; pip's logs of compile failures have a lot of extra stuff not related to the actual compile failure

CI: Skip broken FreeType build on Cygwin

I'll have to update many tests, but that should be doable.

CI: Add FreeType and QHull to Cygwin CI

I'm going the hard way through the testing process.

BLD: Use auto-typed result from dlsym

Try to avoid the Cygwin-vs-Linux dlsym signature incompatibility.
(Cygwin returns FARPROC aka `long long int (*)()`, Linux returns void *)

CI: Add Cygwin dependencies and try to rebase more

Let's hope this sidesteps the BlockingIOErrors in subprocess.run

CI: Split Cygwin rebase step

One step for finding the files.
a separate step for rebasing, using only rebase.

CI: Only rebase files in /usr/ and /usr/local/

GHA gives permissions to write to /usr/local, so pip puts the new files there.

CI: Add premissions to the image comparisons

See if this lets the artifact upload step work.

CI: Use the bundled FreeType on Cygwin again

CI: Try to change rebase to avoid fork failures

BLD: Fix definition of FT_CONFIG_CONFIG_H on Cygwin

The default definition doesn't actually define everything assumed to be defined by that header.
It was changed a few versions before, but they seem to have only checked whether it worked a few places.

BLD: Add autoconf and libtoolize back in

I'd forgotten configure crashed.

BLD: Continue on libtoolize failure

CI: Try to get more information from make

CI: Uses Cygwin FreeType package again.

CI: Try doubled backslashes for result directory.

Let's see if this allows upload-artifact to succeed.

BUG: Rebase DLLs in ediable install

I skipped those earlier, which doesn't do anything useful.

CI, BUG: Install git on Cygwin runner.

Some of the tests use setuptools_scm, which needs git.

CI, BUG: Run actions/checkout before Cygwin install

Otherwise git panics about someone else owning the repository midway through cloning.

CI, BUG: Mark the repo safe again

Git is apparently picky about how I specify Windows paths.

CI, DBG: Print Windows permissions for image dir

Inspired by
https://social.technet.microsoft.com/wiki/contents/articles/31139.powershell-how-to-get-folder-permissions.aspx

Apparently Windows "dir" doesn't have an option to output permissions, that's a separate utility.

CI, DBG: Print more permissions for result_images

Try printing permissions of the images that fail to upload.  The directory seems to deny only write and delete permissions, which shouldn't cause problems.

CI, DBG: Give everyone all permissions to result img dir

Maybe this will let upload-artifact succeed.

CI: Use dash for /bin/sh on Cygwin

This seems to help libtoolize finish without crashing.

CI: Try to compile bundled freetype on Cygwin again

This should get farther.

CI: FIX: Specify proper shell when setting shell.

CI, FIX: Spell shell options correctly to set /bin/sh

CI, FIX: Use dash to set dash as /bin/sh on Cygwin

BLD: Specify /bin/dash explicitly for Cygwin FreeType build

TODO: Remember to revert the configure change.

CI: Keep bash around when making dash /bin/sh

CI: Make /bin/sh a symlink to /bin/dash

CI: Set Cygwin tempdir to /tmp

CI: Explicitly install m4 on Cygwin.

CI, DBG: Clarify what /bin/sh is around def change.

I want to see how they change before and after I change it from /bin/bash to /bin/dash

CI, FIX: Use bash to change /bin/sh to /bin/dash

CI, FIX: Clean up syntax to set sh to dash.

Also avoid duplicate work for a failure.

BLD: Reduce verbosity of FreeType setup on Cygwin

Use autoreconf instead of autoconf --verbose

Should help me figure out where the new failure is.

BLD: Stop running autoreconf on Cygwin

It doesn't seem to help much

CI, DBG: Only run upload-artifact debuggers if tests ran

Don't try to examine images if there hasn't been anything to create them.

CI, FIX: Use single quotes for condition.

Double quotes cause problems

CI: Allow Cygwin CI to run on patch-2

BLD: Revert most changes to setupext.py

Still unconditionally rebuild ./configure on Cygwin, to try to update build system.

BLD: Revert the rest of the changes to setupext.py

FreeType seems to build on unmodified matplotlib locally, so this shouldn't be needed.

CI: Install matplotlib in Cygwin CI

Hopefully this pulls in the last dependencies and keeps  the FreeType build from failing.

CI: Specify MAKEFLAGS, not MAKE

https://www.gnu.org/software/make/manual/make.html#Options_002fRecursion
suggests specifying MAKEFLAGS to ensue options are passed to make, not MAKE.  This might help with the weird "directory not found" errors.

CI, BUG: Specify --coverage in CPPFLAGS and LDFLAGS

Specifying in CPPFLAGS only causes link to fail.
LDFLAGS=-lgcov should also work, but this should be more general.

STY: Remove trainling whitespace

Get pre-commit to stop sending me emails about trailing whitespace.

CI,BUG: Make sure coverage library gets linked

There may be a problem in the configtest procedure.

CI: Cut separate build step

It's failing in the pip install step, and I want to be sure that's not because there's already a build tree.

CI: Cygwin: Don't add coverage tracing to matplotlib

It causes pip install to fail.

CI: Install importlib_resources

Apparently not listed in the requirements*.txt

BUG: Fix spelling

CI: Cygwin: Drop LaTeX packages

The PGF tests mostly fail.

CI: Set ffmpeg path so Cygwin tests don't use Windows executable

CI: Append to mplrc, don't overwrite.

Let's see if this fixes the key lookup error on import

CI: Cygwin: Create new mplrc file instead of overwriting old

Appending creates load errors in forked tests; replacing the line should fix that, and creating a new version should fix the remaining problems.

CI,FIX: Cygwin: Create mpl config directories

CI: Cygwin: Don't install WX, do install ipython

WX imports keep failing.

One test uses IPython and fails, let's make sure that's not because IPython isn't installed.

CI: Cygwin: Uninstall wxPython

It shouldn't be installed, so this should be fast.

CI: Cygwin: Try to get remaining tests working.

Rebase executables in /bin as well as those in /usr/bin
Remove default ACLs on /tmp

CI,FIX: Fix syntax and style of previous commit

//bin is not POSIX-compliant and doesn't work on Cygwin.
Delete trailing whitespace.

CI: Cygwin: Set username to root and uid to 0

This should let checks for "can I access all files regardless of permissions" work the way Linux expects.

CI,FIX: Cygwin: Try to fix sed expression for euid=0

Sed expression #2, character 0: error: no previous regex.
Not really useful, sed.

CI: Cygwin: Stop changing username, just change uid

CI: Stop rebasing binaries in /bin

They are already covered in /usr/bin, and probably the important information is in the file, not a database somewhere (I mean, it's also in a database, but you can turn that off).

CI: Cygwin: Upload code coverage data

CI: Cygwin: Update run condition for upstream repo

I changed it so I'd get quicker feedback on tests some time ago and forgot to change it back.

Update .github/workflows/tests.yml

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

Update .github/workflows/tests.yml

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

CI: Cygwin: Incorporate suggestions from review.

CI: Incorporate suggestions from review.

CI: Cygwin: Use dash for /bin/sh instead of bash.

Faster, and doesn't fail the FreeType build.

CI: Test python 3.8 and 3.9 on Cygwin

Co-authored-by: Thomas A Caswell <tcaswell@gmail.com>

CI: Cygwin: Correct matplotlibrc location

Apparently it doesn't check ~/.config/matplotlib

DOC,CI: Explain why CI runs tests with gid set to 0.

The other option is to look into cygutils-extra's cygdrop.
I have no idea how to use that, so I'm sticking with this.
I think I did this right, we will find out.
@DWesl
Copy link
Contributor Author

DWesl commented Jan 4, 2023

Note, we do prefer rebases over merges, if you can.

Rebased and consolidated commits again.

We discussed this a bit on call; there was some worry about how fragile Cygwin might be given none(?) of the core developers use it. I suggest making it less required by
a) moving it to a separate file cygwin.yml (AFAIK, there's no real dependence on anything else in tests.yml);

Done

b) triggering on a schedule and label instead of pull request as is done in cibuildwheel.yml.

I set up conditions to mirror those in cibuildwheel.yml, though I kept the scheduled runs and "run when requested" bits.

I think we are unlikely to break Cygwin builds often unless editing the C++ files, so a bonus would be to get the build to automatically trigger if files in src/ were changed, but that's just a nice-to-have.

I think I have this in the conditions, though it looks like that one needs both conditions met rather than either.

Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor typos.

It might be good to make all the "try fork; xfail on BlockingIOError if running on Cygwin" code look like each other, or all use a single helper function somewhere.

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
@QuLogic QuLogic added the CI: Run cygwin Run cygwin tests on a PR label Jan 7, 2023
@QuLogic
Copy link
Member

QuLogic commented Jan 7, 2023

DWesl added 2 commits January 9, 2023 11:32
This way I have fewer places to get things wrong and fewer places to remember to fix things.
Ideally, at least.
DWesl added 4 commits January 9, 2023 18:28
…ent.

universal_newlines is apparently the back-compatibility name;
also text describes the biggest difference in the return types.
Also add a few reference links and types.
@tacaswell tacaswell modified the milestones: v3.7.0, v3.8.0 Jan 11, 2023
@tacaswell
Copy link
Member

Lets not backport this.

@tacaswell
Copy link
Member

This has two approval and is green so I'm going to merge!

@tacaswell tacaswell merged commit 7a909fd into matplotlib:main Jan 11, 2023
@tacaswell
Copy link
Member

Thank you for sticking with us @DWesl

@DWesl DWesl deleted the patch-2 branch February 25, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build CI: Run cygwin Run cygwin tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants