Let bundler >= 2.2 handle windows platform #1469
Conversation
Fixes #1157 Because it has support for multiple platforms in Gemfile.lock. So deleting Gemfile.lock when one of the platforms is windows is not good. Bundler's error message tells the user how to handle it. ``` Your bundle only supports platforms ["mingw"] but your local platform is x86_64-linux. Add the current platform to the lockfile with `bundle lock --add-platform x86_64-linux` and try again. ```
schneems
force-pushed
the
schneems/windows-jun19-2024
branch
6 times, most recently
from
cba958c
to
8e797d3
Compare
schneems
force-pushed
the
schneems/windows-jun19-2024
branch
from
8e797d3
to
ed6a33b
Compare
schneems
changed the title
Co-authored-by: Ed Morley <[email protected]>
|
Merged now. I'm going to aim to deploy on monday. |
| https://devcenter.heroku.com/articles/bundler-windows-gemfile | ||
| WARNING | ||
| if bundler.supports_multiple_platforms? | ||
| puts "Windows `Gemfile.lock` detected, bundler #{@bundler.version} supports multiple platforms, no action taken." |
There was a problem hiding this comment.
"No action taken": This text can be confusing, to people who don't know the history.
I think it is better to simply not print anything.
There was a problem hiding this comment.
I have an output goal that is "anytime there's a change in behavior based on buildpack logic, developers should be able to tell by comparing the outputs."
Humans are also bad at seeing "what's not there." I.e. If one build shows "Installing mail" but it's not in the other build, most people usually have to use diff tools to determine notice unless they're explicitly looking for that line.
Another goal is that if they have a failure related to that different action they should be able to trace that action in the same build. If someone's app started to fail they can scan up and might see that line and be curious only to discover that behavior is the cause of their problems and there's a bug in bundler.
So it's a little aggressive to log in the empty if case but I feel like it's a possibly VERY large change to the windows users and they deserve to know about it.
Now about "no action taken." If that phrase is confusing then we can remove it later or ask users what might be better. What I'm trying to avoid with that line is when you show people something, they want to know what happens next because of that thing.
You can see this with a storytelling narrative principle called "checkov's gun" which loosely is "if a gun is introduced in the first act it should go off in the third." Here's a funny video that is satire but demonstrates the principle https://www.youtube.com/watch?v=PQOvv_b9TpY. The viewer keeps waiting for it to "go off."
The problem with saying simply:
"Windows `Gemfile.lock` detected, bundler #{@bundler.version} supports multiple platforms"
Is that the reader will implicitly be waiting for the seemingly missing information. They will be thinking "why did you tell me that" as well as "well, what will you do with that information?"
The current output might be a little curious or weird. Maybe even eyebrow-raising. But it won't be anxiety or stress-inducing.
So I agree it's off, but it's also purposeful in it's off-ness.
There was a problem hiding this comment.
It is a good point. I think something like this will ensure that people who was not aware of the previous behavior and notice the message are not confused and use time on investigating.
| puts "Windows `Gemfile.lock` detected, bundler #{@bundler.version} supports multiple platforms, no action taken." | |
| puts "Windows `Gemfile.lock` detected, bundler #{@bundler.version} supports multiple platforms, no action taken (before June 25th, Heroku would have removed the lock file)." |
There was a problem hiding this comment.
I think another way to think of it is that we are DOING something by not doing something. I.e. in one branch we're deleting a Gemfile.lock and in the other we're preserving it.
With that lense we can rewrite as:
Windows platform detected, preserving `Gemfile.lock` (bundler #{version} >= 2.2)
Would look like:
Windows platform detected, preserving `Gemfile.lock` (bundler 2.5.9 >= 2.2)
I like that better.
* Add platform information to windows support docs Update windows docs to specify how to add Heroku's platform to the Gemfile.lock. In #1469 the behavior changed for Windows users. However if they're depending on a platform specific gem and they don't have the `x86_64-linux` platform locked then their dependency resolution will not use that information and their deploy will fail with a message saying to run those commands. By adding the instructions into the descanter article, warning message, and changelog entry there's a better chance windows users will succeed on their first deploy after #1469 ## Commands breakdown Install the latest bundler version at the system level: ``` > gem install bundler ``` Update the current project to explicitly use the latest version of bundler: ``` > bundle update --bundler ``` Ensure that both `ruby` and `x86_64-linux` platforms are specified in the `Gemfile.lock`: ``` > bundle lock --add-platform ruby > bundle lock --add-platform x86_64-linux ``` Ensure bundler has resolved dependencies with all platforms in mind: ``` > bundle install ``` Commit the results to git: ``` > git add Gemfile.lock > git commit -m "Upgrade bundler" ``` * Fix variable access for windows users Reported in #1472, the code was using an instance variable that does not exist so it raises an error. The `bundler` call is both a class and instance method https://github.com/heroku/heroku-buildpack-ruby/blob/4d2621c7dedff3edc95ee809d1cd71d6186fb796/lib/language_pack/ruby.rb#L27-L33. Internally it's stored as a class instance variable https://www.ruby-lang.org/en/documentation/faq/8/ so the data can be shared between the class and instances. The fix is to use `bundler` instead of `@bundler`. The test exercises and asserts that codepath was executed. Close #1472 * Update warning message to be less confusing Based on this conversation #1469 (comment)
Building on #1458
Tests and documents Windows bundler 2.2+ behavior.