Set BUNDLER_VERSION to the specific installed version in native helpers #5044
Conversation
| @@ -47,7 +47,7 @@ def self.run_bundler_subprocess(function:, args:, bundler_version:, options: {}) | |||
| args: args, | |||
| env: { | |||
| # Bundler will pick the matching installed major version | |||
| "BUNDLER_VERSION" => bundler_version, | |||
| "BUNDLER_VERSION" => installed_bundler_version(major_bundler_version), | |||
There was a problem hiding this comment.
| "BUNDLER_VERSION" => installed_bundler_version(major_bundler_version), | |
| "BUNDLER_VERSION" => installed_bundler_version(bundler_major_version) |
There was a problem hiding this comment.
Nice spot! This fix seems fine to me.
I think in a future PR we can improve on how we use use the env variable that we set here.
The "BUNDLER_VERSION" env variable is passed in when executing a run_helper_subprocess command, but it's not an officially supported/documented feature.
Maybe we should swap it for using rubygem's bundle version prefixes instead?
|
Hi folks, sorry about this issue. I believe it must've been the rubygems upgrade that actually caused this, since it's where the code to activate the Bundler gem lives. In particular, I reviewed the changelog and it must've been rubygems/rubygems#5181. Indeed, although the goal was to remove hard errors when activating Bundler, this "magic" was also removed from rubygems and now This change looks good to adapt to that, and I think #4973 should also fix this. |
|
@deivid-rodriguez Thanks for checking in - we've been testing this fix out but it seems like while it changes behaviour it doesn't remediate the issue downstream. Part of the issue is a lack of an integration test in core that checks the functionality thoroughly so we couldn't really detect this until it was added to an internal integration suite. We're going to rollback the rubygems upgrade as we've confirmed that does resolve the issue and then regroup on how we can roll forward. The long tail while be figuring out how we can move some testing around this earlier in the feedback loop so we don't have to revert contributions, which feels bad. |
|
Don't worry about reverting, I'm glad you have further checks down to the road to detect this kind of issue on time. |
Bump Rubygems to `3.3.22`: https://github.com/rubygems/rubygems/blob/master/CHANGELOG.md#3322--2022-09-07 There's some prior history here: 1. Originally bumped from `3.2.20` to `3.3.11` in #5035. That changed how bundler version detection worked, which caused some problems. 2. We attempted to workaround the issue in #5044, but that wasn't sufficient. 3. So in #5048 we rolled back to `3.2.20` 4. However, as part of the Ruby 3.1 upgrade, we _have_ to bump to Rubygems `>=3.3.3`... so we need to figure out a fix. 5. Now that `updater` is merged into `core`, we can have the full test suite checking the results, and don't have to bump in `core`, then pull into `updater` and see if it passes a separate CI... the two-step dance was the reason why this wasn't caught in the original core PR but instead merged and then reverted. 6. I am reasonably sure that @deivid-rodriguez figured out a way around the Bundler version difficulty in #5513, so this PR requires that to land first. 7. Originally I had the Rubygems upgrade bundled as part of the Ruby 3.1 upgrade, but now that we have a better understanding of what's happening, we should be fine to do these changes independently... which lets us more easily debug if we do happen to encounter problems. So I am splitting this out as a separate PR.
We've encountered an issue in the move forward to bundler 2.3.12 where our approach to setting the
BUNDLER_VERSIONin native helpers now behaves differently.Using
v0.182.0, you can observe the behaviour like so:On main the behaviour is like so:
We updated this behaviour in #4132 when we noted that passing in just the major version, i.e.
BUNDLER_VERSION=1no longer worked and we started using the detected version instead, which bundler would previously 'guess' at a near match.The change
This modifies our
native_helperfunction to set the BUNDLER_VERSION to the actual version of v1.x or v2.x we have installed according to the detected major version.In retrospect, this feels a little bit like it was working by accident and we were relying on bundler doing some magic for us, so I think this is a more correct approach.