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

Support QNX 7.1 with io-sock+libstd and QNX 8.0 (no_std only) #133631

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

flba-eb
Copy link
Contributor

@flba-eb flba-eb commented Nov 29, 2024

Changes of this pull request:

  1. Refactor code for qnx nto targets to share more code in file nto_qnx.rs

  2. Add support for an additional network stack on nto qnx 7.1.

    QNX 7.1 supports two network stacks:

    1. io-pkt, which is default
    2. io-sock, which is optional on 7.1 but default in QNX 8.0

    As one can see in the io-sock migration notes, this changes the libc API in a way similar to e.g. linux-gnu vs. linux-musl.

    This change adds a new target which has a different value for target_env, so that e.g. libc can distinguish between both APIs.

  3. Add initial support for QNX 8.0, thanks to AkhilTThomas. As it turned out, the problem with forking many processes still exists in QNX 8.0. Because if this, we are now using it for any QNX version (i.e. not check for target_env anymore).

@rustbot label +O-neutrino
CC: @jonathanpallant @japaric @gh-tr @AkhilTThomas

@rustbot
Copy link
Collaborator

rustbot commented Nov 29, 2024

r? @pnkfelix

rustbot has assigned @pnkfelix.
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

@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. O-neutrino OS: QNX Neutrino, a POSIX-compatible real-time operating system labels Nov 29, 2024
@rust-log-analyzer

This comment has been minimized.

@flba-eb flba-eb force-pushed the add_nto_qnx71_iosock_support branch from 96a91e2 to 6116b6a Compare November 29, 2024 14:13
@rust-log-analyzer

This comment has been minimized.

@jonathanpallant
Copy link
Contributor

So, first up, excellent! The AWS QNX Neutrino 7.1.0 AMIs appears to use iosock and so I have to keep copying over libsocket.so.3 to run Rust code there. With this change hopefully I won't.

However, I note that there are currently no five-part-triples in the target list, so this would be the first. Perhaps aarch64-unknown-nto-qnx710_iosock would be better.

Will there be a x86_64-unknown-nto-qnx710_iosock to match?

@flba-eb

This comment was marked as resolved.

@flba-eb
Copy link
Contributor Author

flba-eb commented Nov 29, 2024

So, first up, excellent! The AWS QNX Neutrino 7.1.0 AMIs appears to use iosock and so I have to keep copying over libsocket.so.3 to run Rust code there. With this change hopefully I won't.

Beware that version 3 and 4 have different APIs -- they may seem to work, but there might be subtle issues up to undefined behavior.

However, I note that there are currently no five-part-triples in the target list, so this would be the first. Perhaps aarch64-unknown-nto-qnx710_iosock would be better.

Very good idea -- I found it confusing that target_env uses an underscore while the target name does not. I will rename it!

Will there be a x86_64-unknown-nto-qnx710_iosock to match?

Not planned so far, but should be quite easy to do. How big is the need for it?

@jonathanpallant
Copy link
Contributor

How big is the need for it?

No idea. Who was asking for this target?

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@flba-eb

This comment was marked as resolved.

@jonathanpallant
Copy link
Contributor

I was thinking to use an obvious dummy value rather than panicking if the environment variable isn't set. But maybe the project people have a better idea.

Are there another other targets that need to read environment variables?

@flba-eb

This comment was marked as resolved.

@flba-eb flba-eb force-pushed the add_nto_qnx71_iosock_support branch from 5051c34 to 00079dc Compare November 29, 2024 15:42
@rust-log-analyzer

This comment has been minimized.

@lcnr lcnr mentioned this pull request Nov 29, 2024
@flba-eb flba-eb force-pushed the add_nto_qnx71_iosock_support branch 6 times, most recently from 72b6642 to 32310c3 Compare November 29, 2024 17:38
@bors
Copy link
Contributor

bors commented Jan 24, 2025

