std: sys: pal: uefi: os: Implement split_paths#151991
std: sys: pal: uefi: os: Implement split_paths#151991rust-bors[bot] merged 2 commits intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-review
|
r? @ChrisDenton rustbot has assigned @ChrisDenton. Use |
This comment has been minimized.
This comment has been minimized.
Ayush1325
force-pushed
the
uefi-split-path
branch
from
2ac9b50 to
8cc279c
Compare
There was a problem hiding this comment.
It would be good to add a split_paths_uefi test in this file:
Line 27 in 286fbe5
library/std/src/sys/pal/uefi/os.rs
Outdated
| pub fn split_paths(_unparsed: &OsStr) -> SplitPaths<'_> { | ||
| panic!("unsupported") | ||
| pub fn split_paths(unparsed: &OsStr) -> SplitPaths<'_> { | ||
| SplitPaths(unparsed.encode_wide()) |
There was a problem hiding this comment.
Using the split_paths_unix and split_paths_windows tests for comparison, I noticed an issue with the way the UEFI implementation handles empty paths. The Unix and Windows implementations map "" to [""] and ";;" to ["", "", ""], but the UEFI implementation returns an empty iterator for both. We should probably make the UEFI behavior match Unix/Windows here.
There was a problem hiding this comment.
Fixed. Handling things the same way as windows, just without the quote escaping support.
Ayush1325
force-pushed
the
uefi-split-path
branch
from
8cc279c to
eb41d36
Compare
Added. |
This comment has been minimized.
This comment has been minimized.
- Based on Windows implementation. Just removed support for quote escaping since that is not supported in UEFI. - Tested using OVMF on QEMU Signed-off-by: Ayush Singh <ayush@beagleboard.org>
- Add test for split_paths for UEFI target. - `;` is the path separator. Escaping is not supported. Signed-off-by: Ayush Singh <ayush@beagleboard.org>
Ayush1325
force-pushed
the
uefi-split-path
branch
from
93ecb51 to
c37e448
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
I know this isn't your fault (the Windows implementation has the same issue), but I really don't think roundtripping to UTF-16 is a good idea. Perhaps you could find an algorithm that works on the |
|
Though I guess it's easier to do both in another PR. Let's merge this first... |
rust-bors
bot
added
S-waiting-on-bors
std: sys: pal: uefi: os: Implement split_paths - Tested using OVMF on QEMU @rustbot label +O-UEFI cc @nicholasbishop
To clarify, is your concern here about correctness, or performance? (Or both?) |
|
Oh, just performance – otherwise I wouldn't have r+ed. |
…uwer Rollup of 9 pull requests Successful merges: - #146989 (Inhibit all-absent-variant optimization for all enum reprs that inhibit layout optimization, not just repr(C).) - #151991 (std: sys: pal: uefi: os: Implement split_paths) - #152794 (Fix ICE in `try_to_raw_bytes` when array elements have mismatched) - #153052 (std random.rs: update link to RTEMS docs) - #153054 (docs: note env var influence on `temp_dir` and `env_clear` on Windows) - #153061 (cleanup `tests/ui/box`, part 2) - #153197 (style: Update doctests for `TryFrom<integer> for bool` and `From<bool> for float`) - #153210 (Fix ICE on empty file with -Zquery-dep-graph) - #153228 (Remove `TranslationError`)
Rollup merge of #151991 - Ayush1325:uefi-split-path, r=joboet std: sys: pal: uefi: os: Implement split_paths - Tested using OVMF on QEMU @rustbot label +O-UEFI cc @nicholasbishop
6 participants
@rustbot label +O-UEFI
cc @nicholasbishop