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

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

Open
straight-shoota opened this issue Apr 24, 2024 · 10 comments
Open

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

straight-shoota opened this issue Apr 24, 2024 · 10 comments
Labels

Comments

@straight-shoota
Copy link
Member

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 #611 (comment) 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
Copy link
Contributor

ysbaddaden commented Apr 24, 2024

See adefcf3

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
Copy link
Contributor

ysbaddaden commented Apr 24, 2024

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
Copy link
Member Author

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
Copy link
Member Author

straight-shoota commented Apr 24, 2024

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
Copy link

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

@GrantBirki
Copy link

👋 Hey again @straight-shoota! I just wanted to check in to see there has been any updates around this issue. Thanks! 🙇

@ysbaddaden
Copy link
Contributor

We welcome pull requests if anybody's willing to fix an issue.

@GrantBirki
Copy link

@ysbaddaden If no one else is currently taking a shot at fixing this, I can try and allocate some time to take a look in the next several weeks when I'm free. I haven't contributed to this repo before but I'll give it a go 🤞

@GrantBirki
Copy link

@ysbaddaden I have really been trying for a few days now and really can't grasp what the heck is going on here.

I: Resolving dependencies
D: git_url: https://github.com/octokit-cr/octokit.cr.git
D: running with --local and repository cache was found: /home/runner/work/crystal-base-template/crystal-base-template/.cache/shards/github.com/octokit-cr/octokit.cr.git
D: git_url: https://github.com/crystal-ameba/ameba.git
D: running with --local and repository cache was found: /home/runner/work/crystal-base-template/crystal-base-template/.cache/shards/github.com/crystal-ameba/ameba.git
D: command: `git ls-remote --get-url origin --bare` path: `/home/runner/work/crystal-base-template/crystal-base-template/.cache/shards/github.com/octokit-cr/octokit.cr.git` capture: `true` local_path: `/home/runner/work/crystal-base-template/crystal-base-template/.cache/shards/github.com/octokit-cr/octokit.cr.git`
D: command: `git ls-remote --get-url origin --bare` path: `/home/runner/work/crystal-base-template/crystal-base-template/.cache/shards/github.com/crystal-ameba/ameba.git` capture: `true` local_path: `/home/runner/work/crystal-base-template/crystal-base-template/.cache/shards/github.com/crystal-ameba/ameba.git`
E: Failed git ls-remote --get-url origin --bare.
Segmentation fault (core dumped)

Is what I get in my logs when trying to run install shards locally with my changes from this pull request. I'm feeling pretty defeated trying to solve this problem and figure out why --local doesn't care about cached shards at all.

@GrantBirki
Copy link

GrantBirki commented Sep 3, 2024

I was unable to get shards to properly recognize locally cached gems with any of the following:

  • Using git -C <path>
  • Using the --git-dir param
  • Using GIT_DIR via env vars

This was all done is dozens of different ways via the command and the run_in_folder methods with zero success.

Running native git commands on my system worked fine. Running the same commands in CI (Linux GitHub Actions runners) also worked fine. Even running similar function calls through crystal run tmp/test.cr on my system still worked. But I simply could not get it to work the same when making these changes and compiling the shards binary.

My next course of action is to attempt to read the git config file directly and just parse the remote out of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants