Strip registry root from file in Index::add() unconditionally

Cargo.toml

@@ -46,6 +46,7 @@ tracing-subscriber = {version = "0.3", default-features = false, features = ["an
 warp = {version = "0.3", default-features = false}
 
 [dev-dependencies]
+pathdiff = "0.2"
 tempfile = {version = "3.1"}
 tokio = {version = "1.34", default-features = false, features = ["macros", "rt"]}
 

src/index.rs

@@ -180,27 +180,16 @@ impl Index {
     inner(root, addr)
   }
 
-  /// Add a file to the index. Note that this operation only stages the
-  /// file. A commit will still be necessary to make it accessible.
-  pub fn add(&mut self, file: &Path) -> Result<()> {
-    let relative_path = if !file.is_relative() {
-      file.strip_prefix(&self.root).with_context(|| {
-        format!(
-          "failed to make {} relative to {}",
-          file.display(),
-          self.root.display()
-        )
-      })?
-    } else {
-      file
-    };
-
+  /// Add a file to the index. The path must be relative to the index root.
+  /// Note that this operation only stages the file. A commit will still be
+  /// necessary to make it accessible.
+  pub fn add(&mut self, relative_file_path: &Path) -> Result<()> {
     let mut index = self
       .repository
       .index()
       .context("failed to retrieve git repository index")?;
     index
-      .add_path(relative_path)
+      .add_path(relative_file_path)
       .context("failed to add file to git index")?;
     index
       .write()
@@ -497,4 +486,21 @@ mod tests {
     let statuses = index.repository.statuses(Some(&mut options)).unwrap();
     assert_eq!(statuses.len(), 0);
   }
+
+  /// Ensure we can use some special names as relative index root.
+  #[test]
+  fn index_root_relative_path() {
+    let base = tempdir().unwrap();
+    env::set_current_dir(&base).unwrap();
+    let addr = "127.0.0.1:0".parse().unwrap();
+
+    for special_name in ["config.json", "index"] {
+      let relative_index_root = Path::new(special_name);
+      create_dir_all(relative_index_root).unwrap();
+
+      let index = Index::new(relative_index_root, &addr).unwrap();
+      // The repository should be clean.
+      assert_eq!(index.repository.state(), RepositoryState::Clean);
+    }
+  }
 }

src/publish.rs

@@ -254,7 +254,8 @@ pub fn publish_crate(mut body: Bytes, index: &mut Index) -> Result<()> {
     "crate name contains non-ASCII characters"
   );
 
-  let crate_meta_dir = index.root().join(crate_path(&crate_name));
+  let crate_meta_relative_dir = crate_path(&crate_name);
+  let crate_meta_dir = index.root().join(&crate_meta_relative_dir);
   create_dir_all(&crate_meta_dir)
     .with_context(|| format!("failed to create directory {}", crate_meta_dir.display()))?;
 
@@ -265,7 +266,8 @@ pub fn publish_crate(mut body: Bytes, index: &mut Index) -> Result<()> {
 
   // TODO: We may want to sanitize `metadata.vers` somewhat.
   let data = read_crate(&mut body, crate_length).context("failed to read crate data")?;
-  let crate_meta_path = crate_meta_dir.join(&crate_name);
+  let crate_meta_relative_path = crate_meta_relative_dir.join(&crate_name);
+  let crate_meta_path = index.root().join(&crate_meta_relative_path);
 
   let mut file = OpenOptions::new()
     .write(true)
@@ -283,8 +285,8 @@ pub fn publish_crate(mut body: Bytes, index: &mut Index) -> Result<()> {
   to_writer(&mut file, &entry).context("failed to write crate index meta data")?;
   writeln!(file).context("failed to append new line to crate index meta data file")?;
 
-  let crate_file_name = crate_file_name(&crate_name, &crate_vers);
-  let crate_path = index.root().join(crate_file_name);
+  let crate_relative_path = PathBuf::from(crate_file_name(&crate_name, &crate_vers));
+  let crate_path = index.root().join(&crate_relative_path);
   let mut file = OpenOptions::new()
     .write(true)
     .create(true)
@@ -296,15 +298,20 @@ pub fn publish_crate(mut body: Bytes, index: &mut Index) -> Result<()> {
     .write(&data)
     .with_context(|| format!("failed to write to crate file {}", crate_path.display()))?;
 
-  index.add(&crate_meta_path).with_context(|| {
+  index.add(&crate_meta_relative_path).with_context(|| {
     format!(
-      "failed to add {} to git repository",
-      crate_meta_path.display()
+      "failed to add {} to git repository (full path: {})",
+      crate_meta_relative_path.display(),
+      crate_meta_path.display(),
     )
   })?;
   index
-    .add(&crate_path)
-    .with_context(|| format!("failed to add {} to git repository", crate_path.display()))?;
+    .add(&crate_relative_path)
+    .with_context(|| format!(
+      "failed to add {} to git repository (full path: {})",
+      crate_relative_path.display(),
+      crate_path.display(),
+    ))?;
   index
     .commit(&format!("Add {} in version {}", crate_name, crate_vers))
     .context("failed to commit changes to index")?;

tests/end-to-end.rs

@@ -3,6 +3,7 @@
 
 #![allow(clippy::ineffective_open_options)]
 
+use std::env;
 use std::fs::create_dir;
 use std::fs::OpenOptions;
 use std::io::Write as _;
@@ -14,7 +15,7 @@ use std::process::Command;
 use anyhow::bail;
 use anyhow::Context as _;
 use anyhow::Result;
-
+use pathdiff::diff_paths;
 use tempfile::tempdir;
 
 use tokio::spawn;
@@ -51,6 +52,11 @@ enum Locator {
 }
 
 
+enum RegistryRootPath {
+  Absolute,
+  Relative,
+}
+
 /// Append data to a file.
 fn append<B>(file: &Path, data: B) -> Result<()>
 where
@@ -161,9 +167,12 @@ where
 
 
 /// Serve our registry.
-fn serve_registry() -> (JoinHandle<()>, PathBuf, SocketAddr) {
+fn serve_registry(root_path: RegistryRootPath) -> (JoinHandle<()>, PathBuf, SocketAddr) {
   let root = tempdir().unwrap();
-  let path = root.path().to_owned();
+  let path = match root_path {
+    RegistryRootPath::Absolute => root.path().to_owned(),
+    RegistryRootPath::Relative => diff_paths(root.path(), env::current_dir().unwrap()).unwrap(),
+  };
   let addr = "127.0.0.1:0".parse().unwrap();
 
   let (serve, addr) = serve(&path, addr).unwrap();
@@ -182,7 +191,32 @@ fn serve_registry() -> (JoinHandle<()>, PathBuf, SocketAddr) {
 /// Check that we can publish a crate.
 #[tokio::test]
 async fn publish() {
-  let (_handle, _reg_root, addr) = serve_registry();
+  let (_handle, _reg_root, addr) = serve_registry(RegistryRootPath::Absolute);
+
+  let src_root = tempdir().unwrap();
+  let src_root = src_root.path();
+  let home = setup_cargo_home(src_root, Locator::Socket(addr)).unwrap();
+
+  let my_lib = src_root.join("my-lib");
+  cargo_init(&home, ["--lib", my_lib.to_str().unwrap()])
+    .await
+    .unwrap();
+
+  cargo_publish(
+    &home,
+    [
+      "--manifest-path",
+      my_lib.join("Cargo.toml").to_str().unwrap(),
+    ],
+  )
+  .await
+  .unwrap();
+}
+
+
+#[tokio::test]
+async fn publish_relative_index_root() {
+  let (_handle, _reg_root, addr) = serve_registry(RegistryRootPath::Relative);
 
   let src_root = tempdir().unwrap();
   let src_root = src_root.path();
@@ -208,7 +242,7 @@ async fn publish() {
 /// Check that we can publish crates with a renamed dependency.
 #[tokio::test]
 async fn publish_renamed() {
-  let (_handle, _reg_root, addr) = serve_registry();
+  let (_handle, _reg_root, addr) = serve_registry(RegistryRootPath::Absolute);
 
   let src_root = tempdir().unwrap();
   let src_root = src_root.path();
@@ -307,14 +341,14 @@ async fn test_publish_and_consume(registry_locator: Locator) {
 /// Check that we can consume a published crate over HTTP.
 #[tokio::test]
 async fn get_http() {
-  let (_handle, _, addr) = serve_registry();
+  let (_handle, _, addr) = serve_registry(RegistryRootPath::Absolute);
   test_publish_and_consume(Locator::Socket(addr)).await
 }
 
 
 /// Check that we can consume a published crate through the file system.
 #[tokio::test]
 async fn get_filesystem() {
-  let (_handle, root, _) = serve_registry();
+  let (_handle, root, _) = serve_registry(RegistryRootPath::Absolute);
   test_publish_and_consume(Locator::Path(root)).await
 }