X Tutup
Skip to content

GH-107812: extend socket's netlink support to FreeBSD#107813

Merged
corona10 merged 2 commits intopython:mainfrom
igalic:fbsd/netlink
Aug 10, 2023
Merged

GH-107812: extend socket's netlink support to FreeBSD#107813
corona10 merged 2 commits intopython:mainfrom
igalic:fbsd/netlink

Conversation

@igalic
Copy link
Contributor

@igalic igalic commented Aug 9, 2023 •
edited by bedevere-bot
Loading

On FreeBSD, netlink.h lives under the netlink directory.
Extend the AC_CHECK_HEADERS to look there as well.

@bedevere-bot bedevere-bot mentioned this pull request Aug 9, 2023
Closed
@bedevere-bot
Copy link

bedevere-bot commented Aug 9, 2023

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ghost
Copy link

ghost commented Aug 9, 2023 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

corona10
corona10 requested changes Aug 9, 2023
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

I think that you have to declare AF_NETLINK something like this way, no?

cpython/Modules/socketmodule.h

Lines 98 to 105 in 0a7f48b

#ifdef HAVE_LINUX_NETLINK_H
# ifdef HAVE_ASM_TYPES_H
# include <asm/types.h>
# endif
# include <linux/netlink.h>
#else
# undef AF_NETLINK
#endif

@bedevere-bot
Copy link

bedevere-bot commented Aug 9, 2023

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@corona10 corona10 self-assigned this Aug 9, 2023
@igalic
Copy link
Contributor Author

igalic commented Aug 9, 2023

Thanks, i missed that bit.
will try to get it fixed up today.

igalic and others added 2 commits August 9, 2023 18:02
@igalic
pythonGH-107812: extend socket's netlink support to FreeBSD
9cfc0ed 
on FreeBSD, netlink.h lives under netlink.
Extend the AC_HEADER_CHECK to look there as well.
@blurb-it @igalic
📜🤖 Added by blurb_it.
b36cc4c
@igalic
Copy link
Contributor Author

igalic commented Aug 9, 2023

I have made the requested changes; please review again!

@bedevere-bot
Copy link

bedevere-bot commented Aug 9, 2023

Thanks for making the requested changes!

@corona10: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from corona10 August 9, 2023 18:17
cjp256
cjp256 reviewed Aug 9, 2023
View reviewed changes
Copy link

@cjp256 cjp256 left a comment

Choose a reason for hiding this comment

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

awesome, thanks @igalic!

corona10
corona10 approved these changes Aug 10, 2023
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

@corona10 corona10 enabled auto-merge (squash) August 10, 2023 00:47
@corona10 corona10 merged commit f50c172 into python:main Aug 10, 2023
@igalic igalic deleted the fbsd/netlink branch August 10, 2023 11:34
vishwin pushed a commit to vishwin/freebsd-ports that referenced this pull request Feb 1, 2024
@igalic @vishwin
lang/python3.11: backport netlink patch
70c33d0 
this patch backports GH-107812: extend socket's netlink support to FreeBSD
see python/cpython#107813
for the patch.

see python/cpython#107812
for the discussion on the backport itself.

Sponsored by: The FreeBSD Foundation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corona10 corona10 corona10 approved these changes

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner

+1 more reviewer

@cjp256 cjp256 cjp256 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@corona10 corona10

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@igalic @bedevere-bot @cjp256 @corona10
X Tutup