Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clone Git monorepos with several dependencies only once #1275

Merged
merged 23 commits into from
Aug 28, 2024

Conversation

fsoikin
Copy link
Contributor

@fsoikin fsoikin commented Aug 27, 2024

Description of the change

Fixed #1208

  • The git repos are cached in .spago/g/.
  • When Spago would previously clone a repo, it will now look in cache first, and only clone when not found.

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • [ ] Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

Comment on lines 12 to 27
deku:
git: https://github.com/mikesol/purescript-deku.git
subdir: deku
ref: 276f48adde3d9354f61917f7e9ae2ae7b43df6b2
deku-core:
git: https://github.com/mikesol/purescript-deku.git
subdir: deku-core
ref: 98c67533cc8c399aa643b495d3c02bab963e5b80
deku-css:
git: https://github.com/mikesol/purescript-deku.git
subdir: deku-css
ref: 4e68a5cec10c91aa3377ce69cc97c276936a1194
deku-dom:
git: https://github.com/mikesol/purescript-deku.git
subdir: deku-dom
ref: 6b7c392da7782fe0f2e34811e36b11e630e10b26
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used Mike's example from #1208, but with specific SHAs. This should be fine until the repo is deleted or its history is rewritten.

core/src/FS.purs Outdated
@@ -15,6 +15,7 @@ module Spago.FS
, readTextFile
, readYamlDocFile
, readYamlFile
, rmRf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't like this in application code - it looks like it's only used for tests so we should move it into the Test Prelude or just in the Test.Spago.Build.Monorepo module

src/Spago/Command/Fetch.purs Show resolved Hide resolved
@f-f
Copy link
Member

f-f commented Aug 27, 2024

@fsoikin could I ask you to not force-push? Seeing all the commits is actually useful for reviewing things, and we squash-merge all the PRs so there's no effect on what ends up in trunk.

src/Spago/Command/Fetch.purs Outdated Show resolved Hide resolved
Copy link
Member

@f-f f-f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing patch, great work 💯

src/Spago/Prelude.purs Show resolved Hide resolved
test/Spago/Build/Monorepo.purs Show resolved Hide resolved
@f-f f-f merged commit dbc6f44 into purescript:master Aug 28, 2024
5 checks passed
@f-f f-f mentioned this pull request Aug 28, 2024
3 tasks
@fsoikin fsoikin deleted the no-double-git-cloning branch August 28, 2024 17:37
@ysangkok
Copy link
Contributor

ysangkok commented Sep 5, 2024

Thank you, we have tested this and it works great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spago install git clones for same monorepo package multiple times
3 participants