sous-chefs / homebrew

Development repository for the homebrew cookbook
https://supermarket.chef.io/cookbooks/homebrew
Apache License 2.0
151 stars 136 forks source link

Latest brew install needs extended sudo rights #109

Closed Sauraus closed 6 years ago

Sauraus commented 7 years ago

This fixes: https://github.com/chef-cookbooks/homebrew/issues/105

kameghamegha commented 7 years ago

Hey, @Sauraus, have you completed the DCO step? https://blog.chef.io/2016/09/19/introducing-developer-certificate-of-origin/ Here is some more info.

bdangit commented 7 years ago

would really love this fix right now. đź‘Ť

discentem commented 7 years ago

Why has this not been merged?

andrew-cc commented 7 years ago

@Sauraus There are a bunch of failures on CI from Rubocop—perhaps getting those fixed will move this along to getting merged 🙂

andrew-cc commented 7 years ago

Additional errors I’m getting with this change applied onto 3.0.0:

    ==> Searching online for the Command Line Tools
    ==> /usr/bin/sudo /usr/bin/touch /tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
    ==> Installing Command Line Tools (macOS Sierra version 10.12) for Xcode-8.2
    ==> /usr/bin/sudo /usr/sbin/softwareupdate -i Command\ Line\ Tools\ (macOS\ Sierra\ version\ 10.12)\ for\ Xcode-8.2
    Software Update Tool
    Copyright 2002-2015 Apple Inc.

    Downloading Command Line Tools (macOS Sierra version 10.12) for Xcode
    Downloaded Command Line Tools (macOS Sierra version 10.12) for Xcode
    Installing Command Line Tools (macOS Sierra version 10.12) for Xcode
    Done with Command Line Tools (macOS Sierra version 10.12) for Xcode
    Done.
    ==> /usr/bin/sudo /bin/rm -f /tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
    ==> /usr/bin/sudo /usr/bin/xcode-select --switch /Library/Developer/CommandLineTools
    STDERR: sudo: no tty present and no askpass program specified
    Failed during: /usr/bin/sudo /usr/bin/xcode-select --switch /Library/Developer/CommandLineTools
    ---- End output of /opt/workstation/.chef/local-mode-cache/cache/homebrew_go < /dev/null ----
    Ran /opt/workstation/.chef/local-mode-cache/cache/homebrew_go < /dev/null returned 1

    Resource Declaration:
    ---------------------
    # In /opt/workstation/.chef/local-mode-cache/cache/cookbooks/homebrew/recipes/default.rb

     49:   execute 'install homebrew' do
     50:     command "#{homebrew_go} < /dev/null"
     51:     environment lazy { { 'HOME' => ::Dir.home(homebrew_owner), 'USER' => homebrew_owner } }
     52:     user homebrew_owner
     53:     not_if { ::File.exist? '/usr/local/bin/brew' }
     54:   end
     55: ensure

After which I added /usr/bin/xcode-select to the list of commands, and the Homebrew install worked (though a later install of a Homebrew cask failed with sudo issues).

Sauraus commented 7 years ago

I just ran several kitchen tests on my local mac and they all converged just fine, not a single fault to be found.

discentem commented 7 years ago

@Sauraus Did you run rubocop linting though? This is what the failed build indicates.

Sauraus commented 7 years ago

@discentem I did but I wasn't going to break the code just to satisfy rubocop BTW. When I run rubocop locally I get 16 offenses and not 6.

Inspecting 24 files
.C.C.C.C..W...C........C

Offenses:

Gemfile:10:1: C: Gem rake should appear before tomlrb in their gem group.
gem 'rake'
^^^^^^^^^^
Gemfile:12:1: C: Gem community_cookbook_releaser should appear before stove in their gem group.
gem 'community_cookbook_releaser'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Rakefile:24:23: C: Avoid comma after the last item of a hash.
        progress: true,
                      ^
libraries/homebrew_mixin.rb:22:1: C: Missing top-level class documentation comment.
class Chef12HomebrewUser
^^^^^
libraries/homebrew_mixin.rb:42:14: C: Align the parameters of a method call if they span more than one line.
             "Homebrew owner is 'root' which is not supported. " \ ...
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
providers/cask.rb:46:1: C: Use alias instead of alias_method at the top level.
alias_method :action_cask, :action_install
^^^^^^^^^^^^
providers/cask.rb:47:1: C: Use alias instead of alias_method at the top level.
alias_method :action_uncask, :action_uninstall
^^^^^^^^^^^^
recipes/default.rb:43:14: W: (...) interpreted as grouped expression.
    variables (
             ^
recipes/default.rb:44:12: C: Avoid using {...} for multi-line blocks.
      lazy {
           ^
recipes/default.rb:45:11: C: Use the new Ruby 1.9 hash syntax.
        { :user => homebrew_owner, :hostname => node['hostname'],
          ^^^^^^^^
recipes/default.rb:45:36: C: Use the new Ruby 1.9 hash syntax.
        { :user => homebrew_owner, :hostname => node['hostname'],
                                   ^^^^^^^^^^^^
recipes/default.rb:46:11: C: Use the new Ruby 1.9 hash syntax.
          :commands => node['homebrew']['sudo']['commands'] }
          ^^^^^^^^^^^^
recipes/default.rb:72:121: C: Line is too long. [151/120]
  only_if { shell_out('/usr/local/bin/brew analytics state', user: homebrew_owner).stdout.include?('enabled') != node['homebrew']['enable-analytics'] }
                                                                                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
resources/cask.rb:5:3: C: Align the parameters of a method call if they span more than one line.
  name_attribute: true, ...
  ^^^^^^^^^^^^^^^^^^^^^
resources/cask.rb:10:3: C: Align the parameters of a method call if they span more than one line.
  kind_of: String
  ^^^^^^^^^^^^^^^
test/integration/default/default_spec.rb:10:23: C: Use 0o for octal literals.
  it { should be_mode 0755 }
                      ^^^^

24 files inspected, 16 offenses detected
Sauraus commented 7 years ago

@tas50 I've fixed the rubocop warnings, but the spec tests are something entirely different IMO.

andrew-cc commented 7 years ago

FWIW, as Sauraus seemed to see, we went the route of just installing Homebrew under its own system user which can do passwordless sudo. Then a “real” user just does e.g. sudo -Hu homebrew brew install whatever, and then we can just set the chef homebrew cookbook user attribute to “homebrew”.

Sauraus commented 7 years ago

@andrew-cc saw that yes, still scary to having a user with limitless sudo ;)

andrew-cc commented 7 years ago

Since it’s a user that can’t be logged-in to, some other user must use sudo to become them. It’s perhaps a shame in some sense that Homebrew isn’t designed like any other major *nix package manager. In some sense it’s “safer” having everything owned by a different user since it prevents arbitrary user software from modifying /usr/local since it doesn’t have perms.

Sauraus commented 7 years ago

Agree with your sentiment.

PS. Don't say any of that to the Homebrew maintainers @Brantone has experience with that...

aramprice commented 7 years ago

@Sauraus the spec failures are likely related to this issue https://github.com/chef-cookbooks/homebrew/issues/114.

tas50 commented 6 years ago

I'm going to close this out at this point because this just isn't an appropriate solution for most users. We try to implement the path of least surprise at Chef and giving blanket sudo rights is not going to be expected or desired by a good chunk of our userbase.