Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Homebrew should make use of .curlrc #15419

Closed
wants to merge 1 commit into from

4 participants

@sfnelson

Rather than silently ignoring it (#13836).

Includes a doctor check to detect .curlrc and ensure that http downloads work.

(accidentally closed previous pull request (#15383))

@samueljohn

I like this doctor check because it tests general connectivity. Also good that it mentions the connection might be offline. The good thing is people can't open issues about this while being offline :-D

Hopefully, for difficult internet connection setups, people can fiddle with their .curlrc to make it work through proxies and such.

You removed the -q from the curl-flags. What does it do?

Besides: You'll need to squash into a single commit. Do you know how to do that with git rebase -i HEAD~3?

@sfnelson

The -q flag causes curl to ignore a user's curlrc configuration file (see #13836). I did not squash these commits because I've already had comments on them from my previous pull request (#15383). I wanted those who commented to be able to see what has changed. I can squash them if required.

I did wonder whether this doctor check should be applied whether or not the user has a custom .curlrc file: checking internet connectivity seems like a good idea in general. The check could augment the message with a comment about .curlrc if the check fails and the file exists. Thoughts?

@sfnelson sfnelson closed this
@sfnelson sfnelson reopened this
Library/Homebrew/cmd/doctor.rb
((11 lines not shown))
+ end
+ return
+ rescue
+ end
+
+ <<-EOS.undent
+ Homebrew was unable to access 'http://github.com' using curl
+ Homebrew needs curl to download packages. If your internet connection
+ is working otherwise it is possible that your curl configuration file
+ (~/.curlrc) is at fault.
+
+ Please make sure that you can run the following command successfully:
+ curl http://github.com
+ EOS
+end
+
@jacknagel Collaborator

The structure here can be simplified a bit by moving the rescue to the method level and sticking the heredoc inside:

def check_user_curlrc
  return if File.exist? "#{ENV['HOME']}/.curlrc"

  nostdout { curl('-o', '/dev/null', '--head', 'http://github.com') }
rescue
  <<-EOS.undent
    ...
  EOS
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jacknagel
Collaborator

I think the scope of the check is fine for now; I can see the argument for a blanket "can we download things" check, but it does mean that the user wouldn't be able to get a clean bill of health from brew doctor when offline, so...

@sfnelson

Thanks for the suggestions. I'm not very familiar with ruby and I didn't realise I could do that. I've made the changes you suggest.

Given that my doctor check is now 2 functional lines (written by jacknagel), and the whole point of the commit is to remove a flag that should never have been set in the first place... time to pull?

@mxcl
Collaborator

I'm removing the curl test, but leaving the warning if .curlrc exists. This is more in line with how brew doctor works generally (ie. diagnose but don't do full testing for side-effects), and removes my issue with how this patch works.

@mxcl mxcl closed this pull request from a commit
@sfnelson sfnelson Homebrew should not ignore curlrc
Added doctor check for .curlrc rather than silently ignoring it (#13836).

Closes #15419.

Signed-off-by: Max Howell <mxcl@me.com>

Removed test in doctor where it actually curl'd a file. It's enough to warn if the curlrc exists. I understand people want to remove the warnings, but the point in the doctor is to help diagnose and not to be some ramification of your UNIX system.
ef25978
@mxcl mxcl closed this in ef25978
@theirix theirix referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@Sharpie Sharpie referenced this pull request from a commit in Sharpie/homebrew
@sfnelson sfnelson Homebrew should not ignore curlrc
Added doctor check for .curlrc rather than silently ignoring it (#13836).

Closes #15419.

Signed-off-by: Max Howell <mxcl@me.com>

Removed test in doctor where it actually curl'd a file. It's enough to warn if the curlrc exists. I understand people want to remove the warnings, but the point in the doctor is to help diagnose and not to be some ramification of your UNIX system.
183d3c3
@sessyargc sessyargc referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@snakeyroc3 snakeyroc3 referenced this pull request from a commit in snakeyroc3/homebrew
@sfnelson sfnelson Homebrew should not ignore curlrc
Added doctor check for .curlrc rather than silently ignoring it (#13836).

Closes #15419.

Signed-off-by: Max Howell <mxcl@me.com>

Removed test in doctor where it actually curl'd a file. It's enough to warn if the curlrc exists. I understand people want to remove the warnings, but the point in the doctor is to help diagnose and not to be some ramification of your UNIX system.
0c922ee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 15, 2012
  1. @sfnelson

    Homebrew should not ignore curlrc. Added doctor check for .curlrc rat…

    sfnelson authored
    …her than silently ignoring it (#13836).
This page is out of date. Refresh to see the latest.
Showing with 15 additions and 1 deletion.
  1. +14 −0 Library/Homebrew/cmd/doctor.rb
  2. +1 −1  Library/Homebrew/global.rb
View
14 Library/Homebrew/cmd/doctor.rb
@@ -473,6 +473,20 @@ def check_user_path_3
end
end
+def check_user_curlrc
+ return unless File.exists? "#{ENV['HOME']}/.curlrc"
+ nostdout { curl('-o','/dev/null','--head','http://github.com') }
+rescue
+ <<-EOS.undent
+ Homebrew was unable to access 'http://github.com' using curl
+ Homebrew needs curl to download packages. If your internet connection
+ is working otherwise it is possible that your curl configuration file
+ (~/.curlrc) is at fault. Please make sure that you can run the
+ following command successfully:
+ curl http://github.com
+ EOS
+end
+
def check_which_pkg_config
binary = which 'pkg-config'
return if binary.nil?
View
2  Library/Homebrew/global.rb
@@ -74,7 +74,7 @@ def mkpath
HOMEBREW_USER_AGENT = "Homebrew #{HOMEBREW_VERSION} (Ruby #{RUBY_VERSION}-#{RUBY_PATCHLEVEL}; #{OS_VERSION})"
-HOMEBREW_CURL_ARGS = '-qf#LA'
+HOMEBREW_CURL_ARGS = '-f#LA'
module Homebrew extend self
include FileUtils
Something went wrong with that request. Please try again.