Permalink
Browse files

- Escape strings in ruby-format specs using #dump instead of #to_s an…

…d %q. Issue #165
  • Loading branch information...
1 parent 11ba663 commit 81a6f40374fd7339cd124023d266ce9e2ae34c37 @drbrain drbrain committed Aug 26, 2011
@@ -1914,14 +1914,14 @@ def ri_dir
def ruby_code(obj)
case obj
- when String then '%q{' + obj + '}'
+ when String then obj.dump
when Array then '[' + obj.map { |x| ruby_code x }.join(", ") + ']'
when Hash then
- seg = obj.keys.sort.map { |k| "%q{#{k}} => %q{#{obj[k]}}" }
+ seg = obj.keys.sort.map { |k| "#{k.to_s.dump} => #{obj[k].to_s.dump}" }
"{ #{seg.join(', ')} }"
- when Gem::Version then obj.to_s.inspect
- when Date then '%q{' + obj.strftime('%Y-%m-%d') + '}'
- when Time then '%q{' + obj.strftime('%Y-%m-%d') + '}'
+ when Gem::Version then obj.to_s.dump
+ when Date then obj.strftime('%Y-%m-%d').dump
+ when Time then obj.strftime('%Y-%m-%d').dump
when Numeric then obj.inspect
when true, false, nil then obj.inspect
when Gem::Platform then "Gem::Platform.new(#{obj.to_a.inspect})"
@@ -204,7 +204,7 @@ def test_execute_ruby
end
assert_match %r|Gem::Specification.new|, @ui.output
- assert_match %r|s.name = %q\{foo\}|, @ui.output
+ assert_match %r|s.name = "foo"|, @ui.output
assert_equal '', @ui.error
end
@@ -188,6 +188,51 @@ def test_self_load_tainted
assert_equal @a2, spec
end
+ def test_self_load_escape_curly
+ @a2.name = 'a};raise "improper escaping";%q{'
+
+ full_path = @a2.spec_file
+ write_file full_path do |io|
+ io.write @a2.to_ruby_for_cache
+ end
+
+ spec = Gem::Specification.load full_path
+
+ @a2.files.clear
+
+ assert_equal @a2, spec
+ end
+
+ def test_self_load_escape_interpolation
+ @a2.name = 'a#{raise %<improper escaping>}'
+
+ full_path = @a2.spec_file
+ write_file full_path do |io|
+ io.write @a2.to_ruby_for_cache
+ end
+
+ spec = Gem::Specification.load full_path
+
+ @a2.files.clear
+
+ assert_equal @a2, spec
+ end
+
+ def test_self_load_escape_quote
+ @a2.name = 'a";raise "improper escaping";"'
+
+ full_path = @a2.spec_file
+ write_file full_path do |io|
+ io.write @a2.to_ruby_for_cache
+ end
+
+ spec = Gem::Specification.load full_path
+
+ @a2.files.clear
+
+ assert_equal @a2, spec
+ end
+
def test_self_load_legacy_ruby
spec = Gem::Deprecate.skip_during do
eval LEGACY_RUBY_SPEC
@@ -818,19 +863,19 @@ def test_to_ruby
# -*- encoding: utf-8 -*-
Gem::Specification.new do |s|
- s.name = %q{a}
- s.version = \"2\"
+ s.name = "a"
+ s.version = "2"
s.required_rubygems_version = Gem::Requirement.new(\"> 0\") if s.respond_to? :required_rubygems_version=
- s.authors = [%q{A User}]
- s.date = %q{#{Gem::Specification::TODAY.strftime "%Y-%m-%d"}}
- s.description = %q{This is a test description}
- s.email = %q{example@example.com}
- s.files = [%q{lib/code.rb}]
- s.homepage = %q{http://example.com}
- s.require_paths = [%q{lib}]
- s.rubygems_version = %q{#{Gem::VERSION}}
- s.summary = %q{this is a summary}
+ s.authors = ["A User"]
+ s.date = "#{Gem::Specification::TODAY.strftime "%Y-%m-%d"}"
+ s.description = "This is a test description"
+ s.email = "example@example.com"
+ s.files = ["lib/code.rb"]
+ s.homepage = "http://example.com"
+ s.require_paths = ["lib"]
+ s.rubygems_version = "#{Gem::VERSION}"
+ s.summary = "this is a summary"
if s.respond_to? :specification_version then
s.specification_version = #{Gem::Specification::CURRENT_SPECIFICATION_VERSION}
@@ -865,18 +910,18 @@ def test_to_ruby_for_cache
# -*- encoding: utf-8 -*-
Gem::Specification.new do |s|
- s.name = %q{a}
- s.version = \"2\"
+ s.name = "a"
+ s.version = "2"
s.required_rubygems_version = Gem::Requirement.new(\"> 0\") if s.respond_to? :required_rubygems_version=
- s.authors = [%q{A User}]
- s.date = %q{#{Gem::Specification::TODAY.strftime "%Y-%m-%d"}}
- s.description = %q{This is a test description}
- s.email = %q{example@example.com}
- s.homepage = %q{http://example.com}
- s.require_paths = [%q{lib}]
- s.rubygems_version = %q{#{Gem::VERSION}}
- s.summary = %q{this is a summary}
+ s.authors = ["A User"]
+ s.date = "#{Gem::Specification::TODAY.strftime "%Y-%m-%d"}"
+ s.description = "This is a test description"
+ s.email = "example@example.com"
+ s.homepage = "http://example.com"
+ s.require_paths = ["lib"]
+ s.rubygems_version = "#{Gem::VERSION}"
+ s.summary = "this is a summary"
if s.respond_to? :specification_version then
s.specification_version = #{Gem::Specification::CURRENT_SPECIFICATION_VERSION}
@@ -912,26 +957,26 @@ def test_to_ruby_fancy
# -*- encoding: utf-8 -*-
Gem::Specification.new do |s|
- s.name = %q{a}
- s.version = \"1\"
+ s.name = "a"
+ s.version = "1"
s.platform = Gem::Platform.new(#{expected_platform})
s.required_rubygems_version = Gem::Requirement.new(\">= 0\") if s.respond_to? :required_rubygems_version=
- s.authors = [%q{A User}]
- s.date = %q{#{Gem::Specification::TODAY.strftime "%Y-%m-%d"}}
- s.description = %q{This is a test description}
- s.email = %q{example@example.com}
- s.executables = [%q{exec}]
- s.extensions = [%q{ext/a/extconf.rb}]
- s.files = [%q{lib/code.rb}, %q{test/suite.rb}, %q{bin/exec}, %q{ext/a/extconf.rb}]
- s.homepage = %q{http://example.com}
- s.licenses = [%q{MIT}]
- s.require_paths = [%q{lib}]
- s.requirements = [%q{A working computer}]
- s.rubyforge_project = %q{example}
- s.rubygems_version = %q{#{Gem::VERSION}}
- s.summary = %q{this is a summary}
- s.test_files = [%q{test/suite.rb}]
+ s.authors = ["A User"]
+ s.date = "#{Gem::Specification::TODAY.strftime "%Y-%m-%d"}"
+ s.description = "This is a test description"
+ s.email = "example@example.com"
+ s.executables = ["exec"]
+ s.extensions = ["ext/a/extconf.rb"]
+ s.files = ["lib/code.rb", "test/suite.rb", "bin/exec", "ext/a/extconf.rb"]
+ s.homepage = "http://example.com"
+ s.licenses = ["MIT"]
+ s.require_paths = ["lib"]
+ s.requirements = ["A working computer"]
+ s.rubyforge_project = "example"
+ s.rubygems_version = "#{Gem::VERSION}"
+ s.summary = "this is a summary"
+ s.test_files = ["test/suite.rb"]
if s.respond_to? :specification_version then
s.specification_version = 4
@@ -970,6 +1015,17 @@ def test_to_ruby_legacy
assert_equal gemspec1, gemspec2
end
+ def test_to_ruby_nested_hash
+ metadata = {}
+ metadata[metadata] = metadata
+
+ @a2.metadata = metadata
+
+ ruby = @a2.to_ruby
+
+ assert_match /^ s\.metadata = { "/, ruby
+ end
+
def test_to_ruby_platform
@a2.platform = Gem::Platform.local
@a2.instance_variable_set :@original_platform, 'old_platform'
@@ -1536,20 +1592,20 @@ def test_metadata_specs
# -*- encoding: utf-8 -*-
Gem::Specification.new do |s|
- s.name = %q{m}
+ s.name = "m"
s.version = "1"
s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version=
- s.metadata = { %q{one} => %q{two}, %q{two} => %q{three} } if s.respond_to? :metadata=
- s.authors = [%q{A User}]
- s.date = %q{#{Gem::Specification::TODAY.strftime("%Y-%m-%d")}}
- s.description = %q{This is a test description}
- s.email = %q{example@example.com}
- s.files = [%q{lib/code.rb}]
- s.homepage = %q{http://example.com}
- s.require_paths = [%q{lib}]
- s.rubygems_version = %q{#{Gem::VERSION}}
- s.summary = %q{this is a summary}
+ s.metadata = { "one" => "two", "two" => "three" } if s.respond_to? :metadata=
+ s.authors = ["A User"]
+ s.date = "#{Gem::Specification::TODAY.strftime("%Y-%m-%d")}"
+ s.description = "This is a test description"
+ s.email = "example@example.com"
+ s.files = ["lib/code.rb"]
+ s.homepage = "http://example.com"
+ s.require_paths = ["lib"]
+ s.rubygems_version = "#{Gem::VERSION}"
+ s.summary = "this is a summary"
end
EOF

10 comments on commit 81a6f40

Contributor

postmodern replied Aug 26, 2011

Note that String#dump will escape unicode characters, String#inspect will leave them intact.

"まだ既知の小さな問題もいくつか残ってはいますが、これらは1.9.3-p0のリリースまでには修正されることでしょう。".dump
 => "\"\\u{307e}\\u{3060}\\u{65e2}\\u{77e5}\\u{306e}\\u{5c0f}\\u{3055}\\u{306a}\\u{554f}\\u{984c}\\u{3082}\\u{3044}\\u{304f}\\u{3064}\\u{304b}\\u{6b8b}\\u{3063}\\u{3066}\\u{306f}\\u{3044}\\u{307e}\\u{3059}\\u{304c}\\u{3001}\\u{3053}\\u{308c}\\u{3089}\\u{306f}1.9.3-p0\\u{306e}\\u{30ea}\\u{30ea}\\u{30fc}\\u{30b9}\\u{307e}\\u{3067}\\u{306b}\\u{306f}\\u{4fee}\\u{6b63}\\u{3055}\\u{308c}\\u{308b}\\u{3053}\\u{3068}\\u{3067}\\u{3057}\\u{3087}\\u{3046}\\u{3002}\"" 
Owner

drbrain replied Aug 26, 2011

There doesn't seem to be anything wrong with using dump:

$ ruby19 -e 'p "π".dump, eval("π".dump)'
"\"\\u{3c0}\""
"π"
Contributor

postmodern replied Aug 26, 2011

Correct, it evals just fine. More of a readability issue.

Owner

drbrain replied Aug 26, 2011

Files in the specifications directory generated by #to_ruby are not meant to be human-readable.

Contributor

postmodern replied Aug 27, 2011

Not human-readable, you mean like YAML? jk jk :)

I also wondered why does RubyGems load all of the gemspecs at start-up. If you could delay the loading, it might be possible to keep the gemspecs in their YAML format, and not incur the performance hit of YAML.load_file * $num_installed_gems.

Contributor

raggi replied Aug 27, 2011

it's gem plugins that cause the spec loads

Contributor

raggi replied Aug 27, 2011

well, and the dependency maps are in there

Owner

drbrain replied Aug 27, 2011

Additionally, loading YAML is very expensive compared to loading the ruby parser (or Marshal). One of the startup constraints placed on RubyGems by Ruby core committers is that it cannot load too many files or use too much memory. Compared to built-in C functionality, YAML loads many, many files and can use lots of memory parsing YAML.

Contributor

postmodern replied Aug 27, 2011

Also I'm curious why we need to load all information in the gemspec. It seems as if name, version, dependencies are all RubyGems needs to resolve dependencies. Maybe a flat-file index could be re-generated when a Gem is installed, and searched later when a Gem is required? Just brainstorming ways RubyGems could do less work.

Member

luislavena replied Aug 27, 2011

@postmodern: Roger Pack explored the usage of a cache: https://github.com/rdp/faster_rubygems

Indeed the usage of both marshalled data and basic information could speed things up.

Please sign in to comment.