This repository has been archived by the owner. It is now read-only.

Check /usr/local/lib is writable in doctor #18571

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

rtwomey commented Mar 18, 2013

  • Added as part of check_access_usr_local doctor cmd, so both
    /usr/local and /usr/local/lib are checked to be writable.
Contributor

adamv commented Mar 18, 2013

Needs to be separate as the AirFoil comment only applies to /usr/local

Contributor

adamv commented Mar 18, 2013

In the new check, add include too

rtwomey commented Mar 18, 2013

Updated with your suggestions, thanks!

@samueljohn samueljohn and 1 other commented on an outdated diff Mar 26, 2013

Library/Homebrew/cmd/doctor.rb
@@ -310,6 +310,18 @@ def __check_subdir_access base
def check_access_usr_local
return unless HOMEBREW_PREFIX.to_s == '/usr/local'
+ ['/usr/local/include', '/usr/local/lib'].each do |dir|
@samueljohn

samueljohn Mar 26, 2013

Contributor

I have a homebrew at /homebrew, so I wonder if we should perhaps check this for HOMEBREW_PREFIX/'include' and lib instead?
The "return unless" two lines above should then also be removed.

@rtwomey

rtwomey Mar 29, 2013

Updated with your suggestion and refactored that method a little bit. Thanks!

Contributor

adamv commented Mar 29, 2013

Please squash to a single commit for review, thanks

@jacknagel jacknagel commented on an outdated diff Apr 1, 2013

Library/Homebrew/cmd/doctor.rb
- You should probably change the ownership and permissions of /usr/local
- back to your user account.
- EOS
+ return msg + "You should probably change the ownership and permissions of #{dir} back to your user account."
@jacknagel

jacknagel Apr 1, 2013

Contributor

This should be hard wrapped at 80 columns

@jacknagel jacknagel commented on an outdated diff Apr 1, 2013

Library/Homebrew/cmd/doctor.rb
- unless File.writable_real?("/usr/local") then <<-EOS.undent
- The /usr/local directory is not writable.
- Even if this directory was writable when you installed Homebrew, other
- software may change permissions on this directory. Some versions of the
- "InstantOn" component of Airfoil are known to do this.
+ if dir == '/usr/local'
@jacknagel

jacknagel Apr 1, 2013

Contributor

I don't think this will work right, HOMEBREW_PREFIX is a Pathname, and Pathname.new("/usr/local") != "/usr/local"

@jacknagel jacknagel commented on an outdated diff Apr 1, 2013

Library/Homebrew/cmd/doctor.rb
@@ -308,17 +308,20 @@ def __check_subdir_access base
end
def check_access_usr_local
- return unless HOMEBREW_PREFIX.to_s == '/usr/local'
+ [HOMEBREW_PREFIX, "#{HOMEBREW_PREFIX}/include",
+ "#{HOMEBREW_PREFIX}/lib"].each do |dir| unless File.writable_real?(dir)
@jacknagel

jacknagel Apr 1, 2013

Contributor

newline between |dir| and unless

@rtwomey rtwomey Check additional dirs are writable in doctor
* Checks that HOMEBREW_PREFIX, HOMEBREW_PREFIX/include, and
  HOMEBREW_PREFIX/lib are all writable as part of doctor.
b81b0c6
Contributor

jacknagel commented Apr 13, 2013

This still doesn't work quite right; Array#each returns the array, so in the case where there aren't any warnings, it outputs:

$ brew dr check_access_usr_local
Warning: /usr/local/usr/local/include/usr/local/lib

I'll fix it.

Contributor

jacknagel commented Apr 13, 2013

Turns out we have a check for include already, and it makes more sense to add a similar lib check rather than try to cram it all into one method, so I went a different direction here.

@jacknagel jacknagel closed this in fd11f13 Apr 13, 2013

@ekarulf ekarulf added a commit to ekarulf/homebrew that referenced this pull request Apr 15, 2013

@jacknagel @ekarulf jacknagel + ekarulf doctor: consolidate directory access checks
Also include a writability check for lib.

Closes #18571.
8fdf103

@dshean dshean added a commit to dshean/homebrew that referenced this pull request Sep 24, 2013

@jacknagel @dshean jacknagel + dshean doctor: consolidate directory access checks
Also include a writability check for lib.

Closes #18571.
f0d375a

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.