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

Prevent double cloning of git repos when fetching them in parallel #1291

Merged
merged 8 commits into from
Sep 28, 2024

Conversation

f-f
Copy link
Member

@f-f f-f commented Sep 28, 2024

Fix #1206: in #1275 we introduced a cache for git repos so we would not clone them more than once - however this didn't cover the case where we have a valid lockfile and we start with an empty .spago folder (such as the case of a fresh clone of a repo).

This patch introduces a lock for git repos so we clone them only once even when multiple fibers are trying to clone them in parallel. We also add a test that reproduces the issue exactly, and behaves well with this patch.

@f-f f-f requested a review from fsoikin September 28, 2024 15:12
@f-f
Copy link
Member Author

f-f commented Sep 28, 2024

@fsoikin any idea what's up with windows here? The two fixtures are identical...

@fsoikin
Copy link
Collaborator

fsoikin commented Sep 28, 2024

I will take a look later (afk right now), but my first guess would be line breaks.

@fsoikin
Copy link
Collaborator

fsoikin commented Sep 28, 2024

Yep, it's the newlines. They matter sometimes, but not always. I haven't figured out precisely when, but I suspect that the Purs output has platform-dependent newlines in it. That's why it worked before, but not after you added a build, and that's why it started working after you cut out the Purs output.

I pushed a fix. I don't have a lot of time now, so I just added another replaceAll, but I think this whole sanitize business needs to be systematized and extracted in blocks for reuse for better ergonomics.

Co-authored-by: Fyodor Soikin <[email protected]>
@f-f
Copy link
Member Author

f-f commented Sep 28, 2024

Ah yeah this all makes sense. Thanks @fsoikin for pushing the fix!

@f-f f-f merged commit 3b28998 into master Sep 28, 2024
5 checks passed
@f-f f-f deleted the no-double-cloning-take2 branch September 28, 2024 23:23
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.

"dest already exists" error
2 participants