This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9670

rust: fix testing with $TMPDIR ≠ /tmp
ClosedPublic

Authored by danchr on Dec 30 2020, 7:01 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGdb2bc9e667a1: rust: fix testing with $TMPDIR ≠ /tmp

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

danchr created this revision.Dec 30 2020, 7:01 AM
baymax updated this revision to Diff 24557.Dec 31 2020, 5:07 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare accepted this revision.Jan 5 2021, 4:11 AM
Alphare added a subscriber: Alphare.

Thanks, this is already a bit better.

marmoute added a subscriber: marmoute.Jan 7 2021, 6:23 PM

What is this patch doing ? Should we add some comment in the code to explain the rationnal around the skip thingy ?

Alphare added a comment.Jan 8 2021, 4:45 AM
In D9670#146220, @marmoute wrote:

What is this patch doing ? Should we add some comment in the code to explain the rationnal around the skip thingy ?

Thinking about it some more, this should just use https://doc.rust-lang.org/std/path/struct.Path.html#method.strip_prefix

danchr added a comment.Jan 12 2021, 7:09 AM
In D9670#146245, @Alphare wrote:
In D9670#146220, @marmoute wrote:

What is this patch doing ? Should we add some comment in the code to explain the rationnal around the skip thingy ?

Thinking about it some more, this should just use https://doc.rust-lang.org/std/path/struct.Path.html#method.strip_prefix

It took me long enough to realise what that magic “2” meant and why the test was failing; I'm not comfortable with the logic there to do that refactoring.

Alphare added a comment.Jan 12 2021, 12:05 PM
In D9670#146820, @danchr wrote:

It took me long enough to realise what that magic “2” meant and why the test was failing; I'm not comfortable with the logic there to do that refactoring.

rust
-        let buf = b.join("in_a").components().skip(2).collect::<PathBuf>();
+        let buf = b.join("in_a");
+        let buf = buf.strip_prefix(&base_dir_path.parent().unwrap()).unwrap();
         eprintln!("buf: {}", buf.display());
         let path = path_to_hg_path_buf(buf).unwrap();
         assert_eq!(
             auditor.audit_path(&path),
             Err(HgPathError::TraversesSymbolicLink {
-                path: path,
+                path,
                 symlink: path_to_hg_path_buf(
-                    b.components().skip(2).collect::<PathBuf>()
+                    b.strip_prefix(&base_dir_path.parent().unwrap()).unwrap()
                 )
                 .unwrap()
})

This does the trick. Do you want me to send the patch instead?

Dan Villiom Podlaski Christiansen <danchr@gmail.com> added a commit: rHGdb2bc9e667a1: rust: fix testing with $TMPDIR ≠ /tmp.Jan 13 2021, 2:41 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGdb2bc9e667a1: rust: fix testing with $TMPDIR ≠ /tmp (authored by Dan Villiom Podlaski Christiansen <danchr@gmail.com>). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 24804

rust/hg-core/src/utils/path_auditor.rs

}) })
); );
use std::fs::{create_dir, File}; use std::fs::{create_dir, File};
use tempfile::tempdir; use tempfile::tempdir;
let base_dir = tempdir().unwrap(); let base_dir = tempdir().unwrap();
let base_dir_path = base_dir.path(); let base_dir_path = base_dir.path();
let skip = base_dir_path.components().count() - 1;
let a = base_dir_path.join("a"); let a = base_dir_path.join("a");
let b = base_dir_path.join("b"); let b = base_dir_path.join("b");
create_dir(&a).unwrap(); create_dir(&a).unwrap();
let in_a_path = a.join("in_a"); let in_a_path = a.join("in_a");
File::create(in_a_path).unwrap(); File::create(in_a_path).unwrap();
// TODO make portable // TODO make portable
std::os::unix::fs::symlink(&a, &b).unwrap(); std::os::unix::fs::symlink(&a, &b).unwrap();
let buf = b.join("in_a").components().skip(2).collect::<PathBuf>(); let buf = b.join("in_a").components().skip(skip).collect::<PathBuf>();
eprintln!("buf: {}", buf.display()); eprintln!("buf: {}", buf.display());
let path = path_to_hg_path_buf(buf).unwrap(); let path = path_to_hg_path_buf(buf).unwrap();
assert_eq!( assert_eq!(
auditor.audit_path(&path), auditor.audit_path(&path),
Err(HgPathError::TraversesSymbolicLink { Err(HgPathError::TraversesSymbolicLink {
path: path, path: path,
symlink: path_to_hg_path_buf( symlink: path_to_hg_path_buf(
b.components().skip(2).collect::<PathBuf>() b.components().skip(2).collect::<PathBuf>()
) )
.unwrap() .unwrap()
}) })
); );
} }
} }