Remove sequential cloning when resolving dependencies between git repos

src/Spago/Command/Fetch.purs

@@ -224,7 +224,14 @@ run { packages: packagesRequestedToInstall, ensureRanges, isTest, isRepl } = do
     -- (we return them from inside there because we need to update the commit hashes)
     case workspace.packageSet.lockfile of
       Right _lockfile -> pure dependencies
-      Left reason -> writeNewLockfile reason dependencies
+      Left reason -> do
+        -- When generating a lockfile, we need ALL git packages to be fetched so we can
+        -- get their commit hashes. If a package is selected, depsToFetch only includes
+        -- that package's deps, but the lockfile needs all packages.
+        let allDeps = toAllDependencies allTransitiveDeps
+        when (Map.keys allDeps /= Map.keys depsToFetch) do
+          fetchPackagesToLocalCache allDeps
+        writeNewLockfile reason dependencies
 
 fetchPackagesToLocalCache :: ∀ a. Map PackageName Package -> Spago (FetchEnv a) Unit
 fetchPackagesToLocalCache packages = do
@@ -530,16 +537,24 @@ getPackageDependencies packageName package = case package of
     maybeManifest <- Registry.getManifestFromIndex packageName v
     pure $ maybeManifest <#> \(Manifest m) -> { core: m.dependencies, test: Map.empty }
   GitPackage p -> do
-    -- Note: we get the package in local cache nonetheless,
-    -- so we have guarantees about being able to fetch it
-    { rootPath } <- ask
-    let packageLocation = Config.getLocalPackageLocation rootPath packageName package
-    unlessM (FS.exists packageLocation) do
-      getGitPackageInLocalCache packageName p
     case p.dependencies of
-      Just (Dependencies dependencies) ->
+      -- if dependencies are declared, we can use them directly without cloning.
+      -- the package will be fetched later in fetchPackagesToLocalCache.
+      Just (Dependencies dependencies) -> do
+        -- when offline, verify the package is cached before proceeding
+        { offline, rootPath } <- ask
+        let packageLocation = Config.getLocalPackageLocation rootPath packageName (GitPackage p)
+        when (offline == Offline) do
+          unlessM (FS.exists packageLocation) do
+            die $ "Package '" <> PackageName.print packageName <> "' is not in the local cache, and Spago is running in offline mode - can't make progress."
         pure $ Just { core: map (fromMaybe Config.widestRange) dependencies, test: Map.empty }
+      -- if the dependencies are not declared, then we need to clone the repo
+      -- to look at the package manifest inside
       Nothing -> do
+        { rootPath } <- ask
+        let packageLocation = Config.getLocalPackageLocation rootPath packageName package
+        unlessM (FS.exists packageLocation) do
+          getGitPackageInLocalCache packageName p
         readLocalDependencies $ Path.toGlobal $ maybe packageLocation (packageLocation </> _) p.subdir
   LocalPackage p -> do
     readLocalDependencies $ Path.global p.path

src/Spago/Config.purs

@@ -634,18 +634,41 @@ getLocalPackageLocation root name = case _ of
 -- inputs must map to two different outputs _and_ those outputs must differ by
 -- more than just casing.
 --
--- The characters which are most commonly used in version and branch names are
--- those which we allow through as they are (without escaping).
+-- The escape scheme uses:
+-- - `_` followed by lowercase for uppercase letters (A -> _a)
+-- - `_-` for underscore itself
+-- - `%` followed by mnemonic letter for special chars (/ -> %s, \ -> %b, : -> %c)
+-- - `%XX` hex fallback for other chars
 fileSystemCharEscape :: String -> String
 fileSystemCharEscape = String.toCodePointArray >>> map escapeCodePoint >>> Array.fold
   where
-  commonlyUsedChars = map String.codePointFromChar [ '.', ',', '-', '_', '+' ]
-  ignoreEscape = Unicode.isLower || Unicode.isDecDigit || flip Array.elem commonlyUsedChars
+  -- Pass through: lowercase, digits, and safe punctuation (but NOT underscore)
+  safeChars = map String.codePointFromChar [ '.', ',', '-', '+' ]
+  isSafe = Unicode.isLower || Unicode.isDecDigit || flip Array.elem safeChars
 
   escapeCodePoint :: CodePoint -> String
   escapeCodePoint cp
-    | ignoreEscape cp = String.singleton cp
-    | otherwise = append "%" $ Int.toStringAs Int.hexadecimal $ Enum.fromEnum cp
+    | isSafe cp = String.singleton cp
+    | cp == String.codePointFromChar '_' = "_-"
+    | Unicode.isUpper cp = "_" <> String.singleton (Unicode.toLowerSimple cp)
+    | otherwise = escapeSpecial cp
+
+  escapeSpecial :: CodePoint -> String
+  escapeSpecial cp = case String.singleton cp of
+    "/" -> "%s"
+    "\\" -> "%b"
+    ":" -> "%c"
+    "@" -> "%a"
+    "~" -> "%t"
+    "*" -> "%r"
+    "?" -> "%q"
+    "\"" -> "%d"
+    "<" -> "%l"
+    ">" -> "%g"
+    "|" -> "%p"
+    " " -> "%w"
+    "%" -> "%%"
+    _ -> "%" <> Int.toStringAs Int.hexadecimal (Enum.fromEnum cp)
 
 data WithTestGlobs
   = WithTestGlobs

src/Spago/Prelude.purs

@@ -167,7 +167,7 @@ mkTemp' maybeSuffix = liftAff do
     sha <- Sha256.hashString $ show now <> fromMaybe "" maybeSuffix
     shaToHex sha
   -- Return the dir, but don't make it - that's the responsibility of the client
-  let tempDirPath = Paths.paths.temp </> String.drop 50 random
+  let tempDirPath = Paths.paths.temp </> String.drop 56 random
   pure tempDirPath
 
 mkTemp :: forall m. MonadAff m => m Path.GlobalPath

test-fixtures/circular-dependencies.txt

@@ -2,7 +2,6 @@ Reading Spago workspace configuration...
 
 ✓ Selecting package to build: bbb
 
-Cloning https://github.com/purescript/spago.git
 
 ✘ The following packages have circular dependencies:
   - a