ninenines / erlang.mk

A build tool for Erlang that just works.
https://erlang.mk
ISC License
578 stars 241 forks source link

Allow git + hex deps to be cached #968

Closed artman41 closed 1 year ago

artman41 commented 1 year ago
artman41 commented 1 year ago

The reasoning behind this MR is that, recently, I've been doing a lot of offline dev & it's quite frustrating when you know you have the git repo in a project somewhere locally, but not sure where.

Aside from offline benefits, this MR should also speed up subsequent deps calls after a distclean if you're on a slow connection & using the cache.

essen commented 1 year ago

Not a bad idea but I think you are missing a git pull when the repository already exists otherwise it might not be able to checkout new branches/tags?

artman41 commented 1 year ago

Not a bad idea but I think you are missing a git pull when the repository already exists otherwise it might not be able to checkout new branches/tags?

Good shout, threw a git pull in there after the git fetch - anything else you can think of?

Unsure whether it's worthwhile symlinking from the $(CACHE_DIR) or whether using it as a clone source is good enough

essen commented 1 year ago

A clone would work out better across projects, for example if one project depends on a version and another a different version. Which means that the git checkout after the pull is probably not necessary. Also git pull includes git fetch so that's not necessary either, one or the other should be good enough. Not sure if you had git fetch before, maybe I didn't look hard enough.

Other than that, I think having some tests would be good. I'll see if some folks want to try it tomorrow.

artman41 commented 1 year ago

A clone would work out better across projects, for example if one project depends on a version and another a different version. Which means that the git checkout after the pull is probably not necessary. Also git pull includes git fetch so that's not necessary either, one or the other should be good enough. Not sure if you had git fetch before, maybe I didn't look hard enough.

Other than that, I think having some tests would be good. I'll see if some folks want to try it tomorrow.

Sounds good, I've had it in the past where a pull only pulled the latest refs for the branches I had locally (though I guess if we change it to git pull --all then it'll likely pull everything)

Ref the git checkout - you're probably right that we can remove that, now we're using it as a local git repo, leaving it checked out on to a branch doesn't really have a purpose for us.

What're you thinking test wise?

My current idea is the following (otherwise I'm not sure how you'd be able to tell you've cloned it from the cache only)

I also had a realisation while typing this that it'd likely be worthwhile to have a target to clear the cache (maybe with the ability to specify a specific dep?) - what're your thoughts on that?

essen commented 1 year ago

The tests have code you can reuse that creates repos. I would not recommend modifying the cache within the test since we want to test the cache as well not just the code. There's probably a bunch of different scenarios to try, but separate git/subfolder/hex would be a good start. Also testing commits, branches and tags and making sure different projects can depend on different versions and they all can use the same cache.

About clearing the cache, nothing beats rm -rf ~/.cache/erlang.mk/...

artman41 commented 1 year ago

The tests have code you can reuse that creates repos. I would not recommend modifying the cache within the test since we want to test the cache as well not just the code. There's probably a bunch of different scenarios to try, but separate git/subfolder/hex would be a good start. Also testing commits, branches and tags and making sure different projects can depend on different versions and they all can use the same cache.

About clearing the cache, nothing beats rm -rf ~/.cache/erlang.mk/...

essen commented 1 year ago

Merged with some tweaks and a fix for Hex which couldn't fetch if the package was in the cache (misplaced ;). Thanks!