This revision now requires changes to proceed.
martinvonz added inline comments.
martinvonz requested changes to this revision.
INLINE COMMENTS
> files.rs:207-216
> + let name = if !name.is_absolute() {
> + root.join(&cwd).join(&name)
> + } else {
> + name.to_owned()
> + };
> + let mut auditor = PathAuditor::new(&root);
> + if name != root && name.starts_with(&root) {
Might be worth having a fast path for when `name` is not absolute (including
"") and neither `name` nor `cwd` contains "../"so we don't do `let name =
root.join(&cwd).join(&name)` immediately followed by `let name =
name.strip_prefix(&root).unwrap()`. Especially since that seems like the common
case. Or can that ever produce different results? Anyway, that can be done
later.
> files.rs:220-222
> + // Determine whether `name' is in the hierarchy at or beneath `root',
> + // by iterating name=name.parent() until that causes no change (can't
> + // check name == '/', because that doesn't work on windows).
Is this accurate? It looks like we break when `name.parent() == None`.
> files.rs:227-230
> + if name.components().next().is_none() {
> + // `name` was actually the same as root (maybe a symlink)
> + return Ok("".into());
> + }
`name.components().next()` will be `None` only if `name` is something like "/",
right? But then we shouldn't return "" (a repo-relative path), we should error
out.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7871/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7871
To: Alphare, #hg-reviewers, kevincox, martinvonz
Cc: martinvonz, durin42, kevincox, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
