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.

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

Add redox-os to conditional compilation #[cfg] for ucred usage alongside other linux-like OS #5790

Merged
merged 3 commits into from Jun 13, 2023

Conversation

andrewdavidmackenzie
Copy link
Contributor

I recently got an MR merged into rust libc to add the ucred struct for redox-os.
That was what was missing in order to allow tokio to compile for redox-os.
That change was recently released as part of libc version 0.2.245.

This PR adds the redox-os to the conditional compile directives for the linux-like OSs, so that it can use ucred and compile without any tokio code changes.

This will unblock much useage (and fork and patch maintenance) in redox development for tokio, and allow it to be used much more widely.

It's still WIP as I work on an issue on my end, but wanted to get it here sooner rather than later - hope that is OK.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-net Module: tokio/net labels Jun 12, 2023
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

This looks good to me.

Since this is WIP, I will be marking this as a draft. Please remove the draft marker when you think this is ready.

tokio/src/net/unix/ucred.rs Outdated Show resolved Hide resolved
@Darksonn Darksonn marked this pull request as draft June 12, 2023 16:08
@andrewdavidmackenzie andrewdavidmackenzie changed the title WIP - Add redox-os to conditional compilation #[cfg] for ucred usage alongside other linux-like OS Add redox-os to conditional compilation #[cfg] for ucred usage alongside other linux-like OS Jun 12, 2023
@andrewdavidmackenzie andrewdavidmackenzie marked this pull request as ready for review June 12, 2023 16:29
@andrewdavidmackenzie
Copy link
Contributor Author

I don't understand the rustfmt check error. Is a change required to pass it?

@Darksonn
Copy link
Contributor

Yes, you'll need to reformat the file to pass the check. You can do that by executing rustfmt src/path/to/file. (Note that cargo fmt does not work in our repository, see CONTRIBUTING.md for more info.)

@Darksonn
Copy link
Contributor

Do you know if its possible to run our tests on redox OS in our CI setup?

@andrewdavidmackenzie
Copy link
Contributor Author

Re: "Do you know if its possible to run our tests on redox OS in our CI setup?"

Thanks for asking!

That is a definite goal for me.

I will ask our BDFL and resident experts to join in here, about how that can be done (I believe using the "redoxer" image on docker, but I am far from an expert on this matter in redox...)

@andrewdavidmackenzie
Copy link
Contributor Author

andrewdavidmackenzie commented Jun 12, 2023

AN initial compile check could be done using cargo check --target x86_64-unknown-redox, but to link, you need the redox linker, and probably the redoxer image is the path to do that.

We would really appreciate including that (fairly cheap?) check. Then linking and running correctly (which still requires some work) would be on redox for now. Later, maybe offering you an image as a path to link, run and pass tests on redox.

@andrewdavidmackenzie
Copy link
Contributor Author

OK, checks passed. Up to you now @Darksonn :-)

@andrewdavidmackenzie
Copy link
Contributor Author

@Darksonn would you like me to take a shot at a separate PR to add a cargo check check, for redox?

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Thanks.

@Darksonn Darksonn merged commit 00af6ef into tokio-rs:master Jun 13, 2023
55 checks passed
@Darksonn
Copy link
Contributor

Yes, if you're able to get a CI check working, then that would be great. We want to avoid breaking platforms once we get them working.

@andrewdavidmackenzie
Copy link
Contributor Author

I've looked through the ci.yml workflow.
I'm not sure where best to add it....

it's a fairly inexpensive check, but for a platform I wouldn't consider "top tier" yet.
It's just a cargo check (for now) not a full test, so doesn't fit with the tests on matrix.os there.

Add as a new (expensive) step - that runs on ubuntu (for now) and migrate to a redox entry in matrix.os at a later date when we have it running and tests passing?

Thanks for any pointers.

@Darksonn
Copy link
Contributor

If it's possible to run cargo test on Redox via an ubuntu worker, then that sounds like it would be great. As long as it isn't very slow.

@andrewdavidmackenzie
Copy link
Contributor Author

Not yet.

I am proposing a first step that runs cargo check --target=x86_64-unknown-redox that ensures it all compiles OK (linking is another mater).

@Darksonn
Copy link
Contributor

Okay. Let's do that, then.

@andrewdavidmackenzie
Copy link
Contributor Author

