-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(bundler): extract, update, artifacts #3058
Conversation
Re: locating/writing gemspec files before lock file generation, does anyone know how exhaustively we need to check for these? Do we just need to look for |
I don't recall ever running into a gem whose gemspec wasn't |
Is the project name extracted from any project metadata or based purely on the parent directory name? |
That I don't know. I wouldn't be surprised if it's the other way around and it discovers gems via |
I’ll copy over every gemspec file it finds just to be “safe”. |
@rarkins, thanks for doing this! For what it's worth, After a bit more digging, I found that Bundler's handling of I hope this is helpful. I'd be glad to do some more research if there are any other questions that need answering. |
@rye thanks very much for the extra research! Right now the string match we use on the file list should catch every single For now I think we're fine on the gemspec topic. The big remaining one is to be able to dynamically switch Bundler versions to match what's written in the |
@rarkins, yes, that inclusion of other files in a project is actually very commonplace, since gemspecs are actually Ruby source files and are interpreted as such. One approach to gathering information from a gemspec is to do as Bundler does and actually load the gemspec using RubyGems' own code, then extract dependency information and the like from that. I would be glad to work on this approach when I have some more availability—it should be moderately straightforward though, it's just calling As far as the Bundler version switching goes, that sounds like a good plan. I'll note that Bundler 2.0 (recently launched) includes the ability to interpret prior versions of Gemfile.locks, but many projects have not switched yet and it's probably better to use a project's own Bundler version for parity. That will be an intriguing challenge, especially since Bundler 2.0+ have more strict Ruby version requirements. |
My own experience attempting to use Bundler 2.0 for both 2.x and 1.x failed. I see they’ve had teething problems and even backed off requiring Rubygems 3.0.0 now, so I figured it’s easiest to just dynamically switch versions using docker tags instead (and parsing Gemfile.lock). With this current approach I hope we don’t have the need for any advanced ruby requiring as we’ll just use the full file system if necessary plus exact Bundler version. |
@rarkins yep, there have been issues with the 2.0 upgrade all around. I will note, still, that parsing the Gemfile and Gemfile.lock using Bundler's own code may be a better approach, since it's available to you and would ensure consistent results—Gemfiles themselves are also Ruby source code… ( There are idiosyncrasies like that you can use As a proof of concept, I was able to rather easily use require 'bundler'
lockfile_contents = open('Gemfile.lock', 'rb', &:read)
Bundler::LockfileParser.new(lockfile_contents).dependencies
# => {"fastlane"=>Gem::Dependency.new("fastlane", Gem::Requirement.new([">= 0"]), :runtime),
# "fastlane-plugin-bugsnag"=>Gem::Dependency.new("fastlane-plugin-bugsnag", Gem::Requirement.new([">= 0"]), :runtime),
# "json"=>Gem::Dependency.new("json", Gem::Requirement.new([">= 0"]), :runtime),
# "rubocop"=>Gem::Dependency.new("rubocop", Gem::Requirement.new(["~> 0.60"]), :runtime),
# "xcodeproj"=>Gem::Dependency.new("xcodeproj", Gem::Requirement.new([">= 0"]), :runtime)}
Bundler::LockfileParser.new(lockfile_contents).dependencies.keys
# => ["fastlane", "fastlane-plugin-bugsnag", "json", "rubocop", "xcodeproj"]
Bundler::LockfileParser.new(lockfile_contents).sources.map(&:remotes).flatten.map(&:to_s)
# => ["https://rubygems.org/"] (This API works for Bundler 2.0 and 1.17, I didn't test widely but I'd imagine it hasn't changed much.) So, I guess my argument is that your extraction code could be made much shorter (and more general) if you did opt to use the Bundler code itself. Calling this from JS could be as simple as Thoughts? |
@rye I'm still making up my mind regarding whether to do Gemfile parsing and/or updating using Ruby or keep it using JS. I had expected to need Ruby, but for now I think it might be at 95% of use cases already. e.g. even if someone requires something from their Gemfile, it doesn't necessarily mean we need to care about it - or support it. However I do ideally want Renovate to be "flexible so you don't need to be", so I'm opening to switching to Ruby-based parsing if there are clear benefits. One other thing to note is that we need to be able to not only parse a file, but also to be able to update it too. i.e. it's not enough to get a parsed object if is then really hard to perform the update. You can see with the current JS-based approach that we can do it with line numbers, which makes for the easiest possible update logic. BTW I think the lockfile may be the one case where we don't need to do it in Ruby because it's simple and not executable code. One downside of making it Ruby-based is that we may need to not only call a child process but actually call a Docker child process - which significantly increases the latency. The reason for this is that if we are executing arbitrary Ruby code then we need to as sandboxed as possible within Docker, rather than directly calling Ruby on the same machine/VM. That is, unless we think Bundler's parsing is sandboxed enough that it couldn't allow malicious code. But although this increases latency a lot, it remains to be seen whether it matters. e.g. if a typical Ruby project takes 10s to run and we add 0.5s to that, it's really not a big deal. I think the way forward will be like this:
Thanks again for your feedback |
My latest attempt at a Bundler 2 image seems to switch back to 1.x as desired, so I'll use that for now |
Correct. Unfortunately, executable |
As a general rule, I don't mind shortcomings/known limitation so long as they're gracefully wrong and also that they don't render the stuff that works useless. So I think in this case if you're pulling in versions based on Ruby logic, it simply won't update those and will only update gems that have version fields defined. So a quick status update:
|
@rarkins, I definitely see where you're at. Thanks again—at this point, it seems like this PR is only meant to add bundler support in alpha stage, which it does quite well. I was under the impression that more projects than my own use Personally, I'd be more than happy to opt-in on one of my projects to see how well it works. Just out of curiosity, would you be interested in getting Ruby-based parsing (sandboxed appropriately) instead as a contribution later on down the road? I'm now reading through the codebase to figure out how this might be done, but I'd be happy to chip that in later. |
@rye I think this approach may even be good enough for "GA", although obviously wait for feedback first. I'd definitely be happy to have a Ruby-based extract option added later to capture all use cases. Once the JS one is working and we can identify any shortcomings, being able to test/verify a Ruby-based drop-in replacement should be fairly simple. It would pretty much just be a matter of replacing the existing lib/manager/bundler/extract and update functions. BTW we already do a Python-based parsing of Anyway I think I'll merge this in the next day or two and will blog instructions on how to test/what to look for and will appreciate your feedback. Are you using the app or CLI currently? |
@rarkins All this sounds good. I definitely think having a full test base and feedback from more users will make any further changes this faster and higher-confidence, so releasing this as an opt-in kind of thing is the first step so some entirely bundler-based projects can start testing it out. (I have a few projects that I'll be inclined to use Renovate on with even partial Bundler support.) Any yes, I'm using the app, didn't even know that Renovate could be run from the CLI until now, but that actually makes a lot of sense. I'm assuming I'll need to explicitly enable something in the config, but I'm definitely going to do that on StoDevX/AAO-React-Native quite quickly after this is made available to that project. (I could definitely run locally in the meantime if that would be more helpful.) I'll be glad to provide any further feedback and help as a likely end-user of Bundler support in the long run. |
🎉 This PR is included in version 13.175.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Where would you like feedback about this feature? New issues? I don't have anything specific yet other than that every update is in an "error" state (according to master issue) but the job logs don't have anything that sticks out to me (I searched for dep name and for "error" and came up empty handed). |
@bjeanes I see that Renovate has detected |
By the way, I don't see any errors in the logs. It looks like it doesn't see any updates. Are there any that you are expecting? And can you paste a screenshot of the master issue here, or send to support@renovatebot.com if confidential? |
In my case, yes, but this isn't anything common or universal about Ruby-land. I imagine there is some way to disable that that within my config. I don't see errors either. Where I am seeing "error" is in the master issue. Renovate has since edited the issue but from the history view of the comment content: |
First I saw this:
And then the most recent log shows your repo exiting with:
It seems rubygems is rate limiting us, even with a very low number of requests so far. I need to look into it more. |
I forced it in master issue and then it went into error state, so likely you are looking at a log of a newer job which is back to not scheduling it. |
@rarkins from the documentation on the RubyGems API:
From what you're seeing, does it look like Renovate is hitting the 15 requests per second limit? If so, it'll be worth digging into why. Alternatively, I would wonder if it's hitting the 100 request/10 minute limit for some reason? |
@bjeanes I think we're probably hitting the 15 requests per second limit, I will need to work out the best location to limit it |
This PR adds extract, update and artifacts functions to put Bundler support into an "alpha" support stage. Some known gotchas:
gem "uglifier", ">= 1.3.0", "< 1.4.0", require: false
Gemfile.lock
requires any referenced gemspec files to be present, so this only works with gitFs mode right now