-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fix cache failing to resolve dependencies with name collision #2286
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR! We always appreciate when people take the time to fix issues that they encounter. Unfortunately, this isn't the solution we've been hoping for for this particular issue. As noted in #569:
The best way to fix this would be to share the repository across all projects with the same name—but to use a different remote for each project. This allows projects with the same name to share the same object database, potentially saving a lot of disk space and preventing some expensive clones, but would correct the issue you've run up against. |
I thought about that as an option, but it seems more risky and error prone, for not much gain. As I see the key problems being
That means that it will affect enumerations of versions, and semvar checking. It might be possible to use the remote name prefix for all the git commands, but it certainly complicates the process. When the gain in most cases is 10s of megabytes, and also, people are probably not changing between forks very much, and if they are its unlikely to be more than 1 fork. So this seemed like a simple way to make the cache unique and avoid these kind of issues. I am actually now thinking that the non-github types should probably just use the whole url as the cache key. I think reliability is the most important part of the cache, it shouldn't give the wrong result. This is particularly true for CI systems that might be automatically building and uploading to the store. |
Hmm… it seems you're right as far as tags name go. Branches aren't typically a problem since they can be uniqued by origin. Maybe we can share objects between repositories? That leads me to think that the best namespacing would be |
Looking at It comes with this warning about using shared objects,
In my mind, this just bring me back to my previous points
|
8f5c926
to
c267774
Compare
Okay, I'm on board with using separate repos without worrying about sharing objects. But I think we need two things to finish this up:
|
@ikesyo Do you have any concerns about this? |
I've been thinking about how change over the current cache, should be simple enough to detect folder with the old name and rename it. Can I suggest that we stick to flat folder names
|
From what I've read, seems this scenario can be mitigated by the – |
I read about |
ff56392
to
96133fe
Compare
I'm not sure I have a preference. What do you think? |
I think this is a mistake. Caches are just caches; it's fine for them to move if it's very infrequent. I think not reusing/moving the old cache is fine in order to change to an improved layout that fixes a bug. I don't really want to maintain the logic of moving old caches going forward. It adds a continued maintenance cost; I'd rather pay a one-time user cost. I also don't think the method used to move the cache in 43db3dd is appropriate. It adds disk access and side effects into what should remain a pure method.
This shouldn't really be burdensome. We already need to validate that the Cache directory as a whole exists. I think that the best solution here is to move the cache to |
Prevents failed checkout when changing between forks
- Change github names to name_owner
So, playing with this a bit - here is the commit, on alt branch for the moment, with the cache in sub folders, mcfedr@5d0b692, but now I come down to editing
Again, this just feels overly complicated, it would be much simpler to use a flat folder that doesn't conflict with existing cache dirs, the cache is a machine readable cache, its not for people. |
43db3dd
to
5b8995a
Compare
I don't think any of this needs to be done. That's why I suggested using |
ping |
Ill be honest, unfortunately, due to time constraints, I'm unlikely to do any more work on this, so please feel to pick up my fork and finish it, I think its pretty close to being ready to go. |
No problem at all. Your effort will not go wasted. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Fixes #2150
Changes the cache dir to owner_name which should prevent 99% of cache failures.
The problem happens when I had
github "mcfedr/library" ~> 1
in one project, andgithub "someone/library" ~> 1
in another project, as they both get caches in the folderlibrary
.Now they will be cached separately, and nothing will break.
Bonus, when testing this I noticed a few cases of url that might have issues where the name isnt correctly resolved, largely going to be edge cases as I imagine nearly all deps are coming from github anyway, but the sort of thing that would be annoying when you run into it.