PR sent #5800 we can continue any related discussion over there.
Thanks for your collaboration!

crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Jul 6, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dependencies | minor | `1.28.2` -> `1.29.1` |
| [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dev-dependencies | minor | `1.28.2` -> `1.29.1` |

---

### Release Notes

<details>
<summary>tokio-rs/tokio (tokio)</summary>

### [`v1.29.1`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.29.1): Tokio v1.29.1

[Compare Source](tokio-rs/tokio@tokio-1.29.0...tokio-1.29.1)

##### Fixed

-   rt: fix nesting two `block_in_place` with a `block_on` between (#&#8203;5837])

#&#8203;5837]: tokio-rs/tokio#5837

### [`v1.29.0`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.29.0): Tokio v1.29.0

[Compare Source](tokio-rs/tokio@tokio-1.28.2...tokio-1.29.0)

Technically a breaking change, the `Send` implementation is removed from
`runtime::EnterGuard`. This change fixes a bug and should not impact most users.

##### Breaking

-   rt: `EnterGuard` should not be `Send` (#&#8203;5766])

##### Fixed

-   fs: reduce blocking ops in `fs::read_dir` (#&#8203;5653])
-   rt: fix possible starvation (#&#8203;5686], #&#8203;5712])
-   rt: fix stacked borrows issue in `JoinSet` (#&#8203;5693])
-   rt: panic if `EnterGuard` dropped incorrect order (#&#8203;5772])
-   time: do not overflow to signal value (#&#8203;5710])
-   fs: wait for in-flight ops before cloning `File` (#&#8203;5803])

##### Changed

-   rt: reduce time to poll tasks scheduled from outside the runtime (#&#8203;5705], #&#8203;5720])

##### Added

-   net: add uds doc alias for unix sockets (#&#8203;5659])
-   rt: add metric for number of tasks (#&#8203;5628])
-   sync: implement more traits for channel errors (#&#8203;5666])
-   net: add nodelay methods on TcpSocket (#&#8203;5672])
-   sync: add `broadcast::Receiver::blocking_recv` (#&#8203;5690])
-   process: add `raw_arg` method to `Command` (#&#8203;5704])
-   io: support PRIORITY epoll events (#&#8203;5566])
-   task: add `JoinSet::poll_join_next` (#&#8203;5721])
-   net: add support for Redox OS (#&#8203;5790])

##### Unstable

-   rt: add the ability to dump task backtraces (#&#8203;5608], #&#8203;5676], #&#8203;5708], #&#8203;5717])
-   rt: instrument task poll times with a histogram (#&#8203;5685])

#&#8203;5766]: tokio-rs/tokio#5766

#&#8203;5653]: tokio-rs/tokio#5653

#&#8203;5686]: tokio-rs/tokio#5686

#&#8203;5712]: tokio-rs/tokio#5712

#&#8203;5693]: tokio-rs/tokio#5693

#&#8203;5772]: tokio-rs/tokio#5772

#&#8203;5710]: tokio-rs/tokio#5710

#&#8203;5803]: tokio-rs/tokio#5803

#&#8203;5705]: tokio-rs/tokio#5705

#&#8203;5720]: tokio-rs/tokio#5720

#&#8203;5659]: tokio-rs/tokio#5659

#&#8203;5628]: tokio-rs/tokio#5628

#&#8203;5666]: tokio-rs/tokio#5666

#&#8203;5672]: tokio-rs/tokio#5672

#&#8203;5690]: tokio-rs/tokio#5690

#&#8203;5704]: tokio-rs/tokio#5704

#&#8203;5566]: tokio-rs/tokio#5566

#&#8203;5721]: tokio-rs/tokio#5721

#&#8203;5790]: tokio-rs/tokio#5790

#&#8203;5608]: tokio-rs/tokio#5608

#&#8203;5676]: tokio-rs/tokio#5676

#&#8203;5708]: tokio-rs/tokio#5708

#&#8203;5717]: tokio-rs/tokio#5717

#&#8203;5685]: tokio-rs/tokio#5685

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4wLjAiLCJ1cGRhdGVkSW5WZXIiOiIzNi4wLjAiLCJ0YXJnZXRCcmFuY2giOiJkZXZlbG9wIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1958
Reviewed-by: crapStone <crapstone01@gmail.com>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-net Module: tokio/net
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants