-
Notifications
You must be signed in to change notification settings - Fork 533
Extend QNX support by adding all currently supported variants #3723
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
base: main
Are you sure you want to change the base?
Conversation
Add support for the various (apparently ABI incompatible) versions of QNX that are supported by Rust 1.91+ (QNX 7, QNX 7 with iopkt, QNX8).
Since dylibs are supported by QNX, let's not refuse them in case they were added to the rustc command line.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This is required for backwards compatibility, so that builds that relied on this being the default before version introduction will still work.
With the advent of QNX7 support with iosock, we need at least Rust 1.86.0 so that the new target is understood. Bump the version accordingly.
UebelAndre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I had one comment though and think it will delay this change.
| constraint_setting = ":wasi_version", | ||
| ) | ||
|
|
||
| constraint_setting( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would actually like to avoid adding constraint values like this into rules_rust. I've been seeing more activity on bazelbuild/platforms#38 and want to use whatever pattern comes from this issue for constraints like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@UebelAndre Fine for me, I would then simply apply this as a patch for our org as long as there's no decision on the mentioned issue. However, I don't feel like I can contribute much there due to my lack of the overall picture and how things should be. May I ask you to put a comment in the issue about how to move forward and come to a conclusion over there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@UebelAndre since the activity in the mentioned ticket was stalled for 3 years, is there a chance that we say ie, if nothing moves during next month we can try continue in here ? Because rules_rust are important part of bazel and QNX8 is more and more used target - it would make sense to have support for it im my opinion. Otherwise it can happen bazelbuild/platforms#38 will block this next 3 years.
Add support for QNX8 by introducing a version constraint that allows for the selection of the respective ABI target of the Rust compiler. Since QNX also supports dylibs, also include the nto system as dylib supporting platform.
In addition, also add all supported targets to the list of Tier 3 architectures. Since cargo-bazel also needs to know the additional triples, it's version of cfg-expr needs to be bumped.