fix: skip unneeded url parse and only add git+ when needed

crates/pixi_git/src/url.rs

@@ -31,7 +31,12 @@ impl CanonicalUrl {
 
         // Strip credentials.
         let _ = url.set_password(None);
-        let _ = url.set_username("");
+
+        // Only stripping the username if the scheme is not `ssh`. This is because `ssh` URLs should
+        // have the `git` username.
+        if !url.scheme().contains("ssh") {
+            let _ = url.set_username("");
+        }
 
         // Strip a trailing slash.
         if url.path().ends_with('/') {

crates/pixi_uv_conversions/src/requirements.rs

@@ -1,13 +1,9 @@
+use pixi_manifest::{pypi::VersionOrStar, PyPiRequirement};
+use pixi_spec::{GitReference, GitSpec};
 use std::{
     path::{Path, PathBuf},
     str::FromStr,
 };
-
-// use pep440_rs::VersionSpecifiers;
-
-use pixi_git::url::RepositoryUrl;
-use pixi_manifest::{pypi::VersionOrStar, PyPiRequirement};
-use pixi_spec::{GitReference, GitSpec};
 use thiserror::Error;
 use url::Url;
 use uv_distribution_filename::DistExtension;
@@ -24,8 +20,13 @@ fn create_uv_url(
     rev: Option<&GitReference>,
     subdir: Option<&str>,
 ) -> Result<Url, url::ParseError> {
-    // Create the url.
-    let url = format!("git+{url}");
+    // Add the git+ prefix if it doesn't exist.
+    let url = url.to_string();
+    let url = match url.strip_prefix("git+") {
+        Some(_) => url,
+        None => format!("git+{}", url),
+    };
+
     // Add the tag or rev if it exists.
     let url = rev.as_ref().map_or_else(
         || url.clone(),
@@ -57,7 +58,7 @@ fn manifest_version_to_version_specifiers(
 
 #[derive(Error, Debug)]
 pub enum AsPep508Error {
-    #[error("error while canonicalizing {path}")]
+    #[error("error while canonicalization {path}")]
     CanonicalizeError {
         source: std::io::Error,
         path: PathBuf,
@@ -71,11 +72,11 @@ pub enum AsPep508Error {
     NameError(#[from] InvalidNameError),
     #[error("using an editable flag for a path that is not a directory: {path}")]
     EditableIsNotDir { path: PathBuf },
-    #[error("error while canonicalizing {0}")]
-    VerabatimUrlError(#[from] uv_pep508::VerbatimUrlError),
+    #[error("error while canonicalization {0}")]
+    VerbatimUrlError(#[from] uv_pep508::VerbatimUrlError),
     #[error("error in extension parsing")]
     ExtensionError(#[from] uv_distribution_filename::ExtensionError),
-    #[error("error in parsing version specificers")]
+    #[error("error in parsing version specifiers")]
     VersionSpecifiersError(#[from] uv_pep440::VersionSpecifiersParseError),
 }
 
@@ -105,23 +106,39 @@ pub fn as_uv_req(
                 },
             ..
         } => RequirementSource::Git {
-            repository: RepositoryUrl::parse(git.as_str())
-                .map_err(|err| AsPep508Error::UrlParseError {
-                    source: err,
-                    url: git.to_string(),
-                })?
-                .into(),
+            // Url is already a git url, should look like:
+            // - 'ssh://git@github.com/user/repo'
+            // - 'https://github.com/user/repo'
+            repository: {
+                if git.scheme().strip_prefix("git+").is_some() {
+                    // Setting the scheme might fail, so using string manipulation instead
+                    let url_str = git.to_string();
+                    let stripped = url_str.strip_prefix("git+").unwrap_or(&url_str);
+                    // Reparse the url with the new scheme.
+                    Url::parse(stripped).map_err(|e| AsPep508Error::UrlParseError {
+                        source: e,
+                        url: stripped.to_string(),
+                    })?
+                } else {
+                    git.clone()
+                }
+            },
+            // Unique identifier for the commit, as Git object identifier
             precise: rev
                 .as_ref()
                 .map(|s| s.as_full_commit())
                 .and_then(|s| s.map(uv_git::GitOid::from_str))
                 .transpose()
                 .expect("could not parse sha"),
+            // The reference to the commit to use, which could be a branch, tag or revision.
             reference: rev
                 .as_ref()
                 .map(|rev| into_uv_git_reference(rev.clone().into()))
                 .unwrap_or(uv_git::GitReference::DefaultBranch),
             subdirectory: subdirectory.as_ref().and_then(|s| s.parse().ok()),
+            // The full url used to clone, comparable to the git+ url in pip. e.g:
+            // - 'git+SCHEMA://HOST/PATH@REF#subdirectory=SUBDIRECTORY'
+            // - 'git+ssh://github.com/user/repo@d099af3b1028b00c232d8eda28a997984ae5848b'
             url: VerbatimUrl::from_url(
                 create_uv_url(git, rev.as_ref(), subdirectory.as_deref()).map_err(|e| {
                     AsPep508Error::UrlParseError {
@@ -199,3 +216,63 @@ pub fn as_uv_req(
         origin: None,
     })
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_git_url() {
+        let pypi_req = PyPiRequirement::Git {
+            url: GitSpec {
+                git: Url::parse("ssh://git@github.com/user/test.git").unwrap(),
+                rev: Some(GitReference::Rev(
+                    "d099af3b1028b00c232d8eda28a997984ae5848b".to_string(),
+                )),
+                subdirectory: None,
+            },
+            extras: vec![],
+        };
+        let uv_req = as_uv_req(&pypi_req, "test", Path::new("")).unwrap();
+
+        let expected_uv_req = RequirementSource::Git {
+            repository: Url::parse("ssh://git@github.com/user/test.git").unwrap(),
+            precise: Some(uv_git::GitOid::from_str("d099af3b1028b00c232d8eda28a997984ae5848b").unwrap()),
+            reference: uv_git::GitReference::BranchOrTagOrCommit("d099af3b1028b00c232d8eda28a997984ae5848b".to_string()),
+            subdirectory: None,
+            url: VerbatimUrl::from_url(Url::parse("git+ssh://git@github.com/user/test.git@d099af3b1028b00c232d8eda28a997984ae5848b").unwrap()),
+        };
+
+        assert_eq!(uv_req.source, expected_uv_req);
+
+        // With git+ prefix
+        let pypi_req = PyPiRequirement::Git {
+            url: GitSpec {
+                git: Url::parse("git+https://github.com/user/test.git").unwrap(),
+                rev: Some(GitReference::Rev(
+                    "d099af3b1028b00c232d8eda28a997984ae5848b".to_string(),
+                )),
+                subdirectory: None,
+            },
+            extras: vec![],
+        };
+        let uv_req = as_uv_req(&pypi_req, "test", Path::new("")).unwrap();
+        let expected_uv_req = RequirementSource::Git {
+            repository: Url::parse("https://github.com/user/test.git").unwrap(),
+            precise: Some(
+                uv_git::GitOid::from_str("d099af3b1028b00c232d8eda28a997984ae5848b").unwrap(),
+            ),
+            reference: uv_git::GitReference::BranchOrTagOrCommit(
+                "d099af3b1028b00c232d8eda28a997984ae5848b".to_string(),
+            ),
+            subdirectory: None,
+            url: VerbatimUrl::from_url(
+                Url::parse(
+                    "git+https://github.com/user/test.git@d099af3b1028b00c232d8eda28a997984ae5848b",
+                )
+                .unwrap(),
+            ),
+        };
+        assert_eq!(uv_req.source, expected_uv_req);
+    }
+}

src/lock_file/update.rs

@@ -431,8 +431,8 @@ impl<'p> LockFileDerivedData<'p> {
         .await
         .with_context(|| {
             format!(
-                "{}: error installing/updating PyPI dependencies",
-                environment.name()
+                "Failed to update PyPI packages for environment '{}'",
+                environment.name().fancy_display()
             )
         })?;
 
@@ -2065,12 +2065,12 @@ async fn spawn_solve_pypi_task<'p>(
             .collect::<Result<_, ConversionError>>()
             .into_diagnostic()?;
 
-        let index_map = IndexMap::from_iter(dependencies);
+        let requirements = IndexMap::from_iter(dependencies);
 
         let (records, prefix_task_result) = lock_file::resolve_pypi(
             resolution_context,
             &pypi_options,
-            index_map,
+            requirements,
             system_requirements,
             &pixi_solve_records,
             &locked_pypi_records,

src/uv_reporter.rs

@@ -145,7 +145,12 @@ impl uv_installer::PrepareReporter for UvReporter {
     }
 
     fn on_build_start(&self, dist: &BuildableSource) -> usize {
-        self.start(format!("building {}", dist))
+        let name: String = if let Some(name) = dist.name() {
+            name.to_string()
+        } else {
+            dist.to_string()
+        };
+        self.start(format!("building {}", name))
     }
 
     fn on_build_complete(&self, _dist: &BuildableSource, id: usize) {
@@ -161,13 +166,15 @@ impl uv_installer::PrepareReporter for UvReporter {
     }
 
     // TODO: figure out how to display this nicely
-    fn on_download_start(&self, _name: &PackageName, _size: Option<u64>) -> usize {
-        0
+    fn on_download_start(&self, name: &PackageName, _size: Option<u64>) -> usize {
+        self.start(format!("downloading {}", name))
     }
 
     fn on_download_progress(&self, _index: usize, _bytes: u64) {}
 
-    fn on_download_complete(&self, _name: &PackageName, _index: usize) {}
+    fn on_download_complete(&self, _name: &PackageName, id: usize) {
+        self.finish(id);
+    }
 }
 
 impl uv_installer::InstallReporter for UvReporter {
@@ -210,13 +217,15 @@ impl uv_resolver::ResolverReporter for UvReporter {
     }
 
     // TODO: figure out how to display this nicely
-    fn on_download_start(&self, _name: &PackageName, _size: Option<u64>) -> usize {
-        0
+    fn on_download_start(&self, name: &PackageName, _size: Option<u64>) -> usize {
+        self.start(format!("downloading {}", name))
     }
 
     fn on_download_progress(&self, _id: usize, _bytes: u64) {}
 
-    fn on_download_complete(&self, _name: &PackageName, _id: usize) {}
+    fn on_download_complete(&self, _name: &PackageName, id: usize) {
+        self.finish(id);
+    }
 }
 
 impl uv_distribution::Reporter for UvReporter {
@@ -237,11 +246,13 @@ impl uv_distribution::Reporter for UvReporter {
     }
 
     // TODO: figure out how to display this nicely
-    fn on_download_start(&self, _name: &PackageName, _size: Option<u64>) -> usize {
-        0
+    fn on_download_start(&self, name: &PackageName, _size: Option<u64>) -> usize {
+        self.start(format!("downloading {}", name))
     }
 
     fn on_download_progress(&self, _id: usize, _bytes: u64) {}
 
-    fn on_download_complete(&self, _name: &PackageName, _id: usize) {}
+    fn on_download_complete(&self, _name: &PackageName, id: usize) {
+        self.finish(id);
+    }
 }