-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
port rust for vxWorks #61946
port rust for vxWorks #61946
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @rkruppe (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
I don't think I can properly review this, let alone feel qualified to approve the addition of several big new targets. r? @alexcrichton comes to mind. I can tell you though that it would likely be better to make smaller, incremental pull requests that e.g. add compiler triples before diving into standard library support (like the CloudABI-related pull requests for example). |
Thanks for the PR @BaoshanPang and wow does this look extensive! I'm probably the best person to review this but I'll be unfortunately traveling for the rest of this week and the next, so I won't have a chance to really dig into this until early July. In the meantime though others are of course welcome to either take over or add their own review! In the meantime though some suggestions and questions I might have are:
I'll try to make sure I get to this in early July regardless though! |
@alexcrichton
The targets added in rust for vxWorks are mapping to vxWorks board support package(BSP) which are the code that make vxWorks can run on that board(s). For example the target arm-wrs-vxworks is mapping to BSP qsp_arm, with this target specfied in rustc command line, the rust code would be built into a RTP program which is the user space application on vxWorks.
That's what I am hoping for. I am still working on to get rid of all failures in running test suites.
Thanks for the information, I will rebase and rebuild my code. should I cancel this PR and create a different PR after rebasing and rebuilding? Regards, |
@BaoshanPang no, keep using this PR, unless you wanna follow @rkruppe advice and break it into smaller PRs. |
@BaoshanPang yes keeping this PR is fine. I personally prefer big PRs myself as it makes it easier to review and handle feedback, but I don't mind reviewing whatever strategy you feel is best for you to land this in rust-lang/rust. FWIW it may also be easiest/best to not check in changes to the test suite yet. We could just check in rustc and libstd changes, and then follow-up with the test suite if still necessary. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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.
Ok got a chance to look over this, and it looks pretty good! I've got some comments on the integration points but they're pretty minor.
There's quite a few changes to tests in terms of new ignore annotations, and I wonder if it's perhaps best to hold off on those for now? They're likely to bitrot very quickly and otherwise are somewhat cumbersome to deal with, so perhaps just the libstd pieces can be added for now?
I didn't review the src/libstd/sys/vxworks
folder, but if it's a copy of src/libstd/sys/unix
it can likely be trimmed down quite a lot since unix
handles so many platforms in one.
src/librustdoc/html/render.rs
Outdated
@@ -1352,6 +1352,7 @@ fn write_minify_replacer<W: Write>( | |||
.into(); | |||
tokens.apply(|f| { | |||
// We add a backline after the newly created variables. | |||
|
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.
Stray change?
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.
(ping on this diff, I think this is accidental?)
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 don't understand where the difference comes from. I didn't change the file, and there is no log showing I changed it:
https://github.com/BaoshanPang/rust/commits/vxworks/src/librustdoc/html/render.rs
☔ The latest upstream changes (presumably #62335) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably #62458) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
This is looking really good, thanks @BaoshanPang! The main thing I'm still somewhat wary about is the maintenance of all the test ignore annotations all throughout the test suite. Are all the tests working for you locally? Is there anything else blocking this?
Not all test cases are passing, I am still trying to make them works though. About 10 test cases are still fail in test suite run-pass, they are all hard to fix from my understanding, we may want to declare they(like backtrace, out of stack etc) are not supported yet at this point. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Ok, if that's the case could the test annotations be backed out for now? I'd be interested in better supporting them eventually but for now it may be best to trim down this PR |
Sure, I have backed out all changes in test cases for vxWorks. |
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.
Looks great to me! r=me with a few final nits
I think there's still a stray change in |
It seems the change in render.rs was made by GuillaumeGomez, but for some reason it is counted as commited by me. I squashed all my commites and the one commit made by GuillaumeGomez in order to let my branch has only one commit. Please let me know if there is anything else I need to do for the PR. Thanks, |
@bors: r+ Looks good to me now, thanks @BaoshanPang! |
📌 Commit 012e5963cf11c6ad2be7c12bfd38d6e0d36f4dad has been approved by |
@bors p=1 |
⌛ Testing commit 012e5963cf11c6ad2be7c12bfd38d6e0d36f4dad with merge 40d495671a54189e0314468d1d42427c957fab53... |
💔 Test failed - checks-azure |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@alexcrichton I fixed it now. |
@bors: r+ |
📌 Commit 4c0c0f6 has been approved by |
port rust for vxWorks The supporting for vxWorks has been enabled in this branch. Although there are still a lots of work to do, I would like to upstream the code and fix the problems later. Please let me know if there is anything I have to do before upstream the code. r? @alexcrichton Thanks, Baoshan
☀️ Test successful - checks-azure |
The supporting for vxWorks has been enabled in this branch. Although there are still a lots of work to do, I would like to upstream the code and fix the problems later.
Please let me know if there is anything I have to do before upstream the code.
r? @alexcrichton
Thanks,
Baoshan