feat: support installing gems from Git#312
feat: support installing gems from Git#312withered-magic wants to merge 11 commits intobazel-contrib:mainfrom
Conversation
| @@ -0,0 +1,29 @@ | |||
| # frozen_string_literal: true | |||
| require 'fileutils' | |||
There was a problem hiding this comment.
Would it be reasonable to add a high-level comment on what this file does?
I think it's basically rsync'ing gemspec/extconf.rb files?
There was a problem hiding this comment.
just added a comment but yep exactly! we need this rsync-style functionality when there are Git gems because I noticed that there's some logic in Bunder / some gems that tries to normalize these files, which runs into "file is read-only" errors since it eventually tries to rewrite the (read-only bc of sandboxing) files from vendor/cache, so I need to make a copy of vendor/cache so that Bundler can rewrite stuff as it wants.
So to get around this I needed to make another copy which preserves symlinks except for those files that can potentially get rewritten by bundler (so far I've only noticed this happen for gemspecs and extconf.rb)
| 2.3.0 | ||
| """ | ||
|
|
||
| # Sample Gemfile.lock content with multiple Git gems from same repo |
There was a problem hiding this comment.
Not sure these comments add much, I might just drop them?
There was a problem hiding this comment.
(similar observation for a lot of the comments below, my guess is they're AI remnants)
|
|
||
| raise 'must provide both srcdir and destdir' unless ARGV.length >= 2 | ||
|
|
||
| $srcdir, $destdir = *ARGV |
There was a problem hiding this comment.
Is there a particular reason to use globals here vs something a bit more idiomatic, i.e. logic nested under if $PROGRAM_NAME == __FILE__
There was a problem hiding this comment.
just refactored it to make it a bit more idiomatic and remove the globals!
| _git_reset(rctx, git_repo) | ||
| _git_clean(rctx, git_repo) | ||
|
|
||
| git_metadata_folder = git_repo.directory.get_child(".git") |
There was a problem hiding this comment.
Why do we delete this? Worth a comment
There was a problem hiding this comment.
I'll add a comment! Was mostly since this shouldn't be used by Bundler down the line and therefore didn't want it end up as an unnecessary input to the bundle install action later
| if remote_name.endswith(".git"): | ||
| remote_name = remote_name[:-4] | ||
|
|
||
| extracted_path = "%s/%s-%s" % (cache_path, remote_name, git_package.revision[:12]) |
There was a problem hiding this comment.
I assume the 12 here is arbitrary?
There was a problem hiding this comment.
ah this is to match up with the exact naming format that Bundler creates the directory with in vendor/cache for gems installed from Git
There was a problem hiding this comment.
oh, fun! probably worth a comment then, given this is load-bearing
What issues were you seeing? I wonder if those are related to |
|
Ah, it wasn't related to |
It would be nice to have Windows - we generally tend to keep implementation consistent across all operating systems. I'm happy to help you debug this on Slack! |
|
sounds good! I'll keep investigating this tonight and reach out on slack if needed! |
withered-magic
force-pushed
the
withered-magic/git-gems
branch
from
016db8d to
f6762ba
Compare
withered-magic
force-pushed
the
withered-magic/git-gems
branch
from
9b1a2a0 to
6258b77
Compare
|
works on windows now! i also added another test to make sure creating executable targets for git gems also works |
3 participants
Fixes: #62
Adds support for installing gems from Git by including the Git sources in vendor/cache. Currently this uses
gitdirectly, but I'm planning to follow this PR up with another PR to use @sushain97's approach of downloading source archives from GitHub with the Bazel downloader.Right now doesn't support Windows as I wasn't able to get this working with my limited Windows knowledge :(
AI Disclaimer
Core logic was written by me, only used AI for comments, setting up the example scaffolding as well as some of the more tedious parsing tests.