Auto merge of #14266 - epage:path2url, r=weihanglo

fix(test): Move path2url to CargoPathExt::to_url

### What does this PR try to resolve?

This is a small step, like #14243, to improve the clarity of `cargo-test-support`s API.

Overall, I'm trying to make it more obvious on https://docs.rs/cargo-test-support/latest/cargo_test_support/ which items to reach for when.  I figured this is one that could be demoted to `paths` When doing so, I noticed `CargoPathExt`. I figured if we had any extension traits for `Path`, then this is a
reasonable one to add.

### How should we test and review this PR?

### Additional information

crates/cargo-test-support/src/git.rs

@@ -38,7 +38,7 @@ use some of the helper functions in this file to interact with the repository.
 
 */
 
-use crate::{path2url, project, Project, ProjectBuilder, SymlinkBuilder};
+use crate::{paths::CargoPathExt, project, Project, ProjectBuilder, SymlinkBuilder};
 use std::fs;
 use std::path::{Path, PathBuf};
 use std::sync::Once;
@@ -118,7 +118,7 @@ impl Repository {
     }
 
     pub fn url(&self) -> Url {
-        path2url(self.0.workdir().unwrap().to_path_buf())
+        self.0.workdir().unwrap().to_url()
     }
 
     pub fn revparse_head(&self) -> String {

crates/cargo-test-support/src/lib.rs

@@ -74,6 +74,7 @@ pub mod tools;
 
 pub mod prelude {
     pub use crate::cargo_test;
+    pub use crate::paths::CargoPathExt;
     pub use crate::ArgLine;
     pub use crate::CargoCommand;
     pub use crate::ChannelChanger;
@@ -339,7 +340,8 @@ impl Project {
 
     /// File url for root, ex: `file:///path/to/cargo/target/cit/t0/foo`
     pub fn url(&self) -> Url {
-        path2url(self.root())
+        use paths::CargoPathExt;
+        self.root().to_url()
     }
 
     /// Path to an example built as a library.
@@ -1184,10 +1186,6 @@ pub fn basic_lib_manifest(name: &str) -> String {
     )
 }
 
-pub fn path2url<P: AsRef<Path>>(p: P) -> Url {
-    Url::from_file_path(p).ok().unwrap()
-}
-
 struct RustcInfo {
     verbose_version: String,
     host: String,

crates/cargo-test-support/src/paths.rs

@@ -111,6 +111,8 @@ pub fn home() -> PathBuf {
 }
 
 pub trait CargoPathExt {
+    fn to_url(&self) -> url::Url;
+
     fn rm_rf(&self);
     fn mkdir_p(&self);
 
@@ -132,6 +134,10 @@ pub trait CargoPathExt {
 }
 
 impl CargoPathExt for Path {
+    fn to_url(&self) -> url::Url {
+        url::Url::from_file_path(self).ok().unwrap()
+    }
+
     fn rm_rf(&self) {
         let meta = match self.symlink_metadata() {
             Ok(meta) => meta,
@@ -211,6 +217,30 @@ impl CargoPathExt for Path {
     }
 }
 
+impl CargoPathExt for PathBuf {
+    fn to_url(&self) -> url::Url {
+        self.as_path().to_url()
+    }
+
+    fn rm_rf(&self) {
+        self.as_path().rm_rf()
+    }
+    fn mkdir_p(&self) {
+        self.as_path().mkdir_p()
+    }
+
+    fn ls_r(&self) -> Vec<PathBuf> {
+        self.as_path().ls_r()
+    }
+
+    fn move_in_time<F>(&self, travel_amount: F)
+    where
+        F: Fn(i64, u32) -> (i64, u32),
+    {
+        self.as_path().move_in_time(travel_amount)
+    }
+}
+
 fn do_op<F>(path: &Path, desc: &str, mut f: F)
 where
     F: FnMut(&Path) -> io::Result<()>,

tests/testsuite/bench.rs

@@ -1,6 +1,5 @@
 //! Tests for the `cargo bench` command.
 
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::{basic_bin_manifest, basic_lib_manifest, basic_manifest, project, str};
 

tests/testsuite/build.rs

@@ -12,7 +12,7 @@ use cargo::{
     GlobalContext,
 };
 use cargo_test_support::compare::assert_e2e;
-use cargo_test_support::paths::{root, CargoPathExt};
+use cargo_test_support::paths::root;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;

tests/testsuite/build_script.rs

@@ -7,7 +7,6 @@ use std::thread;
 
 use cargo_test_support::compare::assert_e2e;
 use cargo_test_support::install::cargo_home;
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;

tests/testsuite/cache_lock.rs

@@ -3,7 +3,7 @@
 use std::thread::JoinHandle;
 
 use cargo::util::cache_lock::{CacheLockMode, CacheLocker};
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::{retry, thread_wait_timeout, threaded_timeout};
 

tests/testsuite/cargo_command.rs

@@ -8,7 +8,6 @@ use std::process::Stdio;
 use std::str;
 
 use cargo_test_support::basic_manifest;
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;

tests/testsuite/check.rs

@@ -4,7 +4,6 @@ use std::fmt::{self, Write};
 
 use cargo_test_support::compare::assert_e2e;
 use cargo_test_support::install::exe;
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;

tests/testsuite/clean.rs

@@ -1,6 +1,5 @@
 //! Tests for the `cargo clean` command.
 
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;

tests/testsuite/dep_info.rs

@@ -6,7 +6,7 @@ use std::path::Path;
 use std::str;
 
 use cargo_test_support::compare::assert_e2e;
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;

tests/testsuite/doc.rs

@@ -4,7 +4,6 @@ use std::fs;
 use std::str;
 
 use cargo::core::compiler::RustDocFingerprint;
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;

tests/testsuite/features.rs

@@ -1,6 +1,5 @@
 //! Tests for `[features]` table.
 
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::{Dependency, Package};
 use cargo_test_support::str;

tests/testsuite/features2.rs

@@ -4,7 +4,6 @@ use std::fs::File;
 
 use cargo_test_support::cross_compile::{self, alternate};
 use cargo_test_support::install::cargo_home;
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::publish::validate_crate_contents;
 use cargo_test_support::registry::{Dependency, Package};

tests/testsuite/fix.rs

@@ -3,7 +3,7 @@
 use cargo::core::Edition;
 use cargo_test_support::compare::assert_e2e;
 use cargo_test_support::git::{self, init};
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::{Dependency, Package};
 use cargo_test_support::str;

tests/testsuite/freshness.rs

@@ -9,7 +9,7 @@ use std::process::Stdio;
 use std::thread;
 use std::time::SystemTime;
 
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::{

tests/testsuite/git.rs

@@ -10,11 +10,11 @@ use std::sync::Arc;
 use std::thread;
 
 use cargo_test_support::git::cargo_uses_gitoxide;
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::IntoData;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
-use cargo_test_support::{basic_lib_manifest, basic_manifest, git, main_file, path2url, project};
+use cargo_test_support::{basic_lib_manifest, basic_manifest, git, main_file, project};
 use cargo_test_support::{sleep_ms, str, t, Project};
 
 #[cargo_test]
@@ -878,7 +878,7 @@ fn dep_with_submodule() {
     let git_project2 = git::new("dep2", |project| project.file("lib.rs", "pub fn dep() {}"));
 
     let repo = git2::Repository::open(&git_project.root()).unwrap();
-    let url = path2url(git_project2.root()).to_string();
+    let url = git_project2.root().to_url().to_string();
     git::add_submodule(&repo, &url, Path::new("src"));
     git::commit(&repo);
 
@@ -1000,7 +1000,7 @@ fn dep_with_bad_submodule() {
     let git_project2 = git::new("dep2", |project| project.file("lib.rs", "pub fn dep() {}"));
 
     let repo = git2::Repository::open(&git_project.root()).unwrap();
-    let url = path2url(git_project2.root()).to_string();
+    let url = git_project2.root().to_url().to_string();
     git::add_submodule(&repo, &url, Path::new("src"));
     git::commit(&repo);
 
@@ -2441,12 +2441,12 @@ fn dont_require_submodules_are_checked_out() {
     let git2 = git::new("dep2", |p| p);
 
     let repo = git2::Repository::open(&git1.root()).unwrap();
-    let url = path2url(git2.root()).to_string();
+    let url = git2.root().to_url().to_string();
     git::add_submodule(&repo, &url, Path::new("a/submodule"));
     git::commit(&repo);
 
     git2::Repository::init(&p.root()).unwrap();
-    let url = path2url(git1.root()).to_string();
+    let url = git1.root().to_url().to_string();
     let dst = paths::home().join("foo");
     git2::Repository::clone(&url, &dst).unwrap();
 
@@ -2863,7 +2863,7 @@ fn failed_submodule_checkout() {
     drop((repo, url));
 
     let repo = git2::Repository::open(&git_project.root()).unwrap();
-    let url = path2url(git_project2.root()).to_string();
+    let url = git_project2.root().to_url().to_string();
     git::add_submodule(&repo, &url, Path::new("src"));
     git::commit(&repo);
     drop(repo);
@@ -3152,7 +3152,7 @@ fn dirty_submodule() {
         project.no_manifest().file("lib.rs", "pub fn f() {}")
     });
 
-    let url = path2url(git_project2.root()).to_string();
+    let url = git_project2.root().to_url().to_string();
     git::add_submodule(&repo, &url, Path::new("src"));
 
     // Submodule added, but not committed.
@@ -3200,7 +3200,7 @@ to proceed despite this and include the uncommitted changes, pass the `--allow-d
 
     // Try with a nested submodule.
     let git_project3 = git::new("bar", |project| project.no_manifest().file("mod.rs", ""));
-    let url = path2url(git_project3.root()).to_string();
+    let url = git_project3.root().to_url().to_string();
     git::add_submodule(&sub_repo, &url, Path::new("bar"));
     git_project
         .cargo("package --no-verify")
@@ -4085,7 +4085,7 @@ fn git_worktree_with_bare_original_repo() {
             .bare(true)
             .clone_local(git2::build::CloneLocal::Local)
             .clone(
-                path2url(git_project.root()).as_str(),
+                git_project.root().to_url().as_str(),
                 &paths::root().join("foo-bare"),
             )
             .unwrap()

tests/testsuite/global_cache_tracker.rs

@@ -18,7 +18,7 @@ use cargo::core::global_cache_tracker::{self, DeferredGlobalLastUse, GlobalCache
 use cargo::util::cache_lock::CacheLockMode;
 use cargo::util::interning::InternedString;
 use cargo::GlobalContext;
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::{Package, RegistryBuilder};
 use cargo_test_support::{

tests/testsuite/install.rs

@@ -21,7 +21,7 @@ use cargo_util::{ProcessBuilder, ProcessError};
 use cargo_test_support::install::{
     assert_has_installed_exe, assert_has_not_installed_exe, cargo_home, exe,
 };
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 
 fn pkg(name: &str, vers: &str) {
     Package::new(name, vers)

tests/testsuite/install_upgrade.rs

@@ -8,7 +8,6 @@ use std::sync::atomic::{AtomicUsize, Ordering};
 
 use cargo::core::PackageId;
 use cargo_test_support::install::{cargo_home, exe};
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::{self, Package};
 use cargo_test_support::{

tests/testsuite/local_registry.rs

@@ -2,7 +2,7 @@
 
 use std::fs;
 
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::{registry_path, Package};
 use cargo_test_support::{basic_manifest, project, str, t};

tests/testsuite/login.rs

@@ -4,7 +4,7 @@ use std::fs;
 use std::path::PathBuf;
 
 use cargo_test_support::cargo_process;
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::{self, RegistryBuilder};
 use cargo_test_support::str;

tests/testsuite/logout.rs

@@ -1,7 +1,7 @@
 //! Tests for the `cargo logout` command.
 
 use super::login::check_token;
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::TestRegistry;
 use cargo_test_support::{cargo_process, registry, str};

tests/testsuite/old_cargos.rs

@@ -15,7 +15,6 @@
 use std::fs;
 
 use cargo::CargoResult;
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::{self, Dependency, Package};
 use cargo_test_support::{cargo_exe, execs, paths, process, project, rustc_host};

tests/testsuite/owner.rs

@@ -2,7 +2,6 @@
 
 use std::fs;
 
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::project;
 use cargo_test_support::registry::{self, api_path};

tests/testsuite/package.rs

@@ -3,13 +3,12 @@
 use std::fs::{self, read_to_string, File};
 use std::path::Path;
 
-use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::prelude::*;
 use cargo_test_support::publish::validate_crate_contents;
 use cargo_test_support::registry::{self, Package};
 use cargo_test_support::{
-    basic_manifest, cargo_process, git, path2url, paths, project, rustc_host, str,
-    symlink_supported, t, ProjectBuilder,
+    basic_manifest, cargo_process, git, paths, project, rustc_host, str, symlink_supported, t,
+    ProjectBuilder,
 };
 use flate2::read::GzDecoder;
 use tar::Archive;
@@ -608,7 +607,7 @@ fn package_git_submodule() {
     });
 
     let repository = git2::Repository::open(&project.root()).unwrap();
-    let url = path2url(library.root()).to_string();
+    let url = library.root().to_url().to_string();
     git::add_submodule(&repository, &url, Path::new("bar"));
     git::commit(&repository);
 
@@ -656,7 +655,7 @@ fn package_symlink_to_submodule() {
     });
 
     let repository = git2::Repository::open(&project.root()).unwrap();
-    let url = path2url(library.root()).to_string();
+    let url = library.root().to_url().to_string();
     git::add_submodule(&repository, &url, Path::new("submodule"));
     t!(symlink(
         &project.root().join("submodule"),

tests/testsuite/path.rs

@@ -2,7 +2,7 @@
 
 use std::fs;
 
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;