Skip to content

BLD: Define PyErr_SetFromWindowsErr on Cygwin. #23066

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 26, 2022
Merged

BLD: Define PyErr_SetFromWindowsErr on Cygwin. #23066

merged 2 commits into from
Jul 26, 2022
+4 −0

Conversation

DWesl
Copy link
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
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
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
Contributor

anntzer commented May 18, 2022

No hurries.

@tacaswell tacaswell added this to the v3.5.3 milestone May 18, 2022
@DWesl
Copy link
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
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
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