Merge pull request rubygems#8520 from Edouard-chin/ec-writable

Modify `bundle doctor` to not report issue when files aren't writable:

bundler/lib/bundler/cli/doctor.rb

@@ -99,7 +99,7 @@ def run
           end
         end.sort.each {|m| message += m }
         raise ProductionError, message
-      elsif !permissions_valid
+      elsif permissions_valid
         Bundler.ui.info "No issues found with the installed bundle"
       end
     end
@@ -108,21 +108,21 @@ def run
 
     def check_home_permissions
       require "find"
-      files_not_readable_or_writable = []
-      files_not_rw_and_owned_by_different_user = []
-      files_not_owned_by_current_user_but_still_rw = []
+      files_not_readable = []
+      files_not_readable_and_owned_by_different_user = []
+      files_not_owned_by_current_user_but_still_readable = []
       broken_symlinks = []
       Find.find(Bundler.bundle_path.to_s).each do |f|
         if !File.exist?(f)
           broken_symlinks << f
-        elsif !File.writable?(f) || !File.readable?(f)
+        elsif !File.readable?(f)
           if File.stat(f).uid != Process.uid
-            files_not_rw_and_owned_by_different_user << f
+            files_not_readable_and_owned_by_different_user << f
           else
-            files_not_readable_or_writable << f
+            files_not_readable << f
           end
         elsif File.stat(f).uid != Process.uid
-          files_not_owned_by_current_user_but_still_rw << f
+          files_not_owned_by_current_user_but_still_readable << f
         end
       end
 
@@ -134,23 +134,23 @@ def check_home_permissions
         ok = false
       end
 
-      if files_not_owned_by_current_user_but_still_rw.any?
+      if files_not_owned_by_current_user_but_still_readable.any?
         Bundler.ui.warn "Files exist in the Bundler home that are owned by another " \
-          "user, but are still readable/writable. These files are:\n - #{files_not_owned_by_current_user_but_still_rw.join("\n - ")}"
+          "user, but are still readable. These files are:\n - #{files_not_owned_by_current_user_but_still_readable.join("\n - ")}"
 
         ok = false
       end
 
-      if files_not_rw_and_owned_by_different_user.any?
+      if files_not_readable_and_owned_by_different_user.any?
         Bundler.ui.warn "Files exist in the Bundler home that are owned by another " \
-          "user, and are not readable/writable. These files are:\n - #{files_not_rw_and_owned_by_different_user.join("\n - ")}"
+          "user, and are not readable. These files are:\n - #{files_not_readable_and_owned_by_different_user.join("\n - ")}"
 
         ok = false
       end
 
-      if files_not_readable_or_writable.any?
+      if files_not_readable.any?
         Bundler.ui.warn "Files exist in the Bundler home that are not " \
-          "readable/writable by the current user. These files are:\n - #{files_not_readable_or_writable.join("\n - ")}"
+          "readable by the current user. These files are:\n - #{files_not_readable.join("\n - ")}"
 
         ok = false
       end

bundler/spec/commands/doctor_spec.rb

@@ -14,7 +14,7 @@
 
     @stdout = StringIO.new
 
-    [:error, :warn].each do |method|
+    [:error, :warn, :info].each do |method|
       allow(Bundler.ui).to receive(method).and_wrap_original do |m, message|
         m.call message
         @stdout.puts message
@@ -45,20 +45,20 @@
       allow(File).to receive(:writable?).with(File.expand_path("..", Gem.default_dir))
     end
 
-    it "exits with no message if the installed gem has no C extensions" do
+    it "exits with a success message if the installed gem has no C extensions" do
       doctor = Bundler::CLI::Doctor.new({})
       allow(doctor).to receive(:lookup_with_fiddle).and_return(false)
       expect { doctor.run }.not_to raise_error
-      expect(@stdout.string).to be_empty
+      expect(@stdout.string).to include("No issues")
     end
 
-    it "exits with no message if the installed gem's C extension dylib breakage is fine" do
+    it "exits with a success message if the installed gem's C extension dylib breakage is fine" do
       doctor = Bundler::CLI::Doctor.new({})
       expect(doctor).to receive(:bundles_for_gem).exactly(2).times.and_return ["/path/to/myrack/myrack.bundle"]
       expect(doctor).to receive(:dylibs).exactly(2).times.and_return ["/usr/lib/libSystem.dylib"]
       allow(doctor).to receive(:lookup_with_fiddle).with("/usr/lib/libSystem.dylib").and_return(false)
       expect { doctor.run }.not_to raise_error
-      expect(@stdout.string).to be_empty
+      expect(@stdout.string).to include("No issues")
     end
 
     it "exits with a message if one of the linked libraries is missing" do
@@ -105,19 +105,32 @@
       allow(File).to receive(:stat).with(@unwritable_file) { @stat }
     end
 
-    it "exits with an error if home contains files that are not readable/writable" do
+    it "exits with an error if home contains files that are not readable" do
       doctor = Bundler::CLI::Doctor.new({})
       allow(doctor).to receive(:lookup_with_fiddle).and_return(false)
       allow(@stat).to receive(:uid) { Process.uid }
       allow(File).to receive(:writable?).with(@unwritable_file) { false }
       allow(File).to receive(:readable?).with(@unwritable_file) { false }
       expect { doctor.run }.not_to raise_error
       expect(@stdout.string).to include(
-        "Files exist in the Bundler home that are not readable/writable by the current user. These files are:\n - #{@unwritable_file}"
+        "Files exist in the Bundler home that are not readable by the current user. These files are:\n - #{@unwritable_file}"
       )
       expect(@stdout.string).not_to include("No issues")
     end
 
+    it "exits without an error if home contains files that are not writable" do
+      doctor = Bundler::CLI::Doctor.new({})
+      allow(doctor).to receive(:lookup_with_fiddle).and_return(false)
+      allow(@stat).to receive(:uid) { Process.uid }
+      allow(File).to receive(:writable?).with(@unwritable_file) { false }
+      allow(File).to receive(:readable?).with(@unwritable_file) { true }
+      expect { doctor.run }.not_to raise_error
+      expect(@stdout.string).not_to include(
+        "Files exist in the Bundler home that are not readable by the current user. These files are:\n - #{@unwritable_file}"
+      )
+      expect(@stdout.string).to include("No issues")
+    end
+
     context "when home contains files that are not owned by the current process", :permissions do
       before(:each) do
         allow(@stat).to receive(:uid) { 0o0000 }
@@ -130,7 +143,7 @@
         allow(File).to receive(:readable?).with(@unwritable_file) { false }
         expect { doctor.run }.not_to raise_error
         expect(@stdout.string).to include(
-          "Files exist in the Bundler home that are owned by another user, and are not readable/writable. These files are:\n - #{@unwritable_file}"
+          "Files exist in the Bundler home that are owned by another user, and are not readable. These files are:\n - #{@unwritable_file}"
         )
         expect(@stdout.string).not_to include("No issues")
       end
@@ -142,7 +155,7 @@
         allow(File).to receive(:readable?).with(@unwritable_file) { true }
         expect { doctor.run }.not_to raise_error
         expect(@stdout.string).to include(
-          "Files exist in the Bundler home that are owned by another user, but are still readable/writable. These files are:\n - #{@unwritable_file}"
+          "Files exist in the Bundler home that are owned by another user, but are still readable. These files are:\n - #{@unwritable_file}"
         )
         expect(@stdout.string).not_to include("No issues")
       end