Merge pull request #2512 from commitay/cask-binary

Replace `FileUtils.chmod` with `/bin/chmod` for cask binaries.

4 files changed, 38 insertions, 11 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/artifact/binary.rb b/Library/Homebrew/cask/lib/hbc/artifact/binary.rb
index 06bdfe157..21d123ab9 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/binary.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/binary.rb
@@ -9,7 +9,12 @@ module Hbc
 
       def link
         super
-        FileUtils.chmod "+x", source
+        return if source.executable?
+        if source.writable?
+          FileUtils.chmod "+x", source
+        else
+          @command.run!("/bin/chmod", args: ["+x", source], sudo: true)
+        end
       end
     end
   end
diff --git a/Library/Homebrew/test/cask/artifact/binary_spec.rb b/Library/Homebrew/test/cask/artifact/binary_spec.rb
index 69bde3438..ee62e6439 100644
--- a/Library/Homebrew/test/cask/artifact/binary_spec.rb
+++ b/Library/Homebrew/test/cask/artifact/binary_spec.rb
@@ -6,9 +6,7 @@ describe Hbc::Artifact::Binary, :cask do
       end
     end
   }
-  let(:expected_path) {
-    Hbc.binarydir.join("binary")
-  }
+  let(:expected_path) { Hbc.binarydir.join("binary") }
 
   before(:each) do
     Hbc.binarydir.mkpath
@@ -26,15 +24,28 @@ describe Hbc::Artifact::Binary, :cask do
     expect(expected_path.readlink).to exist
   end
 
-  it "makes the binary executable" do
-    expect(FileUtils).to receive(:chmod).with("+x", cask.staged_path.join("binary"))
+  context "when the binary is not executable" do
+    let(:cask) {
+      Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-non-executable-binary.rb").tap do |cask|
+        shutup do
+          InstallHelper.install_without_artifacts(cask)
+        end
+      end
+    }
 
-    shutup do
-      Hbc::Artifact::Binary.new(cask).install_phase
-    end
+    let(:expected_path) { Hbc.binarydir.join("naked_non_executable") }
 
-    expect(expected_path).to be_a_symlink
-    expect(expected_path.readlink).to be_executable
+    it "makes the binary executable" do
+      expect(FileUtils).to receive(:chmod)
+        .with("+x", cask.staged_path.join("naked_non_executable")).and_call_original
+
+      shutup do
+        Hbc::Artifact::Binary.new(cask).install_phase
+      end
+
+      expect(expected_path).to be_a_symlink
+      expect(expected_path.readlink).to be_executable
+    end
   end
 
   it "avoids clobbering an existing binary by linking over it" do
diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-non-executable-binary.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-non-executable-binary.rb
new file mode 100644
index 000000000..4bd2f0882
--- /dev/null
+++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-non-executable-binary.rb
@@ -0,0 +1,9 @@
+cask 'with-non-executable-binary' do
+  version '1.2.3'
+  sha256 'd5b2dfbef7ea28c25f7a77cd7fa14d013d82b626db1d82e00e25822464ba19e2'
+
+  url "file://#{TEST_FIXTURE_DIR}/cask/naked_non_executable"
+  homepage 'http://example.com/with-binary'
+
+  binary "naked_non_executable"
+end
diff --git a/Library/Homebrew/test/support/fixtures/cask/naked_non_executable b/Library/Homebrew/test/support/fixtures/cask/naked_non_executable
new file mode 100644
index 000000000..039e4d006
--- /dev/null
+++ b/Library/Homebrew/test/support/fixtures/cask/naked_non_executable
@@ -0,0 +1,2 @@
+#!/bin/sh
+exit 0