Unify subdirectory handling in source.rs (#314)

Avoids having to encode all the `git+` and `subdirectory=` logic in
multiple places.

crates/puffin-resolver/src/distribution/source.rs

@@ -1,6 +1,7 @@
 use std::path::PathBuf;
 
 use anyhow::{Error, Result};
+
 use url::Url;
 
 use puffin_distribution::RemoteDistributionRef;
@@ -50,3 +51,28 @@ impl<'a> TryFrom<&'a RemoteDistributionRef<'_>> for Source<'a> {
         }
     }
 }
+
+impl From<Source<'_>> for Url {
+    fn from(value: Source) -> Self {
+        match value {
+            Source::RegistryUrl(url) => url,
+            Source::RemoteUrl(url, subdirectory) => {
+                if let Some(subdirectory) = subdirectory {
+                    let mut url = (*url).clone();
+                    url.set_fragment(Some(&format!("subdirectory={}", subdirectory.display())));
+                    url
+                } else {
+                    url.clone()
+                }
+            }
+            Source::Git(git, subdirectory) => {
+                let mut url = Url::parse(&format!("{}{}", "git+", Url::from(git).as_str()))
+                    .expect("git url is valid");
+                if let Some(subdirectory) = subdirectory {
+                    url.set_fragment(Some(&format!("subdirectory={}", subdirectory.display())));
+                }
+                url
+            }
+        }
+    }
+}

crates/puffin-resolver/src/distribution/source_distribution.rs

@@ -125,7 +125,7 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> {
         Ok(metadata21)
     }
 
-    /// Given a URL dependency for a source distribution, return a precise variant, if possible.
+    /// Given a remote source distribution, return a precise variant, if possible.
     ///
     /// For example, given a Git dependency with a reference to a branch or tag, return a URL
     /// with a precise reference to the current commit of that branch or tag.
@@ -133,35 +133,24 @@ impl<'a, T: BuildContext> SourceDistributionFetcher<'a, T> {
     /// This method takes into account various normalizations that are independent from the Git
     /// layer. For example: removing `#subdirectory=pkg_dir`-like fragments, and removing `git+`
     /// prefix kinds.
-    pub(crate) async fn precise(&self, url: &Url) -> Result<Option<Url>> {
+    pub(crate) async fn precise(
-        // Extract the subdirectory.
+        &self,
-        let subdirectory = url.fragment().and_then(|fragment| {
+        distribution: &RemoteDistributionRef<'_>,
-            fragment
+    ) -> Result<Option<Url>> {
-                .split('&')
+        let source = Source::try_from(distribution)?;
-                .find_map(|fragment| fragment.strip_prefix("subdirectory="))
+        let Source::Git(git, subdirectory) = source else {
-        });
-
-        let Some(url) = url.as_str().strip_prefix("git+") else {
             return Ok(None);
         };
 
         // Fetch the precise SHA of the Git reference (which could be a branch, a tag, a partial
         // commit, etc.).
-        let git = Git::try_from(Url::parse(url)?)?;
         let dir = self.0.cache().join(GIT_CACHE);
         let source = GitSource::new(git, dir);
         let precise = tokio::task::spawn_blocking(move || source.fetch()).await??;
         let git = Git::from(precise);
 
-        // TODO(charlie): Avoid this double-parse by encoding the source kind separately from the
+        // Re-encode as a URL.
-        // URL.
+        let source = Source::Git(git, subdirectory);
-        let mut url = Url::parse(&format!("{}{}", "git+", Url::from(git).as_str()))?;
+        Ok(Some(source.into()))
-
-        // Re-add the subdirectory fragment.
-        if let Some(subdirectory) = subdirectory {
-            url.set_fragment(Some(&format!("subdirectory={subdirectory}")));
-        }
-
-        Ok(Some(url))
     }
 }

crates/puffin-resolver/src/resolver.rs

@@ -705,14 +705,13 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
                 let _guard = lock.lock().await;
 
                 let fetcher = SourceDistributionFetcher::new(self.build_context);
-                let precise =
+                let precise = fetcher
-                    fetcher
+                    .precise(&RemoteDistributionRef::from_url(&package_name, &url))
-                        .precise(&url)
+                    .await
-                        .await
+                    .map_err(|err| ResolveError::UrlDistribution {
-                        .map_err(|err| ResolveError::UrlDistribution {
+                        url: url.clone(),
-                            url: url.clone(),
+                        err,
-                            err,
+                    })?;
-                        })?;
 
                 let distribution = RemoteDistributionRef::from_url(
                     &package_name,