Preserve partial normalized value in NormalizeError#159866
Conversation
It is common to pin a path normalization to root for many languages:
```ruby
# Ruby
File.expand_path("/tmp/../../lol")
=> "/lol"
```
The error on `normalize_lexically` makes sense for a relative path because Rust CANNOT know how to construct the parent path without performing a call to get CWD. However, for absolute path, I am proposing that we stabilize this behavior to pin at the root for absolute paths. There are links to the POSIX spec and documentation in the commit.
The prior code used `root` which was a len of the **base** of a path sometimes, and simply a low number at other times. It also tracked if the base dir was root or not. Eliminating both yields less code with fewer intermediate variables.
We cannot normalize a relative path past the base because we do not know what the parent is (without touching disk, and assuming CWD). However, a developer might still want a partially normalized value if they're using it for something like comparing if two paths are (roughly) equal or not.
This change adds the ability to `normalize_lexically` users to retrieve the partially normalized value:
```
#![feature(normalize_lexically)]
use std::path::{Path, PathBuf};
let a = Path::new("a/../../b")
.normalize_lexically()
.unwrap_or_else(|e| e.into_partial());
let b = Path::new("../x/../b")
.normalize_lexically()
.unwrap_or_else(|e| e.into_partial());
assert_eq!(a, b);
assert_eq!(a, PathBuf::from("../b"));
```
rustbot
added
S-waiting-on-author
|
|
||
| /// An error returned from [`Path::normalize_lexically`] if a `..` parent reference | ||
| /// would escape the path. | ||
| /// |
There was a problem hiding this comment.
First commit is stacked on #159863. This logic is easier to represent when the absolute path does not error. If that gets merged, I'll promote this from draft. If we want to close it and move forward with this alone, I can.
Also happy to rebase/squash the refactor intermediate commit, I left it in to make the change in form/format different than the logic changes needed to preserve and return the partially normalized path.
| } | ||
| } | ||
| Component::Normal(path) => lexical.push(path), | ||
| Component::ParentDir => match lexical.components().next_back() { |
There was a problem hiding this comment.
Perf note: This code is called every time a .. is in the path. Constructing components re-parses the root of the output string, then the next_back parses backward (IIRC). So this is slower. It's balanced out with the PathBuf::with_capacity() optimization above. I didn't profile the difference.`
Reviewer: Please dismiss the note at will. Leaving it as a self-comment to note a design tradeoff in implementation.
2 participants
We cannot normalize a relative path past the base because we do not know what the parent is (without touching disk, and assuming CWD). However, a developer might still want a partially normalized value if they're using it for something like comparing if two paths are (roughly) equal or not.
This change adds the ability to
normalize_lexicallyusers to retrieve the partially normalized value:Related tracking issue for
normalize_lexicallyunstable feature #134694.