Skip to content

BLD: Define PyErr_SetFromWindowsErr on Cygwin.#23066

Merged
anntzer merged 2 commits intomatplotlib:mainfrom
DWesl:cygwin-define-pyerr-setfromwindowserr
Jul 26, 2022
Merged

BLD: Define PyErr_SetFromWindowsErr on Cygwin.#23066
anntzer merged 2 commits intomatplotlib:mainfrom
DWesl:cygwin-define-pyerr-setfromwindowserr

Conversation

@DWesl
Copy link
Copy Markdown
Contributor

@DWesl DWesl commented May 18, 2022 •
edited
Loading

PR Summary

Define PyErr_SetFromWindowsErr on Cygwin. This function is defined on Windows, not Cygwin.

Closes #22997.

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

@DWesl
BLD: Define an error function on Cygwin.
2bea21b 
This function is defined on Windows, not Cygwin.
Should maybe request that upstream.
@DWesl DWesl mentioned this pull request May 18, 2022
Closed
anntzer
anntzer reviewed May 18, 2022
View reviewed changes
@anntzer
Copy link
Copy Markdown
Contributor

anntzer commented May 18, 2022

@DWesl just as curiosity, if you can build cpython from source yourself, can you check whether changing the ifdef at https://github.com/python/cpython/blob/39a54ba63850e081a4a5551a773df5b4d5b1d3cd/Include/pyerrors.h#L188 and https://github.com/python/cpython/blob/a4460f2eb8b9db46a9bce3c450c8b038038a7c93/Python/errors.c#L867 to also define these functions on cygwin works?

Also, should the blocks protected by ifdef _WIN32 in _c_internal_utils.c also likewise be also enabled on cygwin (with similar fixes for the uses of PyErr_SetFromWindowsErr)?

@DWesl
BUG: Fix the returned value for PyErr_SetFromWindowsErr
8924913 
I didn't notice that PyErr_SetString returned void.
This should return something normal.
@DWesl DWesl changed the title BLD: Define an error function on Cygwin. BLD: Define PyErr_SetFromWindowsErr on Cygwin. May 18, 2022
@DWesl
Copy link
Copy Markdown
Contributor Author

DWesl commented May 18, 2022

It looks like enabling the Windows code in _c_internal_utils.c wouldn't hurt anything, and I might be able to rewrite the Linux code to work as well. It may be a bit before I can run and test that.

@anntzer
Copy link
Copy Markdown
Contributor

anntzer commented May 18, 2022

No hurries.

@tacaswell tacaswell added this to the v3.5.3 milestone May 18, 2022
@DWesl
Copy link
Copy Markdown
Contributor Author

DWesl commented May 30, 2022

It looks like it compiles, but it's a bit tricky to get useful test results with the system FreeType, rather than specifically FreeType 2.6.1

@jklymak
Copy link
Copy Markdown
Member

jklymak commented Jun 30, 2022

Is this still a work-in-progress? Moved to draft, but feel free to ask that it be moved back for active review...

@jklymak jklymak marked this pull request as draft June 30, 2022 09:10
QuLogic
QuLogic approved these changes Jul 26, 2022
View reviewed changes
Copy link
Copy Markdown
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.

This successfully fixes build on Cygwin; not sure there is anything else that really needs to be done.

@QuLogic QuLogic marked this pull request as ready for review July 26, 2022 04:05
@QuLogic QuLogic mentioned this pull request Jul 26, 2022
Merged
6 tasks
@anntzer anntzer merged commit 446de7b into matplotlib:main Jul 26, 2022
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jul 26, 2022
@anntzer @meeseeksmachine
Backport PR matplotlib#23066: BLD: Define PyErr_SetFromWindowsErr on …
a372bb3 
…Cygwin.
@meeseeksmachine meeseeksmachine mentioned this pull request Jul 26, 2022
Merged
QuLogic added a commit that referenced this pull request Jul 26, 2022
@QuLogic
Merge pull request #23488 from meeseeksmachine/auto-backport-of-pr-23…
4ee298b 
…066-on-v3.5.x

Backport PR #23066 on branch v3.5.x (BLD: Define PyErr_SetFromWindowsErr on Cygwin.)
@DWesl DWesl deleted the cygwin-define-pyerr-setfromwindowserr branch February 25, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QuLogic QuLogic QuLogic approved these changes

@anntzer anntzer anntzer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.5.3

Development

Successfully merging this pull request may close these issues.

[Bug]: Cygwin build fails due to use of Windows-only functions in _tkagg.cpp

5 participants

@DWesl @anntzer @jklymak @QuLogic @tacaswell