Alphare added a comment.
In D7058#103954 <https://phab.mercurial-scm.org/D7058#103954>, @yuja wrote: > Just quickly scanned. Not reviewed the core logic. > >> +/// Get name in the case stored in the filesystem >> +/// The name should be relative to root, and be normcase-ed for efficiency. >> +/// >> +/// Note that this function is unnecessary, and should not be >> +// called, for case-sensitive filesystems (simply because it's expensive). >> +/// The root should be normcase-ed, too. >> +pub fn filesystem_path<P: AsRef<HgPath>>(root: &HgPath, path: P) -> HgPathBuf { > > Unused? Indeed, it was for a future patch, removed. > I think `path: impl AsRef<HgPath>` syntax is easier to follow unless we need > a type variable. I changed to `impl Trait` in all relevant places in this patch. >> + // TODO path for case-insensitive filesystems, for now this is transparent >> + root.to_owned().join(path.as_ref()) >> +} > > `filesystems_path()` sounds like returning a `std::path::PathBuf`, but > `HgPath` should be a repo-relative path. > >> +/// Returns `true` if path refers to an existing path. >> +/// Returns `true` for broken symbolic links. >> +/// Equivalent to `exists()` on platforms lacking `lstat`. >> +pub fn lexists<P: AsRef<Path>>(path: P) -> bool { > > Unused? Same as above, removed. >> + if !path.as_ref().exists() { >> + return read_link(path).is_ok(); >> + } >> + true >> +} > > Maybe you want `fs::symlink_metadata()`? > I don't know which is faster, but the behavior should be closer to `lstat`. > https://doc.rust-lang.org/std/fs/fn.symlink_metadata.html > >> +impl HgMetadata { >> + #[cfg(unix)] >> + pub fn from_metadata(metadata: Metadata) -> Self { >> + use std::os::linux::fs::MetadataExt; > > `unix != linux`. Maybe you want `std::os::unix::fs::MetadataExt`. Thanks. I also caught a typo in the name of the non-unix impl. It's now split it in two impls. >> +pub fn status<P: AsRef<Path>>( >> + mut dmap: &mut DirstateMap, >> + root_dir: P, >> + files: Vec<HgPathBuf>, >> + list_clean: bool, >> + last_normal_time: i64, >> + check_exec: bool, >> +) -> std::io::Result<(Vec<HgPathBuf>, StatusResult)> >> +where >> + P: Sync, > > `P: AsRef<path> + Sync`. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7058/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7058 To: Alphare, #hg-reviewers Cc: yuja, martinvonz, durin42, kevincox, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel