Conversation
|
r? @ibraheemdev rustbot has assigned @ibraheemdev. Use |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
|
||
| #[test] | ||
| #[cfg_attr(target_os = "android", ignore)] // Android SELinux rules prevent creating Unix sockets | ||
| #[cfg_attr(any(target_os = "vxworks", target_os = "android"), ignore)] |
There was a problem hiding this comment.
Can you make these look like this, with some reasoning for why this test is ignored on vxworks (is there a fundamental reason, is it just not implemented yet, etc)?
| #[cfg_attr(any(target_os = "vxworks", target_os = "android"), ignore)] | |
| #[cfg_attr(target_os = "android", ignore = "Android SELinux rules prevent creating Unix sockets")] | |
| #[cfg_attr(target_os = "vxworks", ignore = "...")] |
| #[cfg(all(not(target_os = "vxworks"),unix))] | ||
| error!(result, "Not a directory"); | ||
| #[cfg(target_os = "vxworks")] | ||
| error!(result, "not a directory"); |
There was a problem hiding this comment.
This is cleaner as a cfg_select!, e.g.
| #[cfg(all(not(target_os = "vxworks"),unix))] | |
| error!(result, "Not a directory"); | |
| #[cfg(target_os = "vxworks")] | |
| error!(result, "not a directory"); | |
| cfg_select! { | |
| target_os = "vxworks" => { | |
| error!(result, "not a directory"); | |
| } | |
| unix => { | |
| error!(result, "Not a directory"); | |
| } | |
| windows => { | |
| error!(result, 267); // ERROR_DIRECTORY - The directory name is invalid. | |
| } | |
| _ => { | |
| // all good. | |
| } | |
| } |
There was a problem hiding this comment.
Do vxworks errors typically have different capitalization than the rest of Unix? Seems like a weird thing to cfg on
This description restates in four different ways that it improves vxworks support, but gives no indication of what it changes or why that is. Please make sure it’s easy for reviewers to understand what is going on, even “Ignore X-related tests that currently fail on VxWorks” is more useful. Pinging target maintainer @biabbas for review |
There was a problem hiding this comment.
- The change to the error string in concurrent recursive mkdir test is a valid change. But it is better done in the suggested way.
- Af_unix mechanism is not supported on vxworks. Hence the Unix datagram tests should be ignored. Specifying this reason in the cfg attribute as suggested would be better. Also, make sure the reason specified for the android selinux target is preserved.
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
|
☔ The latest upstream changes (presumably #146282) made this pull request unmergeable. Please resolve the merge conflicts. |
Summary
This PR improves support for the
vxworkstarget in the Rust standard library by fixing platform-specific issues and ensuring better compatibility with VxWorks.These changes are isolated to
vxworksand do not affect other platforms.Notes
Tested manually on a VxWorks environment. Feedback on potential edge cases or additional tests would be appreciated.