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

Revise network definitions for HorizonOS #3863

Merged
merged 1 commit into from
Sep 6, 2024
Merged

Conversation

Meziu
Copy link
Contributor

@Meziu Meziu commented Aug 21, 2024

@ian-h-chamberlain @FenrirWolf @adryzz

Following some discussion in rust3ds/ctru-rs#174.

The changes include:

  • Update to the hostent definition for HorizonOS (armv6k-nintendo-3ds) following upstream changes.

  • A step back on the decision to remove the trailing zero bytes from sockaddr_in that was made all the way back in Horizon OS Nintendo 3DS - Fixed size of sockaddr_in #2725. This change will require a little work in std, but we came to understand it's only right to keep the definitions true to the upstream versions.

Also, should this PR be flagged for release in 0.2.x as well as the 1.0 milestone? Work on tier-3 targets isn't clearly talked about in the new contribution guidelines (not that this PR is of any relevance to the release process).

@rustbot
Copy link
Collaborator

rustbot commented Aug 21, 2024

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Sep 1, 2024
@tgross35
Copy link
Contributor

tgross35 commented Sep 1, 2024

Thanks for the fix! Looks like this is defined here https://libctru.devkitpro.org/in_8h_source.html. Can you verify that tests pass with ./ci/run.sh relevant-target when checking Horizon?

Also, should this PR be flagged for release in 0.2.x as well as the 1.0 milestone? Work on tier-3 targets isn't clearly talked about in the new contribution guidelines (not that this PR is of any relevance to the release process).

You are welcome to stable nominate anything (the label works via rustbot). I don't mind backporting as long as it's not overly conflicty.

src/unix/newlib/mod.rs Outdated Show resolved Hide resolved
@tgross35
Copy link
Contributor

tgross35 commented Sep 1, 2024

Also, do you have a link to the relevant headers where hostent is defined? Trying to get these crosslinked in all PRs.

@rustbot author

@FenrirWolf
Copy link
Contributor

FenrirWolf commented Sep 1, 2024

hostent is in https://libctru.devkitpro.org/netdb_8h_source.html

@Meziu
Copy link
Contributor Author

Meziu commented Sep 1, 2024

Thanks for the fix! Looks like this is defined here https://libctru.devkitpro.org/in_8h_source.html. Can you verify that tests pass with ./ci/run.sh relevant-target when checking Horizon?

The test (rightfully) panics at this line because our target has no specified tests. I will add that our target isn't a host target (it's basically an embedded device) and that cross-compiling and running tests is a problem we've had for quite a long time due to the very weird setup with the required toolchain libraries.

Is it necessary to implement these tests? I might be able to cook up something that checks out the headers (assuming the host has all the libraries installed), but it would serve little purpose as the toolchain used is (and will always be) fundamentally experimental.

@tgross35
Copy link
Contributor

tgross35 commented Sep 2, 2024

Thanks for the change, looks good now! Could you please squash?

Regarding the tests, no need to do anything here. At some point in the future you are welcome to add a similar function, but it doesn't need to happen now.

@tgross35 tgross35 added this pull request to the merge queue Sep 6, 2024
Merged via the queue into rust-lang:main with commit 37f0e1e Sep 6, 2024
41 checks passed
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Sep 6, 2024
(backport <rust-lang#3863>)
(cherry picked from commit 82ebf14)
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants