Skip to content

Add support for vita#465

Merged
Thomasdezeeuw merged 3 commits into
rust-lang:masterfrom
pheki:vita
Oct 9, 2023
Merged

Add support for vita#465
Thomasdezeeuw merged 3 commits into
rust-lang:masterfrom
pheki:vita

Conversation

@pheki

@pheki pheki commented Sep 6, 2023

Copy link
Copy Markdown
Contributor

Needs rust-lang/libc#3284 to compile

@nikarh

nikarh commented Sep 6, 2023

Copy link
Copy Markdown
Contributor

I think socketaddr_in is also a bit different on Vita and we need to add sin_len and sin_vport fields under a cfg flag
NVM, I think it's related to 0.4.9 and hyper using it

@Thomasdezeeuw Thomasdezeeuw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm fine with merging this, but I don't think we can officially support it at this time.

Any way we can set up CI for this? Running cargo test or cargo check?

@pheki

pheki commented Sep 7, 2023

Copy link
Copy Markdown
Contributor Author

No official support is fine for me, it's a tier 3 platform after all :)

Any way we can set up CI for this? Running cargo test or cargo check?

AFAIK there's currently no way to run tests right now. Check works using:

cargo +nightly check -Z build-std=std --target armv7-sony-vita-newlibeabihf

So we could add check to Github CI using nightly and -Z build-std.

@Thomasdezeeuw

Copy link
Copy Markdown
Collaborator

No official support is fine for me, it's a tier 3 platform after all :)

Any way we can set up CI for this? Running cargo test or cargo check?

AFAIK there's currently no way to run tests right now. Check works using:

cargo +nightly check -Z build-std=std --target armv7-sony-vita-newlibeabihf

So we could add check to Github CI using nightly and -Z build-std.

That sounds good. You can copy what I removed in 14be1bc

@nikarh

nikarh commented Sep 8, 2023

Copy link
Copy Markdown
Contributor

Some tests are failing, because on vita there's no fcntl for non-blocking sockets, need to do it via setsockopt like in std:

setsockopt(fd, libc::SOL_SOCKET, libc::SO_NONBLOCK, nonblocking as libc::c_int)

@pheki pheki marked this pull request as draft September 9, 2023 15:12
@pheki

pheki commented Sep 9, 2023

Copy link
Copy Markdown
Contributor Author

Converted this PR to draft as we (vita-rust community) are still figuring some things out

@nikarh

nikarh commented Sep 23, 2023

Copy link
Copy Markdown
Contributor

At this point PR can be undrafted when rust-lang/libc#3284 is merged and released (we will need to bump libc version here for a green pipeline).

bors added a commit to rust-lang/libc that referenced this pull request Oct 6, 2023
Update crate version to 0.2.149

Contains fixes for net structs/calls for Vita target - #3284 and #3366. Required for rust-lang/socket2/pull/465
@nikarh

nikarh commented Oct 7, 2023

Copy link
Copy Markdown
Contributor

So, we're finally ready 😇

  • Added check and docs for VIta to CI pipeline
  • Nonblocking flag is set in via setsockopt and not fcntl on Vita (fcntl doesn't exist in Vita newlib, though at some point we probably will implement it least for that particular operation)
  • There is no ipv6 stack on Vita, so all tests using that are disabled for the target
  • All tests that are enabled for Vita are passing

Don't run tests for properties not available

Added CI, fixed tests
@pheki pheki marked this pull request as ready for review October 7, 2023 16:07

@Thomasdezeeuw Thomasdezeeuw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One change has to be reverted as it's unsound. The other comments are small points.

Other than that I think this on the way to get merged.

Comment thread src/lib.rs Outdated
Comment thread src/sys/unix.rs Outdated
Comment thread tests/socket.rs
Comment thread tests/socket.rs Outdated
Comment thread tests/socket.rs
Comment thread tests/socket.rs Outdated
Comment thread tests/socket.rs Outdated
@nikarh

nikarh commented Oct 9, 2023

Copy link
Copy Markdown
Contributor

Fixed all the comments, please check. With the API change I shouldn't have trusted clippy --fix, reverted and added clippy allow there.

@Thomasdezeeuw Thomasdezeeuw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, LGTM!

Also forgot to say that I like the CI setup.

@Thomasdezeeuw Thomasdezeeuw merged commit f61a788 into rust-lang:master Oct 9, 2023
@Thomasdezeeuw

Copy link
Copy Markdown
Collaborator

Thanks @nikarh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants