spago
spago copied to clipboard
purescript
Introduce global git cache
Description of the change
Another attempt at #1238. This introduces a global git cache which is used for non-branches. That means that if you have a tag/commit specified on multiple dependencies, it will only be cloned once.
Checklist:
- [X] 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)
Ah right, CI fails because this change is correct 🙂
In the can't install (uncached) dependencies if offline test we were verifying that git repos never get cached, but with this change we hit the cache when trying to clone the same repo twice. We'll have to clone a different version of either in there, and/or add a test where we clone a branch.
@f-f I applied your suggested changes, and also an mkdirp for the deeper directory hierarchy. The reason I joined package.git with package.ref is because there is no synchronization outside the filesystem, so if there is a subdirectory, I am worried that the more work happen between the call to exists and the mkdirp and the moveSync, the more likely threads are to interleave any of this, which wouldn't work. Ideally, the move should be called unconditionally, and it fails because of an existing destination, the failure could be ignored. That would be correct on many Linux filesystems since moves are atomic. Now that we have three file system operations, it is not atomic. Not sure what to do about this.
I don't know how to change the version of either, so I tried to put identity instead, but it doesn't seem to have worked. I wonder if identity is somehow already in the cache. Not sure what other package to use.
@f-f Let me know if there are any remaining tasks for this, please! I have fixed the formatting.
CI fails because on Windows (and only there) the folder in the cache is not found when we go copy it:
Error: ENOENT: no such file or directory, lstat 'C:\\Users\\runneradmin\\AppData\\Local\\spago-nodejs\\Cache\\git\\https%3a%2f%2fgithub.com%2fpurescript%2fpurescript-either.git\\af655a04ed2fd694b6688af39ee20d7907ad0763'
at Object.lstatSync (node:fs:1666:3)
at Object.lstatSync (D:\\a\\spago\\spago\\node_modules\\graceful-fs\\polyfills.js:318:34)
at statFunc (D:\\a\\spago\\spago\\node_modules\\fs-extra\\lib\\util\\stat.js:24:20)
at getStatsSync (D:\\a\\spago\\spago\\node_modules\\fs-extra\\lib\\util\\stat.js:25:19)
at Object.checkPathsSync (D:\\a\\spago\\spago\\node_modules\\fs-extra\\lib\\util\\stat.js:64:33)
at Object.copySync (D:\\a\\spago\\spago\\node_modules\\fs-extra\\lib\\copy\\copy-sync.js:27:38)
at file:///D:/a/spago/spago/output/Spago.FS/foreign.js:9:54
at runSync (file:///D:/a/spago/spago/output/Effect.Aff/foreign.js:88:20)
at run (file:///D:/a/spago/spago/output/Effect.Aff/foreign.js:326:22)
at file:///D:/a/spago/spago/output/Effect.Aff/foreign.js:346:19 {
errno: -4058,
syscall: 'lstat',
code: 'ENOENT',
path: 'C:\\\\Users\\\\runneradmin\\\\AppData\\\\Local\\\\spago-nodejs\\\\Cache\\\\git\\\\https%3a%2f%2fgithub.com%2fpurescript%2fpurescript-either.git\\\\af655a04ed2fd694b6688af39ee20d7907ad0763'
}
I had weird encounters with windows not writing files to disk in a timely manner, but it would be strange if it would do it for entire folders. I wonder if we're writing to a place and trying to read from another, or if some of the characters we use in the escaping are not good
I can't seem to reproduce this failure locally. So not sure how to proceed. I could insert some delays between tests, but that wouldn't be an acceptable permanent solution.
Ah that's strange - I wonder if this is about Node versions? In CI we're using a pretty old one, 18: https://github.com/purescript/spago/blob/52999702842304b360b24f57571523564d261823/.github/workflows/build.yml#L41
Bumping it up to 22 might just fix it. If not I'll try to take a look on my Windows machine sometime in the next days
Is there a way to get the test runner to just run a single test? That would help with diagnosing whether it's the state carried over between tests that is messing up the failing test.
Trying this out on my windows machine, I can't reproduce it either unfortunately. I'd really like to merge this ASAP - but adding debug prints to CI seems to be the only way to debug this, so it will be slow