crystal-lang / shards

Dependency manager for the Crystal language
Other
758 stars 99 forks source link

`shards install --local` doesn't recognize cached shards #628

Open straight-shoota opened 2 months ago

straight-shoota commented 2 months ago

When a git repository is cached, shards should use that and only that with --local (no network requests). This does not seem to work, however. The cache contains bare repositories (only the git metadata without a worktree) which causes some issues with git commands.

In https://github.com/crystal-lang/shards/issues/611#issuecomment-2073641481 I noticed a particular issue with GitResolver#origin_url which runs git ls-remote --get-url origin in the cache directory. But git doesn't know its a bare repository and searches for the next .git folder up in the directory tree. If the cache path is in the work tree of another git repository, it'll result in this repository's origin, which is obviously not what we're looking for. If it's not in the work tree of another git repository (which should typically be the case with the default SHARDS_CACHE_PATH), the result is origin. This is a bit surprising and also not what what we're looking for. So I'm wondering how (or if) origin_url even works in the first place?

This should technically be easy to solve by adding the flag --bare to let git now it's working on a bare repository. However we need to understand a bit more why this issue is happening and why it's not a bigger problem. I'm confused how other git commands seem to work with a bare repository just fine, even though they should be affected in a similar way.

I presume a similar issue might exist for other resolvers as well.

ysbaddaden commented 2 months ago

See https://github.com/crystal-lang/shards/commit/adefcf3a725ec77961154510b1a4cb6c8ac96cbc

The origin changed, and Shards goes to delete the cloned repository to clone the new origin. Hence new origin => delete clone => new clone => fails because we specified --local.

ysbaddaden commented 2 months ago

There are no issues with the bare repositories in the standard cache path (~/.cache/shards). The problem is that Git likely becomes confused by the presence of a .git folder in a parent folder when the cache is set to be in the subdirectory of a Git project.

We probably need to specify GIT_DIR or --git-dir to specify the path

straight-shoota commented 2 months ago

Yeah, I figure we can even drop run_in_folder for git commands and just configure the git dir to point local_path.

straight-shoota commented 2 months ago

The problem is that Git likely becomes confused by the presence of a .git folder in a parent folder when the cache is set to be in the subdirectory of a Git project.

Yes, that seems to be it. Only if git cannot determine it's in a work tree (i.e. neither the current directory nor any of its parents have a .git folder) it tries to treat the current directory as a bare repo, even without --bare. That's super confusing because the implicit recognition as a bare repository depends on an unrelated fact.

GrantBirki commented 1 week ago

Has there been any progress made around vendoring shards since the last comment on this issue?