diff options
author | The Bundler Bot <bot@bundler.io> | 2018-02-01 22:26:36 +0000 |
---|---|---|
committer | Colby Swandale <me@colby.fyi> | 2018-04-20 10:27:32 +1000 |
commit | d9b3854b2d4bbd10575e420c1655833bee7e9037 (patch) | |
tree | afb67c7f550dc01aab2e493abf874d98583a5eaa | |
parent | 95689a54f107697fe208eedb0ef3131a1db16707 (diff) | |
download | bundler-d9b3854b2d4bbd10575e420c1655833bee7e9037.tar.gz |
Auto merge of #6279 - deivid-rodriguez:fix/encoding_bug, r=indirect
Fix hang when gemspec has incompatible encoding
### What was the end-user problem that led to this PR?
The problem was that if a gemspec had a non ascii character on it, and the external encoding is not utf-8, bundler would fail with a very cryptic error. CircleCI is very likely to reproduce this bug since its base images default to `LANG=C`. See [here](https://circleci.com/gh/deivid-rodriguez/pry-byebug/12) for an example.
In bundler's master, `bundle install` seems to hang in this same situation. I have no idea why but CPU usage goes crazy (100%) when running something like `LANG=C bundle install` with such a gemspec.
Note that adding a utf-8 magic encoding comment seems to fix the problem, but I think `bundler` should just work in this situation, or at least give the user a more helpful error message. And of course never hang either.
### What was your diagnosis of the problem?
My diagnosis was that bundler was reading an incompatible character from the gemspec and was not able to deal with it, nor give a useful error message.
### What is your fix for the problem, implemented in this PR?
My fix initial fix was to at least give a better error message, preventing the method building the error message itself from blowing up due to incompatible encodings. This is the initial patch that I wrote to improve the error message:
```diff
diff --git a/lib/bundler/dsl.rb b/lib/bundler/dsl.rb
index 37bfe3674..79ef563d1 100644
--- a/lib/bundler/dsl.rb
+++ b/lib/bundler/dsl.rb
@@ -581,7 +581,14 @@ The :#{name} git source is deprecated, and will be removed in Bundler 2.0.
def parse_line_number_from_description
description = self.description
- if dsl_path && description =~ /((#{Regexp.quote File.expand_path(dsl_path)}|#{Regexp.quote dsl_path.to_s}):\d+)/
+ return [nil, description] unless dsl_path
+
+ quoted_expanded_dsl_path = Regexp.quote(File.expand_path(dsl_path))
+ quoted_dsl_path = Regexp.quote(dsl_path.to_s)
+
+ return [nil, description] unless Encoding.compatible?(quoted_dsl_path, description) && Encoding.compatible?(quoted_expanded_dsl_path, description)
+
+ if description =~ /((#{quoted_expanded_dsl_path}|#{quoted_dsl_path}):\d+)/
trace_line = Regexp.last_match[1]
description = description.sub(/#{Regexp.quote trace_line}:\s*/, "").sub("\n", " - ")
end
```
With that patch, the error would be more clear, pointing at the exact line in the gemspec where the incompatible character was found.
However, after that I considered that maybe bundler doesn't need to read the gemspec as a text file, but could read it "binarily". That seemed to fix the problem indeed and made the `bundle install` succeed in this situation.
### Why did you choose this fix out of the possible options?
I chose this fix because not only fixes the error message but also seems to make `bundler` just work.
(cherry picked from commit 83c23ecac258dfcec4ab3d552f0d09725bd09145)
-rw-r--r-- | lib/bundler.rb | 2 | ||||
-rw-r--r-- | spec/install/gemspecs_spec.rb | 17 |
2 files changed, 18 insertions, 1 deletions
diff --git a/lib/bundler.rb b/lib/bundler.rb index 010b26f3b5..b7002db99e 100644 --- a/lib/bundler.rb +++ b/lib/bundler.rb @@ -427,7 +427,7 @@ EOF def load_gemspec_uncached(file, validate = false) path = Pathname.new(file) - contents = path.read + contents = read_file(file) spec = if contents.start_with?("---") # YAML header eval_yaml_gemspec(path, contents) else diff --git a/spec/install/gemspecs_spec.rb b/spec/install/gemspecs_spec.rb index 0c1ed99097..5dca6f68c0 100644 --- a/spec/install/gemspecs_spec.rb +++ b/spec/install/gemspecs_spec.rb @@ -46,6 +46,23 @@ RSpec.describe "bundle install" do expect(the_bundle).to include_gems "activesupport 2.3.2" end + it "does not hang when gemspec has incompatible encoding" do + create_file("foo.gemspec", <<-G) + Gem::Specification.new do |gem| + gem.name = "pry-byebug" + gem.version = "3.4.2" + gem.author = "David RodrÃguez" + gem.summary = "Good stuff" + end + G + + install_gemfile <<-G, :env => { "LANG" => "C" } + gemspec + G + + expect(out).to include("Bundle complete!") + end + context "when ruby version is specified in gemspec and gemfile" do it "installs when patch level is not specified and the version matches" do build_lib("foo", :path => bundled_app) do |s| |