Skip to content
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

Expose sched_yield on non-linux-like hosts #1090

Merged
merged 3 commits into from
Jul 3, 2019
Merged

Expose sched_yield on non-linux-like hosts #1090

merged 3 commits into from
Jul 3, 2019

Conversation

kubkon
Copy link
Contributor

@kubkon kubkon commented Jun 24, 2019

Currently, sched module is compiled only for linux-like hosts. However, the recently added syscall sched_yield is universal to all *nixes. This PR pulls all linux-like functionality into a private sched_linux_like module which is enabled only for android and linux hosts, while sched_yield and sched modules are now compiled for all *nixes, thus, correctly exposing sched_yield to other non-linux hosts.

@kubkon kubkon force-pushed the sched_yield_fix branch from 255e897 to e2fde19 Compare June 24, 2019 22:12
@asomers
Copy link
Member

asomers commented Jun 25, 2019

It looks like you're doing two separate things in a single commit.

  1. Enabling sched_yield on more platforms
  2. Refactoring the sched without change in functionality to move some stuff into a submodule.

Would you mind separating those two separate things into two commits? That will make this PR easier to review, and it will make the history clearer. Also, you're going to need a CHANGELOG entry.

@kubkon
Copy link
Contributor Author

kubkon commented Jun 25, 2019

It looks like you're doing two separate things in a single commit.

1. Enabling sched_yield on more platforms

2. Refactoring the sched without change in functionality to move some stuff into a submodule.

Would you mind separating those two separate things into two commits? That will make this PR easier to review, and it will make the history clearer. Also, you're going to need a CHANGELOG entry.

No probs! Will do! :-)

@kubkon kubkon force-pushed the sched_yield_fix branch from e2fde19 to 60d89b8 Compare June 25, 2019 08:01
asomers
asomers reviewed Jun 25, 2019
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

LGTM, apart from the extra changelog entry.

CHANGELOG.md Outdated Show resolved Hide resolved
@kubkon kubkon force-pushed the sched_yield_fix branch from 3bf82cf to 417f1d8 Compare June 25, 2019 13:34
@kubkon
Copy link
Contributor Author

kubkon commented Jun 26, 2019

LGTM, apart from the extra changelog entry.

@asomers Changelog is now fixed. Lemme know if there's anything else! :-)

asomers
asomers approved these changes Jun 26, 2019
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

bors r+

@kubkon
Copy link
Contributor Author

kubkon commented Jun 26, 2019 •
edited
Loading

Hmm, is it just me, or did bors get somehow stuck? @asomers thoughts?

@asomers
Copy link
Member

asomers commented Jul 1, 2019

I don't know what the problem was, but bors is fine now. However, there's been an additional problem that you'll have to rebase to fix, or else the tests will fail.

@kubkon kubkon force-pushed the sched_yield_fix branch from 417f1d8 to b9a21ab Compare July 1, 2019 06:03
@kubkon
Copy link
Contributor Author

kubkon commented Jul 1, 2019

I don't know what the problem was, but bors is fine now. However, there's been an additional problem that you'll have to rebase to fix, or else the tests will fail.

Done!

asomers
asomers approved these changes Jul 1, 2019
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot added a commit that referenced this pull request Jul 1, 2019
@bors
Merge #1090
426451f 
1090: Expose sched_yield on non-linux-like hosts r=asomers a=kubkon

Currently, `sched` module is compiled only for linux-like hosts. However, the recently added syscall `sched_yield` is universal to all *nixes. This PR pulls all linux-like functionality into a private `sched_linux_like` module which is enabled only for android and linux hosts, while `sched_yield` and `sched` modules are now compiled for all *nixes, thus, correctly exposing `sched_yield` to other non-linux hosts.

Co-authored-by: Jakub Konka <jakub.konka@golem.network>
@bors
Copy link
Contributor

bors bot commented Jul 1, 2019

Timed out

@asomers
Copy link
Member

asomers commented Jul 2, 2019

bors retry

bors bot added a commit that referenced this pull request Jul 2, 2019
@bors
Merge #1090
b23b77e 
1090: Expose sched_yield on non-linux-like hosts r=asomers a=kubkon

Currently, `sched` module is compiled only for linux-like hosts. However, the recently added syscall `sched_yield` is universal to all *nixes. This PR pulls all linux-like functionality into a private `sched_linux_like` module which is enabled only for android and linux hosts, while `sched_yield` and `sched` modules are now compiled for all *nixes, thus, correctly exposing `sched_yield` to other non-linux hosts.

Co-authored-by: Jakub Konka <jakub.konka@golem.network>
@bors
Copy link
Contributor

bors bot commented Jul 2, 2019

Timed out

@asomers
Copy link
Member

asomers commented Jul 3, 2019

Travis and Cirrus seem to be fine. I don't know why bors isn't working. Third try's a charm?

bors retry

bors bot added a commit that referenced this pull request Jul 3, 2019
@bors
Merge #1090
112438b 
1090: Expose sched_yield on non-linux-like hosts r=asomers a=kubkon

Currently, `sched` module is compiled only for linux-like hosts. However, the recently added syscall `sched_yield` is universal to all *nixes. This PR pulls all linux-like functionality into a private `sched_linux_like` module which is enabled only for android and linux hosts, while `sched_yield` and `sched` modules are now compiled for all *nixes, thus, correctly exposing `sched_yield` to other non-linux hosts.

Co-authored-by: Jakub Konka <jakub.konka@golem.network>
@bors
Copy link
Contributor

bors bot commented Jul 3, 2019

Build succeeded

@bors bors bot merged commit b9a21ab into nix-rust:master Jul 3, 2019
@kubkon kubkon deleted the sched_yield_fix branch July 3, 2019 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asomers asomers asomers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kubkon @asomers