From 836b51abebc2cff16fa9d18dfaa90fb2633a0305 Mon Sep 17 00:00:00 2001 From: Fabrizio Ferrai Date: Thu, 22 Jan 2026 21:01:29 +0200 Subject: [PATCH 1/4] Remove sequential cloning when resolving dependencies between git repos --- src/Spago/Command/Fetch.purs | 31 +- test-fixtures/circular-dependencies.txt | 1 - test-fixtures/git-declared-deps/spago.lock | 477 ++++++++++++++++++ .../git-declared-deps/with-declared-deps.yaml | 17 + .../without-declared-deps.yaml | 13 + test-fixtures/offline.txt | 2 +- test/Prelude.purs | 32 ++ test/Spago/Install.purs | 67 ++- test/Spago/Publish.purs | 11 - 9 files changed, 619 insertions(+), 32 deletions(-) create mode 100644 test-fixtures/git-declared-deps/spago.lock create mode 100644 test-fixtures/git-declared-deps/with-declared-deps.yaml create mode 100644 test-fixtures/git-declared-deps/without-declared-deps.yaml diff --git a/src/Spago/Command/Fetch.purs b/src/Spago/Command/Fetch.purs index 953bc9f0e..aa22a077e 100644 --- a/src/Spago/Command/Fetch.purs +++ b/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.size allDeps /= Map.size depsToFetch) do + fetchPackagesToLocalCache allDeps + writeNewLockfile reason dependencies fetchPackagesToLocalCache :: ∀ a. Map PackageName Package -> Spago (FetchEnv a) Unit fetchPackagesToLocalCache packages = do @@ -522,16 +529,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 diff --git a/test-fixtures/circular-dependencies.txt b/test-fixtures/circular-dependencies.txt index ab2aa236e..a31e94433 100644 --- a/test-fixtures/circular-dependencies.txt +++ b/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 diff --git a/test-fixtures/git-declared-deps/spago.lock b/test-fixtures/git-declared-deps/spago.lock new file mode 100644 index 000000000..0f808face --- /dev/null +++ b/test-fixtures/git-declared-deps/spago.lock @@ -0,0 +1,477 @@ +{ + "workspace": { + "packages": { + "test-declared-deps": { + "path": "./", + "core": { + "dependencies": [ + "either" + ], + "build_plan": [ + "control", + "either", + "invariant", + "maybe", + "newtype", + "prelude", + "safe-coerce", + "unsafe-coerce" + ] + }, + "test": { + "dependencies": [], + "build_plan": [] + } + } + }, + "package_set": { + "address": { + "registry": "0.0.1" + }, + "compiler": ">=0.15.4 <0.16.0", + "content": { + "ace": "9.0.0", + "aff": "7.1.0", + "aff-bus": "6.0.0", + "aff-coroutines": "9.0.0", + "aff-promise": "4.0.0", + "aff-retry": "2.0.0", + "affjax": "13.0.0", + "affjax-node": "1.0.0", + "affjax-web": "1.0.0", + "ansi": "7.0.0", + "argonaut": "9.0.0", + "argonaut-codecs": "9.1.0", + "argonaut-core": "7.0.0", + "argonaut-generic": "8.0.0", + "argonaut-traversals": "10.0.0", + "argparse-basic": "2.0.0", + "array-builder": "0.1.2", + "arraybuffer": "13.0.0", + "arraybuffer-builder": "3.0.1", + "arraybuffer-types": "3.0.2", + "arrays": "7.1.0", + "arrays-zipper": "2.0.1", + "ask": "1.0.0", + "assert": "6.0.0", + "avar": "5.0.0", + "b64": "0.0.8", + "barbies": "1.0.1", + "barlow-lens": "0.9.0", + "bifunctors": "6.0.0", + "bigints": "7.0.1", + "bolson": "0.1.1", + "bower-json": "3.0.0", + "call-by-name": "4.0.1", + "canvas": "6.0.0", + "canvas-action": "9.0.0", + "cartesian": "1.0.6", + "catenable-lists": "7.0.0", + "channel": "1.0.0", + "checked-exceptions": "3.1.1", + "classnames": "2.0.0", + "codec": "5.0.0", + "codec-argonaut": "9.0.0", + "colors": "7.0.1", + "concur-core": "0.5.0", + "concur-react": "0.5.0", + "concurrent-queues": "3.0.0", + "console": "6.0.0", + "const": "6.0.0", + "contravariant": "6.0.0", + "control": "6.0.0", + "convertable-options": "1.0.0", + "coroutines": "7.0.0", + "css": "6.0.0", + "datetime": "6.1.0", + "datetime-parsing": "0.2.0", + "debug": "6.0.2", + "decimals": "7.1.0", + "deku": "0.6.1", + "deno": "0.0.5", + "dissect": "1.0.0", + "distributive": "6.0.0", + "dodo-printer": "2.2.1", + "dom-filereader": "7.0.0", + "dom-indexed": "11.0.0", + "dotenv": "3.0.0", + "droplet": "0.5.0", + "dynamic-buffer": "3.0.1", + "effect": "4.0.0", + "either": "6.1.0", + "elmish": "0.8.3", + "elmish-enzyme": "0.1.1", + "elmish-hooks": "0.8.2", + "elmish-html": "0.7.2", + "elmish-testing-library": "0.2.0", + "email-validate": "7.0.0", + "encoding": "0.0.8", + "enums": "6.0.1", + "error": "2.0.0", + "exceptions": "6.0.0", + "exists": "6.0.0", + "exitcodes": "4.0.0", + "expect-inferred": "3.0.0", + "fallback": "0.1.0", + "fast-vect": "0.7.0", + "fetch": "1.1.4", + "fetch-argonaut": "1.0.0", + "fetch-core": "4.0.4", + "fetch-yoga-json": "1.1.0", + "filterable": "5.0.0", + "fixed-points": "7.0.0", + "fixed-precision": "5.0.0", + "flame": "1.2.0", + "float32": "2.0.0", + "foldable-traversable": "6.0.0", + "foreign": "7.0.0", + "foreign-object": "4.0.0", + "foreign-readwrite": "3.0.0", + "fork": "6.0.0", + "form-urlencoded": "7.0.0", + "formatters": "7.0.0", + "free": "7.0.0", + "freeap": "7.0.0", + "freer-free": "0.0.1", + "freet": "7.0.0", + "functions": "6.0.0", + "functor1": "3.0.0", + "functors": "5.0.0", + "fuzzy": "0.4.0", + "gen": "4.0.0", + "generate-values": "1.0.1", + "generic-router": "0.0.1", + "geometry-plane": "1.0.3", + "github-actions-toolkit": "0.5.0", + "graphql-client": "9.2.2", + "graphs": "8.1.0", + "group": "4.1.1", + "halogen": "7.0.0", + "halogen-css": "10.0.0", + "halogen-formless": "4.0.2", + "halogen-hooks": "0.6.1", + "halogen-hooks-extra": "0.9.0", + "halogen-store": "0.5.4", + "halogen-storybook": "2.0.0", + "halogen-subscriptions": "2.0.0", + "halogen-svg-elems": "6.0.0", + "halogen-vdom": "8.0.0", + "halogen-vdom-string-renderer": "0.5.0", + "heckin": "2.0.1", + "heterogeneous": "0.6.0", + "heterogeneous-extrablatt": "0.2.1", + "homogeneous": "0.4.0", + "http-methods": "6.0.0", + "httpure": "0.15.0", + "httpurple": "3.0.0", + "httpurple-argonaut": "1.0.1", + "httpurple-yoga-json": "1.0.0", + "hyrule": "2.1.0", + "identity": "6.0.0", + "indexed-monad": "2.1.0", + "int64": "2.0.0", + "integers": "6.0.0", + "interpolate": "5.0.0", + "invariant": "6.0.0", + "jarilo": "1.0.1", + "jelly": "0.4.1", + "jest": "1.0.0", + "js-date": "8.0.0", + "js-fileio": "3.0.0", + "js-promise": "1.0.0", + "js-timers": "6.1.0", + "js-uri": "3.1.0", + "justifill": "0.5.0", + "jwt": "0.0.9", + "language-cst-parser": "0.12.1", + "lazy": "6.0.0", + "lazy-joe": "1.0.0", + "lcg": "4.0.0", + "leibniz": "5.0.0", + "linalg": "5.1.0", + "lists": "7.0.0", + "literals": "1.0.2", + "logging": "3.0.0", + "logging-journald": "0.4.0", + "machines": "7.0.0", + "matrices": "5.0.1", + "matryoshka": "1.0.0", + "maybe": "6.0.0", + "mdast-util-from-markdown": "0.2.1", + "media-types": "6.0.0", + "midi": "4.0.0", + "milkis": "9.0.0", + "minibench": "4.0.0", + "mmorph": "7.0.0", + "monad-control": "5.0.0", + "monad-logger": "1.3.1", + "monad-loops": "0.5.0", + "monad-unlift": "1.0.1", + "monoid-extras": "0.0.1", + "monoidal": "0.16.0", + "morello": "0.3.2", + "mote": "3.0.0", + "motsunabe": "2.0.0", + "nano-id": "1.1.0", + "naturals": "3.0.0", + "nested-functor": "0.2.1", + "newtype": "5.0.0", + "node-buffer": "8.0.0", + "node-buffer-blob": "1.0.0", + "node-child-process": "9.0.0", + "node-fs": "8.1.0", + "node-fs-aff": "9.1.0", + "node-http": "8.0.0", + "node-net": "4.0.0", + "node-path": "5.0.0", + "node-process": "10.0.0", + "node-readline": "7.0.0", + "node-sqlite3": "8.0.0", + "node-streams": "7.0.0", + "node-streams-aff": "4.0.0", + "node-url": "6.0.0", + "nonempty": "7.0.0", + "now": "6.0.0", + "npm-package-json": "2.0.0", + "nullable": "6.0.0", + "numbers": "9.0.0", + "ocarina": "1.3.0", + "open-folds": "6.3.0", + "open-memoize": "6.1.0", + "open-pairing": "6.1.0", + "options": "7.0.0", + "optparse": "5.0.0", + "ordered-collections": "3.0.0", + "ordered-set": "0.4.0", + "orders": "6.0.0", + "pairs": "9.0.0", + "parallel": "6.0.0", + "parsing": "10.0.0", + "parsing-dataview": "3.1.0", + "partial": "4.0.0", + "pathy": "9.0.0", + "pha": "0.9.0", + "phaser": "0.6.0", + "pipes": "8.0.0", + "point-free": "1.0.0", + "pointed-list": "0.5.1", + "polymorphic-vectors": "4.0.0", + "posix-types": "6.0.0", + "precise": "6.0.0", + "precise-datetime": "7.0.0", + "prelude": "6.0.1", + "prettier-printer": "3.0.0", + "profunctor": "6.0.0", + "profunctor-lenses": "8.0.0", + "protobuf": "4.0.0", + "ps-cst": "1.2.0", + "psa-utils": "8.0.0", + "psc-ide": "19.0.0", + "psci-support": "6.0.0", + "qualified-do": "2.2.0", + "quantities": "12.1.0", + "quickcheck": "8.0.1", + "quickcheck-combinators": "0.1.3", + "quickcheck-laws": "7.0.0", + "quickcheck-utf8": "0.0.0", + "random": "6.0.0", + "rationals": "5.0.0", + "react": "10.0.1", + "react-basic": "17.0.0", + "react-basic-classic": "3.0.0", + "react-basic-dnd": "10.1.0", + "react-basic-dom": "6.0.0", + "react-basic-emotion": "7.0.0", + "react-basic-hooks": "8.0.0", + "react-dom": "8.0.0", + "react-halo": "3.0.0", + "react-icons": "1.0.8", + "react-testing-library": "4.0.1", + "read": "1.0.1", + "record": "4.0.0", + "refs": "6.0.0", + "remotedata": "5.0.0", + "resource": "2.0.1", + "resourcet": "1.0.0", + "result": "1.0.3", + "return": "0.2.0", + "ring-modules": "5.0.1", + "rito": "0.1.0", + "routing": "11.0.0", + "routing-duplex": "0.6.0", + "run": "5.0.0", + "safe-coerce": "2.0.0", + "safely": "4.0.1", + "selection-foldable": "0.2.0", + "semirings": "7.0.0", + "signal": "13.0.0", + "simple-emitter": "2.0.0", + "simple-json": "9.0.0", + "sized-matrices": "1.0.0", + "sized-vectors": "5.0.2", + "slug": "3.0.8", + "soundfonts": "4.1.0", + "sparse-matrices": "1.2.1", + "sparse-polynomials": "1.0.5", + "spec": "7.0.0", + "spec-discovery": "8.0.1", + "spec-quickcheck": "5.0.0", + "splitmix": "2.1.0", + "ssrs": "1.0.0", + "st": "6.0.0", + "strictlypositiveint": "1.0.1", + "string-parsers": "8.0.0", + "strings": "6.0.1", + "strings-extra": "4.0.0", + "stringutils": "0.0.12", + "substitute": "0.2.3", + "sunde": "3.0.0", + "supply": "0.2.0", + "svg-parser": "3.0.0", + "systemd-journald": "0.3.0", + "tailrec": "6.1.0", + "test-unit": "17.0.0", + "thermite": "6.3.1", + "thermite-dom": "0.3.1", + "these": "6.0.0", + "toppokki": "4.0.0", + "transformers": "6.0.0", + "tree-rose": "4.0.2", + "tuples": "7.0.0", + "two-or-more": "1.0.0", + "type-equality": "4.0.1", + "typelevel": "6.0.0", + "typelevel-lists": "2.1.0", + "typelevel-peano": "1.0.1", + "typelevel-prelude": "7.0.0", + "typelevel-rows": "0.1.0", + "uint": "7.0.0", + "ulid": "3.0.1", + "uncurried-transformers": "1.1.0", + "undefined": "2.0.0", + "undefined-is-not-a-problem": "1.1.0", + "unfoldable": "6.0.0", + "unicode": "6.0.0", + "unlift": "1.0.1", + "unordered-collections": "3.0.1", + "unsafe-coerce": "6.0.0", + "unsafe-reference": "5.0.0", + "untagged-union": "1.0.0", + "uri": "9.0.0", + "uuid": "9.0.0", + "validation": "6.0.0", + "variant": "8.0.0", + "vectorfield": "1.0.1", + "versions": "7.0.0", + "web-clipboard": "4.1.0", + "web-cssom": "2.0.0", + "web-dom": "6.0.0", + "web-dom-parser": "8.0.0", + "web-dom-xpath": "3.0.0", + "web-encoding": "3.0.0", + "web-events": "4.0.0", + "web-fetch": "3.0.0", + "web-file": "4.0.0", + "web-html": "4.1.0", + "web-pointerevents": "1.0.0", + "web-promise": "3.1.0", + "web-router": "1.0.0", + "web-socket": "4.0.0", + "web-storage": "5.0.0", + "web-streams": "3.0.0", + "web-touchevents": "4.0.0", + "web-uievents": "4.0.0", + "web-url": "2.0.0", + "web-workers": "1.1.0", + "web-xhr": "5.0.0", + "which": "2.0.0", + "yoga-fetch": "1.0.1", + "yoga-json": "3.0.2", + "yoga-postgres": "6.0.0" + } + }, + "extra_packages": { + "either": { + "git": "https://github.com/purescript/purescript-either.git", + "ref": "af655a04ed2fd694b6688af39ee20d7907ad0763", + "dependencies": [ + "control", + "invariant", + "maybe", + "prelude" + ] + } + } + }, + "packages": { + "control": { + "type": "registry", + "version": "6.0.0", + "integrity": "sha256-sH7Pg9E96JCPF9PIA6oQ8+BjTyO/BH1ZuE/bOcyj4Jk=", + "dependencies": [ + "newtype", + "prelude" + ] + }, + "either": { + "type": "git", + "url": "https://github.com/purescript/purescript-either.git", + "rev": "af655a04ed2fd694b6688af39ee20d7907ad0763", + "dependencies": [ + "control", + "invariant", + "maybe", + "prelude" + ] + }, + "invariant": { + "type": "registry", + "version": "6.0.0", + "integrity": "sha256-RGWWyYrz0Hs1KjPDA+87Kia67ZFBhfJ5lMGOMCEFoLo=", + "dependencies": [ + "control", + "prelude" + ] + }, + "maybe": { + "type": "registry", + "version": "6.0.0", + "integrity": "sha256-5cCIb0wPwbat2PRkQhUeZO0jcAmf8jCt2qE0wbC3v2Q=", + "dependencies": [ + "control", + "invariant", + "newtype", + "prelude" + ] + }, + "newtype": { + "type": "registry", + "version": "5.0.0", + "integrity": "sha256-gdrQu8oGe9eZE6L3wOI8ql/igOg+zEGB5ITh2g+uttw=", + "dependencies": [ + "prelude", + "safe-coerce" + ] + }, + "prelude": { + "type": "registry", + "version": "6.0.1", + "integrity": "sha256-o8p6SLYmVPqzXZhQFd2hGAWEwBoXl1swxLG/scpJ0V0=", + "dependencies": [] + }, + "safe-coerce": { + "type": "registry", + "version": "2.0.0", + "integrity": "sha256-a1ibQkiUcbODbLE/WAq7Ttbbh9ex+x33VCQ7GngKudU=", + "dependencies": [ + "unsafe-coerce" + ] + }, + "unsafe-coerce": { + "type": "registry", + "version": "6.0.0", + "integrity": "sha256-IqIYW4Vkevn8sI+6aUwRGvd87tVL36BBeOr0cGAE7t0=", + "dependencies": [] + } + } +} diff --git a/test-fixtures/git-declared-deps/with-declared-deps.yaml b/test-fixtures/git-declared-deps/with-declared-deps.yaml new file mode 100644 index 000000000..562716338 --- /dev/null +++ b/test-fixtures/git-declared-deps/with-declared-deps.yaml @@ -0,0 +1,17 @@ +package: + name: test-declared-deps + dependencies: + - either + +workspace: + packageSet: + registry: 0.0.1 + extraPackages: + either: + git: https://github.com/purescript/purescript-either.git + ref: af655a04ed2fd694b6688af39ee20d7907ad0763 + dependencies: + - control + - invariant + - maybe + - prelude diff --git a/test-fixtures/git-declared-deps/without-declared-deps.yaml b/test-fixtures/git-declared-deps/without-declared-deps.yaml new file mode 100644 index 000000000..0066dc94b --- /dev/null +++ b/test-fixtures/git-declared-deps/without-declared-deps.yaml @@ -0,0 +1,13 @@ +package: + name: consumer + dependencies: + - mylib + +workspace: + packageSet: + registry: 0.0.1 + extraPackages: + mylib: + git: + ref: main + # No dependencies declared - spago must clone to read spago.yaml diff --git a/test-fixtures/offline.txt b/test-fixtures/offline.txt index 76f0161df..020b406d0 100644 --- a/test-fixtures/offline.txt +++ b/test-fixtures/offline.txt @@ -3,4 +3,4 @@ Reading Spago workspace configuration... ✓ Selecting package to build: eee -✘ You are offline and the repo 'https://github.com/purescript/purescript-either.git' is not available locally, can't make progress. +✘ Package 'either' is not in the local cache, and Spago is running in offline mode - can't make progress. diff --git a/test/Prelude.purs b/test/Prelude.purs index 38823edac..67e234973 100644 --- a/test/Prelude.purs +++ b/test/Prelude.purs @@ -421,3 +421,35 @@ assertWarning paths shouldHave stdErr = do <> "\n\nStderr was:\n" <> stdErr +-- | Run a git command in the current directory +git :: Array String -> Aff Unit +git = git' Nothing + +-- | Run a git command in a specific directory +git' :: Maybe GlobalPath -> Array String -> Aff Unit +git' cwd args = + Cmd.exec (Path.global "git") args + (Cmd.defaultExecOptions { pipeStdout = false, pipeStderr = false, pipeStdin = StdinNewPipe, cwd = cwd }) + >>= shouldBeSuccess + +-- | Create a git repo with a spago.yaml in the test's temp directory. +-- | Cleanup is automatic when the test's temp directory is removed. +mkGitRepo + :: RootPath + -> { name :: String, deps :: Array String } + -> Aff LocalPath +mkGitRepo testCwd { name, deps } = do + let repo = testCwd ("lib-" <> name) + FS.mkdirp repo + let depsYaml = if Array.null deps then "[]" else "\n" <> foldMap (\d -> " - " <> d <> "\n") deps + FS.writeTextFile (repo "spago.yaml") $ + "package:\n name: " <> name <> "\n dependencies: " <> depsYaml <> "\nworkspace:\n packageSet:\n registry: 0.0.1\n" + FS.mkdirp (repo "src") + FS.writeTextFile (repo "src/Main.purs") $ "module " <> String.toUpper name <> ".Main where\n" + git' (Just $ Path.toGlobal repo) [ "init", "-b", "main" ] + git' (Just $ Path.toGlobal repo) [ "config", "user.name", "test" ] + git' (Just $ Path.toGlobal repo) [ "config", "user.email", "test@test.com" ] + git' (Just $ Path.toGlobal repo) [ "add", "." ] + git' (Just $ Path.toGlobal repo) [ "commit", "-m", "initial" ] + pure repo + diff --git a/test/Spago/Install.purs b/test/Spago/Install.purs index 2a382ef1e..0910855ea 100644 --- a/test/Spago/Install.purs +++ b/test/Spago/Install.purs @@ -7,10 +7,10 @@ import Test.Prelude import Data.Array as Array import Data.Map as Map +import Data.String as String import Effect.Now as Now import Registry.Version as Version import Spago.Command.Init as Init -import Spago.Cmd as Cmd import Spago.Core.Config (Dependencies(..), Config) import Spago.Core.Config as Config import Spago.FS as FS @@ -138,6 +138,61 @@ spec = Spec.around withTempDir do writeConfigWithEither testCwd spago [ "install", "--offline", "either" ] >>= shouldBeFailureErr (fixture "offline.txt") + Spec.it "skips cloning during resolution when git package has declared deps" \{ spago, testCwd, fixture } -> do + FS.copyFile { src: fixture "git-declared-deps/with-declared-deps.yaml", dst: testCwd "spago.yaml" } + FS.mkdirp (testCwd "src") + FS.writeTextFile (testCwd "src/Main.purs") "module Main where" + + -- first build: cloning should happen AFTER "Downloading dependencies..." (during fetch phase) + result1 <- spago [ "build" ] + result1 # shouldBeSuccess + let stderr1 = either _.stderr _.stderr result1 + let downloadingIdx = String.indexOf (String.Pattern "Downloading dependencies...") stderr1 + let cloningIdx = String.indexOf (String.Pattern "Cloning") stderr1 + case downloadingIdx, cloningIdx of + Just d, Just c -> d `Assert.shouldSatisfy` \_ -> d < c + _, _ -> Assertions.fail $ "Expected 'Downloading dependencies...' before 'Cloning' but got:\n" <> stderr1 + + -- Verify lockfile was created with correct content + checkFixture (testCwd "spago.lock") (fixture "git-declared-deps/spago.lock") + + -- Second build: remove .spago cache, lockfile should still defer cloning to fetch phase + rmRf (testCwd ".spago") + result2 <- spago [ "build" ] + result2 # shouldBeSuccess + let stderr2 = either _.stderr _.stderr result2 + let downloadingIdx2 = String.indexOf (String.Pattern "Downloading dependencies...") stderr2 + let cloningIdx2 = String.indexOf (String.Pattern "Cloning") stderr2 + case downloadingIdx2, cloningIdx2 of + Just d, Just c -> d `Assert.shouldSatisfy` \_ -> d < c + _, _ -> Assertions.fail $ "Expected 'Downloading dependencies...' before 'Cloning' but got:\n" <> stderr2 + -- Lockfile should not be regenerated + when (isJust $ String.indexOf (String.Pattern "generating it") stderr2) do + Assertions.fail $ "Lockfile was regenerated unexpectedly:\n" <> stderr2 + + -- Third build (offline): should work because package is cached + spago [ "build", "--offline" ] >>= shouldBeSuccess + + Spec.it "must clone during resolution when git package has no declared deps" \{ spago, testCwd, fixture } -> do + libRepo <- mkGitRepo testCwd { name: "mylib", deps: [ "prelude" ] } + -- Set up consumer project with git dep (no declared deps) + FS.mkdirp (testCwd "src") + FS.writeTextFile (testCwd "src/Main.purs") "module Main where\nimport MYLIB.Main\n" + -- Use fixture with placeholder replacement (like 1208 test) + content <- FS.readTextFile $ fixture "git-declared-deps/without-declared-deps.yaml" + FS.writeTextFile (testCwd "spago.yaml") $ + String.replaceAll (String.Pattern "") (String.Replacement $ Path.toRaw libRepo) content + -- Without declared deps, spago must clone to read spago.yaml from the repo. + -- "Cloning" should appear BEFORE "Downloading dependencies..." + result <- spago [ "build" ] + result # shouldBeSuccess + let stderr = either _.stderr _.stderr result + let downloadingIdx = String.indexOf (String.Pattern "Downloading dependencies...") stderr + let cloningIdx = String.indexOf (String.Pattern "Cloning") stderr + case downloadingIdx, cloningIdx of + Just d, Just c -> c `Assert.shouldSatisfy` \_ -> c < d + _, _ -> Assertions.fail $ "Expected 'Cloning' before 'Downloading dependencies...' but got:\n" <> stderr + Spec.it "installs a package version by branch name with / in it" \{ spago, testCwd } -> do spago [ "init" ] >>= shouldBeSuccess let @@ -272,7 +327,6 @@ spec = Spec.around withTempDir do spago [ "install", "either" ] >>= shouldBeSuccess checkFixture (testCwd "spago.yaml") (fixture "spago-install-solver-ranges.yaml") - insertConfigDependencies :: Config -> Dependencies -> Dependencies -> Config insertConfigDependencies config core test = ( config @@ -382,12 +436,3 @@ forceResetSpec = Spec.around withTempDir do content1 `shouldEqualStr` "content1" content2 <- FS.readTextFile (cachedRepo "file2.txt") content2 `shouldEqualStr` "content2-updated" - -git :: Array String -> Aff Unit -git = git' Nothing - -git' :: Maybe GlobalPath -> Array String -> Aff Unit -git' cwd args = - Cmd.exec (Path.global "git") args - (Cmd.defaultExecOptions { pipeStdout = false, pipeStderr = false, pipeStdin = StdinNewPipe, cwd = cwd }) - >>= shouldBeSuccess diff --git a/test/Spago/Publish.purs b/test/Spago/Publish.purs index 016ed5aed..2aeb47894 100644 --- a/test/Spago/Publish.purs +++ b/test/Spago/Publish.purs @@ -11,9 +11,7 @@ import Data.String.Regex as Regex import Data.String.Regex.Flags as RF import Node.Platform as Platform import Node.Process as Process -import Spago.Cmd as Cmd import Spago.FS as FS -import Spago.Path as Path import Test.Spec (Spec) import Test.Spec as Spec @@ -195,12 +193,3 @@ doTheGitThing = do git [ "commit", "-m", "first" ] git [ "tag", "v0.0.1" ] git [ "remote", "add", "origin", "git@github.com:purescript/aaa.git" ] - -git :: Array String -> Aff Unit -git = git' Nothing - -git' :: Maybe GlobalPath -> Array String -> Aff Unit -git' cwd args = - Cmd.exec (Path.global "git") args - (Cmd.defaultExecOptions { pipeStdout = false, pipeStderr = false, pipeStdin = StdinNewPipe, cwd = cwd }) - >>= shouldBeSuccess From 4c7e8c0427d43d6939c32300395cb6ab6399cd86 Mon Sep 17 00:00:00 2001 From: Fabrizio Ferrai Date: Sat, 24 Jan 2026 17:46:56 +0200 Subject: [PATCH 2/4] Match map keys instead of size --- src/Spago/Command/Fetch.purs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Spago/Command/Fetch.purs b/src/Spago/Command/Fetch.purs index aa22a077e..e38c8fb43 100644 --- a/src/Spago/Command/Fetch.purs +++ b/src/Spago/Command/Fetch.purs @@ -229,7 +229,7 @@ run { packages: packagesRequestedToInstall, ensureRanges, isTest, isRepl } = do -- 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.size allDeps /= Map.size depsToFetch) do + when (Map.keys allDeps /= Map.keys depsToFetch) do fetchPackagesToLocalCache allDeps writeNewLockfile reason dependencies From 733b4745fdc0edd2baad99e610da630a1c49b651 Mon Sep 17 00:00:00 2001 From: Fabrizio Ferrai Date: Sat, 24 Jan 2026 19:17:01 +0200 Subject: [PATCH 3/4] Shorten directory length for cloned repos on windows --- src/Spago/Config.purs | 35 +++++++++++++++++++++++++++++------ src/Spago/Prelude.purs | 2 +- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 958cf7694..8a13d7fd2 100644 --- a/src/Spago/Config.purs +++ b/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 diff --git a/src/Spago/Prelude.purs b/src/Spago/Prelude.purs index 9fa336f7a..6144f420e 100644 --- a/src/Spago/Prelude.purs +++ b/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 From 232cfea76ada31acb765ea71f9fc30952089a3ee Mon Sep 17 00:00:00 2001 From: Fabrizio Ferrai Date: Sat, 24 Jan 2026 20:10:13 +0200 Subject: [PATCH 4/4] Fix fixture --- test/Spago/Install.purs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Spago/Install.purs b/test/Spago/Install.purs index 0910855ea..a99d2821a 100644 --- a/test/Spago/Install.purs +++ b/test/Spago/Install.purs @@ -220,7 +220,7 @@ spec = Spec.around withTempDir do } ) spago [ "install", "nonexistent-package" ] >>= shouldBeSuccess - let slashyPath = testCwd Paths.localCachePackagesPath "nonexistent-package" "spago-test%2fbranch-with-slash" + let slashyPath = testCwd Paths.localCachePackagesPath "nonexistent-package" "spago-test%sbranch-with-slash" unlessM (FS.exists slashyPath) do Assertions.fail $ "Expected path to exist: " <> Path.quote slashyPath kids <- FS.ls slashyPath