D7058: rust-dirstate-status: add first Rust implementation of `dirstate.status`
Yuya Nishihara
yuya at tcha.org
Mon Oct 14 15:11:03 UTC 2019
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?
I think `path: impl AsRef<HgPath>` syntax is easier to follow unless we need
a type variable.
> + // 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?
> + 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`.
> +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`.
More information about the Mercurial-devel
mailing list