☔ The latest upstream changes (presumably #135978) made this pull request unmergeable. Please resolve the merge conflicts.

@flba-eb flba-eb force-pushed the add_nto_qnx71_iosock_support branch from 08bc749 to 23e1f42 Compare January 24, 2025 12:38
flba-eb and others added 2 commits January 24, 2025 12:41
@flba-eb flba-eb force-pushed the add_nto_qnx71_iosock_support branch from 23e1f42 to 48850f1 Compare January 24, 2025 12:42
@flba-eb
Copy link
Contributor Author

flba-eb commented Jan 24, 2025

Fixed the conflict with 23e1f42 and added the comment with 48850f1 as suggested in #133631 (comment).
@workingjubilee : With almost 100 comments, I'm losing the overview. Is there anything left for us to do?

flba-eb and others added 2 commits January 24, 2025 12:51
QNX SDP 8.0 comes with newly renamed QNX OS 8.0, so update the page to talk about QNX, QNX Neutrino 7.0, QNX Neutrino 7.1 or QNX OS 8.0.

Also actually add a list of target triples.
@flba-eb flba-eb force-pushed the add_nto_qnx71_iosock_support branch from 48850f1 to 0462826 Compare January 24, 2025 12:51
@madsmtm
Copy link
Contributor

madsmtm commented Jan 24, 2025

Is there anything left for us to do?

Since this is adding (4) new targets, I think you procedurally have to fill out the Tier 3 target policy (probably by editing the top-level PR comment).

@jonathanpallant
Copy link
Contributor

I don't know if it just happened in the rebases, but there's a typo in the git commit message:

Add new target for supporting Neutrino QNX 6.1 with io-socket network stack on aarch64

I had to double-take because I thought you'd added a fifth target to this PR 🤣

@madsmtm
Copy link
Contributor

madsmtm commented Jan 24, 2025

Lesson learned: Define new target first, with minimal (or no) other changes, and create a PR for it. All other changes like refactoring and documentation should came later and separately.

Well I think it needs to go into the platform docs, but you should mark it as having only libcore support (because libstd requires cc-rs and libc and a bunch of other updates that can't happen until they know about the target).

I've opened #135992 to fix that.

@workingjubilee
Copy link
Member

My last material question was #133631 (comment) which was because I thought I might have had an idea for how we could do without the additional target definition. Alas, it is not so.

Anyway, these maintainers know what the policy is. The PR description does not need a bunch of additional noise.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 24, 2025

📌 Commit 0462826 has been approved by workingjubilee

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 24, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2025
…rt, r=workingjubilee

Support QNX 7.1 with `io-sock`+libstd and QNX 8.0 (`no_std` only)

Changes of this pull request:

1. Refactor code for qnx nto targets to share more code in file `nto_qnx.rs`
1. Add support for an additional network stack on nto qnx 7.1.

   QNX 7.1 supports two network stacks:

   1. `io-pkt`, which is default
   2. `io-sock`, which is optional on 7.1 but default in QNX 8.0

   As one can see in the [io-sock migration notes](https://www.qnx.com/developers/docs/7.1/index.html#com.qnx.doc.neutrino.io_sock/topic/migrate_app.html), this changes the libc API in a way similar to e.g. linux-gnu vs. linux-musl.

   This change adds a new target which has a different value for `target_env`, so that e.g. libc can distinguish between both APIs.

2. Add initial support for QNX 8.0, thanks to AkhilTThomas. As it turned out, the problem with forking many processes still exists in QNX 8.0. Because if this, we are now using it for any QNX version (i.e. not check for `target_env` anymore).
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#133631 (Support QNX 7.1 with `io-sock`+libstd and QNX 8.0 (`no_std` only))
 - rust-lang#133951 (Make the wasm_c_abi future compat warning a hard error)
 - rust-lang#134283 (fix(libtest): Deprecate '--logfile')
 - rust-lang#134679 (Windows: remove readonly files)
 - rust-lang#135635 (Move `std::io::pipe` code into its own file)
 - rust-lang#135842 (TRPL: more backward-compatible Edition changes)
 - rust-lang#135953 (ci.py: check the return code in `run-local`)
 - rust-lang#136031 (Expand polonius MIR dump)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#133631 (Support QNX 7.1 with `io-sock`+libstd and QNX 8.0 (`no_std` only))
 - rust-lang#133951 (Make the wasm_c_abi future compat warning a hard error)
 - rust-lang#134283 (fix(libtest): Deprecate '--logfile')
 - rust-lang#134679 (Windows: remove readonly files)
 - rust-lang#135635 (Move `std::io::pipe` code into its own file)
 - rust-lang#135842 (TRPL: more backward-compatible Edition changes)
 - rust-lang#135953 (ci.py: check the return code in `run-local`)
 - rust-lang#136031 (Expand polonius MIR dump)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2025
…rt, r=workingjubilee

Support QNX 7.1 with `io-sock`+libstd and QNX 8.0 (`no_std` only)

Changes of this pull request:

1. Refactor code for qnx nto targets to share more code in file `nto_qnx.rs`
1. Add support for an additional network stack on nto qnx 7.1.

   QNX 7.1 supports two network stacks:

   1. `io-pkt`, which is default
   2. `io-sock`, which is optional on 7.1 but default in QNX 8.0

   As one can see in the [io-sock migration notes](https://www.qnx.com/developers/docs/7.1/index.html#com.qnx.doc.neutrino.io_sock/topic/migrate_app.html), this changes the libc API in a way similar to e.g. linux-gnu vs. linux-musl.

   This change adds a new target which has a different value for `target_env`, so that e.g. libc can distinguish between both APIs.

2. Add initial support for QNX 8.0, thanks to AkhilTThomas. As it turned out, the problem with forking many processes still exists in QNX 8.0. Because if this, we are now using it for any QNX version (i.e. not check for `target_env` anymore).
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 26, 2025
…rt, r=workingjubilee

Support QNX 7.1 with `io-sock`+libstd and QNX 8.0 (`no_std` only)

Changes of this pull request:

1. Refactor code for qnx nto targets to share more code in file `nto_qnx.rs`
1. Add support for an additional network stack on nto qnx 7.1.

   QNX 7.1 supports two network stacks:

   1. `io-pkt`, which is default
   2. `io-sock`, which is optional on 7.1 but default in QNX 8.0

   As one can see in the [io-sock migration notes](https://www.qnx.com/developers/docs/7.1/index.html#com.qnx.doc.neutrino.io_sock/topic/migrate_app.html), this changes the libc API in a way similar to e.g. linux-gnu vs. linux-musl.

   This change adds a new target which has a different value for `target_env`, so that e.g. libc can distinguish between both APIs.

2. Add initial support for QNX 8.0, thanks to AkhilTThomas. As it turned out, the problem with forking many processes still exists in QNX 8.0. Because if this, we are now using it for any QNX version (i.e. not check for `target_env` anymore).
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang#133631 (Support QNX 7.1 with `io-sock`+libstd and QNX 8.0 (`no_std` only))
 - rust-lang#134358 (compiler: Set `target_abi = "ilp32e"` on all riscv32e targets)
 - rust-lang#135764 (Fix tests on LLVM 20)
 - rust-lang#135812 (Fix GDB `OsString` provider on Windows )
 - rust-lang#135842 (TRPL: more backward-compatible Edition changes)
 - rust-lang#135946 (Remove extra whitespace from rustdoc breadcrumbs for copypasting)
 - rust-lang#135953 (ci.py: check the return code in `run-local`)
 - rust-lang#136019 (Add an `unchecked_div` alias to the `Div<NonZero<_>>` impls)

Failed merges:

 - rust-lang#136037 (Mark all NuttX targets as tier 3 target and support the standard library)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 26, 2025
…rt, r=workingjubilee

Support QNX 7.1 with `io-sock`+libstd and QNX 8.0 (`no_std` only)

Changes of this pull request:

1. Refactor code for qnx nto targets to share more code in file `nto_qnx.rs`
1. Add support for an additional network stack on nto qnx 7.1.

   QNX 7.1 supports two network stacks:

   1. `io-pkt`, which is default
   2. `io-sock`, which is optional on 7.1 but default in QNX 8.0

   As one can see in the [io-sock migration notes](https://www.qnx.com/developers/docs/7.1/index.html#com.qnx.doc.neutrino.io_sock/topic/migrate_app.html), this changes the libc API in a way similar to e.g. linux-gnu vs. linux-musl.

   This change adds a new target which has a different value for `target_env`, so that e.g. libc can distinguish between both APIs.

2. Add initial support for QNX 8.0, thanks to AkhilTThomas. As it turned out, the problem with forking many processes still exists in QNX 8.0. Because if this, we are now using it for any QNX version (i.e. not check for `target_env` anymore).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-neutrino OS: QNX Neutrino, a POSIX-compatible real-time operating system O-unix Operating system: Unix-like relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants