Refactor so package publishing only determines git root in one place

src/cargo/ops/cargo_package.rs

@@ -76,6 +76,7 @@ struct GitVcsInfo {
 pub fn package_one(
     ws: &Workspace<'_>,
     pkg: &Package,
+    repo: Option<&git2::Repository>,
     opts: &PackageOpts<'_>,
 ) -> CargoResult<Option<FileLock>> {
     let config = ws.config();
@@ -96,9 +97,9 @@ pub fn package_one(
 
     // Check (git) repository state, getting the current commit hash if not
     // dirty.
-    let vcs_info = if !opts.allow_dirty {
+    let vcs_info = if repo.is_some() && !opts.allow_dirty {
         // This will error if a dirty repo is found.
-        check_repo_state(pkg, &src_files, config)?
+        check_repo_state(repo.unwrap(), pkg, &src_files, config)?
     } else {
         None
     };
@@ -165,9 +166,11 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
     }
 
     for (pkg, cli_features) in pkgs {
+        let repo = get_repo(pkg, opts.config)?;
         let result = package_one(
             ws,
             pkg,
+            repo.as_ref(),
             &PackageOpts {
                 config: opts.config,
                 list: opts.list,
@@ -388,44 +391,53 @@ fn check_metadata(pkg: &Package, config: &Config) -> CargoResult<()> {
     Ok(())
 }
 
+pub fn get_repo(p: &Package, config: &Config) -> CargoResult<Option<git2::Repository>> {
+    if let Ok(repo) = git2::Repository::discover(p.root()) {
+        if repo.workdir().is_some() {
+            Ok(Some(repo))
+        } else {
+            Ok(None)
+        }
+    } else {
+        config.shell().verbose(|shell| {
+            shell.warn(format!("No (git) VCS found for `{}`", p.root().display()))
+        })?;
+        Ok(None)
+    }
+}
+
 /// Checks if the package source is in a *git* DVCS repository. If *git*, and
 /// the source is *dirty* (e.g., has uncommitted changes) then `bail!` with an
 /// informative message. Otherwise return the sha1 hash of the current *HEAD*
 /// commit, or `None` if no repo is found.
 fn check_repo_state(
+    repo: &git2::Repository,
     p: &Package,
     src_files: &[PathBuf],
     config: &Config,
 ) -> CargoResult<Option<VcsInfo>> {
-    if let Ok(repo) = git2::Repository::discover(p.root()) {
-        if let Some(workdir) = repo.workdir() {
-            debug!("found a git repo at {:?}", workdir);
-            let path = p.manifest_path();
-            let path = path.strip_prefix(workdir).unwrap_or(path);
-            if let Ok(status) = repo.status_file(path) {
-                if (status & git2::Status::IGNORED).is_empty() {
-                    debug!(
-                        "found (git) Cargo.toml at {:?} in workdir {:?}",
-                        path, workdir
-                    );
-                    return Ok(Some(VcsInfo {
-                        git: git(p, src_files, &repo)?,
-                    }));
-                }
-            }
-            config.shell().verbose(|shell| {
-                shell.warn(format!(
-                    "No (git) Cargo.toml found at `{}` in workdir `{}`",
-                    path.display(),
-                    workdir.display()
-                ))
-            })?;
+    let workdir = repo.workdir().unwrap();
+    debug!("found a git repo at {:?}", workdir);
+    let path = p.manifest_path();
+    let path = path.strip_prefix(workdir).unwrap_or(path);
+    if let Ok(status) = repo.status_file(path) {
+        if (status & git2::Status::IGNORED).is_empty() {
+            debug!(
+                "found (git) Cargo.toml at {:?} in workdir {:?}",
+                path, workdir
+            );
+            return Ok(Some(VcsInfo {
+                git: git(p, src_files, &repo)?,
+            }));
         }
-    } else {
-        config.shell().verbose(|shell| {
-            shell.warn(format!("No (git) VCS found for `{}`", p.root().display()))
-        })?;
     }
+    config.shell().verbose(|shell| {
+        shell.warn(format!(
+            "No (git) Cargo.toml found at `{}` in workdir `{}`",
+            path.display(),
+            workdir.display()
+        ))
+    })?;
 
     // No VCS with a checked in `Cargo.toml` found, so we don't know if the
     // directory is dirty or not, thus we have to assume that it's clean.

src/cargo/ops/mod.rs

@@ -13,7 +13,7 @@ pub use self::cargo_generate_lockfile::UpdateOptions;
 pub use self::cargo_install::{install, install_list};
 pub use self::cargo_new::{init, new, NewOptions, VersionControl};
 pub use self::cargo_output_metadata::{output_metadata, ExportInfo, OutputMetadataOptions};
-pub use self::cargo_package::{package, package_one, PackageOpts};
+pub use self::cargo_package::{get_repo, package, package_one, PackageOpts};
 pub use self::cargo_pkgid::pkgid;
 pub use self::cargo_read_manifest::{read_package, read_packages};
 pub use self::cargo_run::run;

src/cargo/ops/registry.rs

@@ -103,11 +103,14 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
     )?;
     verify_dependencies(pkg, &registry, reg_id)?;
 
+    let repo = ops::get_repo(pkg, opts.config)?;
+
     // Prepare a tarball, with a non-suppressible warning if metadata
     // is missing since this is being put online.
     let tarball = ops::package_one(
         ws,
         pkg,
+        repo.as_ref(),
         &ops::PackageOpts {
             config: opts.config,
             verify: opts.verify,
@@ -128,6 +131,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
     transmit(
         opts.config,
         pkg,
+        repo,
         tarball.file(),
         &mut registry,
         reg_id,
@@ -175,6 +179,7 @@ fn verify_dependencies(
 fn transmit(
     config: &Config,
     pkg: &Package,
+    repo: Option<git2::Repository>,
     tarball: &File,
     registry: &mut Registry,
     registry_id: SourceId,
@@ -242,6 +247,17 @@ fn transmit(
                 .with_context(|| format!("failed to read `readme` file for package `{}`", pkg))
         })
         .transpose()?;
+
+    let readme_path_relative_to_repo = readme.as_ref().map(|readme| {
+        || -> Option<String> {
+            let repo_root = repo?.workdir().unwrap().to_owned();
+            let rel = paths::strip_prefix_canonical(pkg.root().join(readme), repo_root).ok()?;
+            let rel = rel.to_str()?;
+            Some(rel.replace("\\", "/"))
+        }()
+        .unwrap_or(readme.clone())
+    });
+
     if let Some(ref file) = *license_file {
         if !pkg.root().join(file).exists() {
             bail!("the license file `{}` does not exist", file)
@@ -281,7 +297,7 @@ fn transmit(
                 keywords: keywords.clone(),
                 categories: categories.clone(),
                 readme: readme_content,
-                readme_file: readme.clone(),
+                readme_file: readme_path_relative_to_repo,
                 repository: repository.clone(),
                 license: license.clone(),
                 license_file: license_file.clone(),