Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Enable Linux and BSD compatibility #49

Closed
wants to merge 4 commits into from

Conversation

lamaral
Copy link
Contributor

@lamaral lamaral commented Jun 3, 2021

Hey guys, first of all, great work on the framework.

I need to use the framework on FreeBSD servers and it was quite a boomer when I saw it was depending on epoll, so I took some time and came up with this solution using selectors instead of select, that way things got OS agnostic and selectors should in theory use the most efficient method mechanism available.

I took the care of also adjusting the poll_mock() in the tests. Please double check the work there as I was slightly unsure if that was the right way to go.

Luiz

@facebook-github-bot
Copy link

Hi @lamaral!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@skozlov404
Copy link

Hi @lamaral!

Thanks for your contribution, I'll take a closer look next week. :)

@skozlov404 skozlov404 mentioned this pull request Jun 4, 2021
@skozlov404
Copy link

Okay, I think the diff is good, I just need you to test this on both FreeBSD and Linux manually before I can approve it. Please save and attach the console output. Please perform simple 1MB file download using stock system tftp clients.

Also - do you plan on creating a port for FreeBSD?

@lamaral
Copy link
Contributor Author

lamaral commented Jun 8, 2021

Nice :)
I'll come back with the console outputs by the end of the week.

Creating a public port for FreeBSD wasn't really on the plan, since we have our own internal ports tree, but shouldn't be too hard, since we already have it internally. I'll have a more careful look at it on the weekend.

@lamaral
Copy link
Contributor Author

lamaral commented Jun 14, 2021

Hi Sergey,
Here are the console outputs. Let me know if they are not what you expected.

freebsd.log
linux.log

I checked the process to create a FreeBSD port, but unfortunately I can't afford the time to do it currently. Our internal ports are done with some extra magic, so they can't be easily turned into a public port.

@facebook-github-bot
Copy link

@skozlov404 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@skozlov404
Copy link

Thanks a lot, @lamaral. The diff is good to land now, let me just guide it through the internal approval process, and it will close automatically

@facebook-github-bot
Copy link

@skozlov404 merged this pull request in 65fd535.

@skozlov404
Copy link

All done, thanks again @lamaral for your contribution! :) I'm gonna pack a new version for PyPI soon.

@skozlov404
Copy link

I've packed the new version, you can now create the port (if you're still want to do it) :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants