build_extensions conditions check fix

[dunric]

There is a logical flaw if gem's extension has or may be (re)built.
It can start compilation even without write permissions to the build/target directory:

return if !File.writable?(base_dir) &&
          !File.exist?(File.join(base_dir, 'extensions'))
# Stop build attempt when `base_dir` is not writable **and** `base_dir/extensions` not yet exists

In other words condition will pass if base_dir/extensions is missing even when directory is not writable. It may result in following exception for some gems with C/C++ extensions like nokogiri:

/usr/lib64/ruby/2.1.0/fileutils.rb:250:in `mkdir': Permission denied @ dir_s_mkdir - /usr/lib64/ruby/gems/2.1.0/extensions/x86_64-linux (Errno::EACCES)
        from /usr/lib64/ruby/2.1.0/fileutils.rb:250:in `fu_mkdir'
        from /usr/lib64/ruby/2.1.0/fileutils.rb:224:in `block (2 levels) in mkdir_p'
        from /usr/lib64/ruby/2.1.0/fileutils.rb:222:in `reverse_each'
        from /usr/lib64/ruby/2.1.0/fileutils.rb:222:in `block in mkdir_p'
        from /usr/lib64/ruby/2.1.0/fileutils.rb:208:in `each'
        from /usr/lib64/ruby/2.1.0/fileutils.rb:208:in `mkdir_p'
        from /usr/lib64/ruby/2.1.0/rubygems/ext/builder.rb:210:in `write_gem_make_out'
        from /usr/lib64/ruby/2.1.0/rubygems/ext/builder.rb:132:in `build_error'
        from /usr/lib64/ruby/2.1.0/rubygems/ext/builder.rb:171:in `rescue in build_extension'
        from /usr/lib64/ruby/2.1.0/rubygems/ext/builder.rb:156:in `build_extension'
        from /usr/lib64/ruby/2.1.0/rubygems/ext/builder.rb:198:in `block in build_extensions'
        from /usr/lib64/ruby/2.1.0/rubygems/ext/builder.rb:195:in `each'
        from /usr/lib64/ruby/2.1.0/rubygems/ext/builder.rb:195:in `build_extensions'
        from /usr/lib64/ruby/2.1.0/rubygems/specification.rb:1436:in `block in build_extensions'
        from /usr/lib64/ruby/2.1.0/rubygems/user_interaction.rb:45:in `use_ui'
        from /usr/lib64/ruby/2.1.0/rubygems/specification.rb:1434:in `build_extensions'
        from /usr/lib64/ruby/2.1.0/rubygems/stub_specification.rb:60:in `build_extensions'
        from /usr/lib64/ruby/2.1.0/rubygems/basic_specification.rb:56:in `contains_requirable_file?'
        from /usr/lib64/ruby/2.1.0/rubygems/specification.rb:925:in `block in find_inactive_by_path'
        --- snip ---

Proposed simple fix just narrows this flaw so compilation won't start if permissions lacking or if directory permissions are ok but extensions are not yet built.

[drbrain]

This seems OK, can you update the test?

[zzak]

Was looking into this to write a test and found the #build_extensions return logic confusing, so I wrote a patch but haven't fixed the test yet:

diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb
index fedd52f..c4d7f6c 100644
--- a/lib/rubygems/specification.rb
+++ b/lib/rubygems/specification.rb
@@ -1440,8 +1440,10 @@ class Gem::Specification < Gem::BasicSpecification
     return if extensions.empty?
     return if installed_by_version < Gem::Version.new('2.2.0.preview.2')
     return if File.exist? gem_build_complete_path
-    return if !File.writable?(base_dir) ||
-              !File.exist?(File.join(base_dir, 'extensions'))
+    return if !File.exist?(base_dir)
+    return if !File.writable?(base_dir)
+    return if !File.exist?(File.join(base_dir, 'extensions'))
+    return if !File.writable?(File.join(base_dir, 'extensions'))

     begin
       # We need to require things in $LOAD_PATH without looking for the

Each return can be independent, but I'm wondering if we only need the last 2 checks for #{base_dir}/extensions.

[zzak]

Merged manually via 4411bca

Thanks!